Skip to content

Conversation

showier-drastic
Copy link
Contributor

@showier-drastic showier-drastic commented Oct 9, 2024

In section 4 of RFC 9112, we can see that the status line should adhere to this format:

  status-line = HTTP-version SP status-code SP [ reason-phrase ]

Even if no reason-phrase is given, there should still be a SP (space) after status-code. edge-http does not have this second SP, if reason is empty.

Example (| marks begin and end of line):

Valid status line: |HTTP/1.1 101 |
Wrong status line (which is the current behavior): |HTTP/1.1 101|

This is causing trouble for the python websockets library, because it want two SPs in the status line (which is valid according to the spec). See code here

This can be reproduced by running the ws_server example on Linux machine, and running python3 -m websockets ws://<ip>:8881:

Failed to connect to ws://<ip>:8881: invalid HTTP status line: HTTP/1.1 101.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Oct 9, 2024

Fair enough - thanks!

@ivmarkov ivmarkov merged commit 8c40cf9 into sysgrok:master Oct 9, 2024
1 check passed
@showier-drastic
Copy link
Contributor Author

Actually I would suggest to remove send_status_line function, and put the logic into send_request and send_status. They are fundamentally different:

  request-line   = method SP request-target SP HTTP-version
  status-line = HTTP-version SP status-code SP [ reason-phrase ]

And, only reason-phrase here is optional. Other elements, such as method and request-target, are mandatory and cannot be empty, so I think they should not be Option<String>. if written checks can also be removed because there will always be spaces.

I can do this refactor if that sounds appropriate.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants