Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

externalPort check cannot be disabled (as the testsuite uses old autobahn ..) #128

Open
joffrey-bion opened this issue Aug 10, 2023 · 9 comments
Labels

Comments

@joffrey-bion
Copy link

joffrey-bion commented Aug 10, 2023

I'm running the Autobahn test suite server in a docker container (actually I may run multiple Autobahn test suite servers on the same machine). The external port through which I communicate is different from 9001 (because I can't have all test servers running on 9001).

However, the tests immediately fail during handshake with the following HTTP 400 error:

port 53701 in HTTP Host header 'localhost:53701' does not match server listening port 9001

The Host header is supposed to be the one used to connect to the server, so it should be expected that the Host contains the port 53701. Why is the test failing?

@oberstet
Copy link
Contributor

what origin header is your client using when connecting to the testsuite containers?

these checks are related to "same origin" policy in websocket, and externalPort is a configuration parameter in autobahn servers (which the testsuite uses) to allow to cover situation where the server cannot determine the port used from outside (eg when you run a LB in front of your servers)

https://github.com/crossbario/autobahn-python/blob/359f868f9db410586cf01c071220994d8d7f165a/autobahn/websocket/protocol.py#L2686
https://github.com/crossbario/autobahn-python/blob/359f868f9db410586cf01c071220994d8d7f165a/autobahn/websocket/protocol.py#L2831

@joffrey-bion
Copy link
Author

what origin header is your client using when connecting to the testsuite containers?

I'm writing a wrapper over several web socket implementations, and using Autobahn TestSuite to verify that my wrapper behaves correctly. So this varies quite a bit:

  • OkHttp: no Origin header, Host: localhost:54634 (or whatever port was dynamically chosen)
  • JDK11 client: no Origin header, Host: localhost:54663 (or whatever port was dynamically chosen)
  • Spring: Origin: http://localhost:54596, Host: localhost:54596 (or whatever port was dynamically chosen)

I have also a couple implementations for the browser, nodejs, and native which might send something different, but it should be along the same lines.

externalPort is a configuration parameter in autobahn servers (which the testsuite uses) to allow to cover situation where the server cannot determine the port used from outside (eg when you run a LB in front of your servers)

This is sort of my case, because I don't control the dynamic external port used for the container, I only know I map it to 9001.
But is externalPort actually configurable for the testsuite server anyway? I didn't find such config param anywhere that I could put in fuzzingserver.json.

@oberstet
Copy link
Contributor

oberstet commented Aug 13, 2023

But is externalPort actually configurable for the testsuite server anyway?

nope, I don't think so, but not setting the externalPort should deactivate the port check altogether (https://github.com/crossbario/autobahn-python/blob/359f868f9db410586cf01c071220994d8d7f165a/autobahn/websocket/protocol.py#L2686) - hence I was confused why it fails!

after a bit of digging, turns out the Autobahn-Python which the Autobahn-Testsuite uses is an older one (frozen to keep results static), and lacks this patch

crossbario/autobahn-python@80fe02d

this patch was done to fix this on autobahn trunk, but the old autobahn used in the testsuite fills externalPort automatically from the websocket URL defined in the testsuite server spec - and that has port 9001

and because of that, the port check will run and then fails:(

@oberstet oberstet changed the title Autobahn test suite server doesn't accept connection if the external port doesn't match the listening port externalPort check cannot be disabled (as the testsuite uses old autobahn ..) Aug 13, 2023
@oberstet oberstet added the bug label Aug 13, 2023
@joffrey-bion
Copy link
Author

Thanks a lot for looking into this! 🙏 That explains it.

the Autobahn-Python which the Autobahn-Testsuite uses is an older one (frozen to keep results static)

Does it mean it will never be upgraded? Or is there a chance this will be fixed in future versions of the test suite?

@oberstet
Copy link
Contributor

Does it mean it will never be upgraded? Or is there a chance this will be fixed in future versions of the test suite?

There are 2 aspects from my PoV:

  1. Autobahn-Python has had a very good I'd claim WebSocket implementation at its core for a long time. Even that "old" version of Autobahn-Python being used in the testsuite. Yes, some issues do pop up over time .. like this one, but nothing I'm aware of that touches the core functionality of the testsuite. And then: a testsuite should act as a "known good" for WebSocket implementors to develop and test against, and it is quite widely used today. What I'm trying to say: any change in the testsuite would need to be tested extensively itself. Risks vs gains? It's not rocket science, even though the old Autobahn used isn't even fully Python 3 (because the testsuite is old now), but would be significant work, also because the testsuite is using Autobahn-Python to break the protocol selectively!
  2. I have personally sunk countless hours into the testsuite and Autobahn, my old company sponsored that work, and I think it made a significant contribution overall to the WebSocket world. And even though I am still an OSS fan and developer, it didn't pay off economically. Very far from it. That's just a sad fact. I am cutting my losses, I've written too much unpaid OSS in my life already;) Hence I want to spend more time on paid closed source. Sorry, bills want to be paid.

@joffrey-bion
Copy link
Author

Thanks for taking the time to answer this anyway. Both points are very fair.

You seem quite exhausted about this project. I hope you'll be able to pass the flame or at least share the burden of maintenance.

@joffrey-bion
Copy link
Author

As for this issue, I'm not sure I have any way to work around it 🤔 I believe I will eventually implement my own test server as an alternative to Autobahn TestSuite.

@oberstet
Copy link
Contributor

You seem quite exhausted about this project.

oh yes, that is true;) looking back, maybe I should have spend my time differently.

I'm not sure I have any way to work around it

you could fork autobahn 0.10.9

https://github.com/crossbario/autobahn-testsuite/blob/master/autobahntestsuite/setup.py#L88

then backport the patch I linked above, and then direct to build the docker image from that

RUN pip install -U pip \

should be doable in 1-2 hours. developing your own testsuite from scratch: sure, but tbh, it'll be lots of work and how can you compare results with other implementations? anyways, good luck!

@joffrey-bion
Copy link
Author

joffrey-bion commented Aug 13, 2023

Thanks for the detailed suggestions ❤️ I might give it a go.

developing your own testsuite from scratch: sure, but tbh, it'll be lots of work and how can you compare results with other implementations? anyways, good luck!

I technically don't have the same goal. I don't intend to compare "my" implementation to others, nor even to the spec itself. I'm just writing adapters of various implementations from different Kotlin target platforms and building a multiplatform library on top of that. My only goal with the testsuite was to test if I didn't mess up the adapters, but not really to verify that the adapted implementations (which are not mine) are 100% spec-compliant. So technically I don't need an extended test suite for all aspects of the RFC.

Also, I sunk many hours trying to get Autobahn Testsuite to work in my project, as it had many ramifications into other areas. There is no real doc about how to run test cases, which required me to read and understand Autobahn testsuite's code (being a Python noob). Also, it requires either a python stack setup or Docker, and the python setup seemed quite complicated (I wanted to have an easy local setup on my dev machines and easy setup on CI too), and outdated (no Python 3), so I opted for Docker. But then Docker is a pain on macOS GitHub Actions runners (killing my CI time, although that's getting better lately with colima support), and is still preventing me from using windows runners at all. Then running Autobahn in a docker-compose from Gradle as a service for my unit tests was also kind of a pain (because the existing plugin is written poorly). Anyway, all this to say that this compounded into a lot of wasted time, every time getting me closer and closer to giving up and writing my own server. This issue is kind of the straw breaking the camel's back :D

PS: Sorry for the wall of text! And sorry because this doesn't really have its place in this GitHub issue 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants