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

Allign websocket sending close codes with RFC #1642

Open
soerenbe opened this issue Jun 7, 2024 · 3 comments
Open

Allign websocket sending close codes with RFC #1642

soerenbe opened this issue Jun 7, 2024 · 3 comments

Comments

@soerenbe
Copy link

soerenbe commented Jun 7, 2024

Hi everyone,
I use autobahn with daphne/Django for a websocket application.
We got a usecase where we disconnect the client and reset the connection. To show this in the log files and properly us another reconnect strategy, we decided to use different websocket close codes. (e.g. 1012; Service Restart). We realized that this codes are not supported by daphne (django/daphne#374). After looking around in the code I realized that this is implemented in the in this project.

Code: https://github.com/crossbario/autobahn-python/blob/master/autobahn/websocket/protocol.py#L2054

I am not sure why not all close codes are supported. Even the linked document in the code (https://www.iana.org/assignments/websocket/websocket.xml#close-code-number-rules) list all close codes.

Is there any reason why the close codes 1001-2999 are not supported? I think at least the defined codes 1000-1015 should be supported like described in the document.

I would create a MR, but the code look like this is intentional. :-)

Kind Regards
Sören

@oberstet
Copy link
Contributor

oberstet commented Jun 8, 2024

Autobahn is supposed to support all valid WebSocket standard close codes, and I believe the code is written

# WebSocket protocol close codes

and tested accordingly

https://github.com/crossbario/autobahn-testsuite/blob/master/autobahntestsuite/autobahntestsuite/case/case7_7_X.py

@soerenbe
Copy link
Author

soerenbe commented Jun 8, 2024

Hi @oberstet,.
thanks for point this out. I think I have to dive a little deeper anf figure out whats the difference between the condition/error I ran into ( https://github.com/crossbario/autobahn-python/blob/master/autobahn/websocket/protocol.py#L2054) and the line you pointed out (gets checked at

if code is not None and (code < 1000 or (1000 <= code <= 2999 and code not in WebSocketProtocol.CLOSE_STATUS_CODES_ALLOWED) or code >= 5000):
)

From the first look i might be the difference, that I want to SEND the close code and your code is for RECEIVING close codes.

I will look into that on Monday.

@soerenbe soerenbe changed the title Allign websocket close codes with RFC Allign websocket sending close codes with RFC Jun 10, 2024
@soerenbe
Copy link
Author

Hi @oberstet,
I just explored the project testsuite and autobahntestsuite project. The second one was kinda overwhelming to me, but luckily I can describe the problem with the project testsuite.

I think my first look was right. You where pointing at receiving close codes and I had problems with sending close codes.

I am also was able to specify a "minimal" Testcase. I took the test test_sendClose_invalid_code_value from here:

def test_sendClose_invalid_code_value(self):
and changed the parameter of the sendClose method from 10 to 1012.

I would say, that following test must success (but it doesnt)

    def test_sendClose_invalid_code_value_1012(self):
        self.protocol.sendClose(code=1012)

Also the interface description states that the close code must be "1000 for normal close or 3000-4999 for application specific close":

def sendClose(self, code: Optional[int] = None, reason: Optional[str] = None):

As described above this matches with the implementation in

if code != 1000 and not (3000 <= code <= 4999):
, but I would argue that there are missing close codes.

Ich would also say, that to list of acceptable close codes for sending and receiving should be the same. As pointed out by you, they are already listed in

CLOSE_STATUS_CODES_ALLOWED = [CLOSE_STATUS_CODE_NORMAL,

and checked at

if code is not None and (code < 1000 or (1000 <= code <= 2999 and code not in WebSocketProtocol.CLOSE_STATUS_CODES_ALLOWED) or code >= 5000):

I changed the topic name to match my description.

I think I now have everything together, I have some residual doubts since this is the first time I look that close into this implementation.

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

No branches or pull requests

2 participants