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

Support overlapping SIP&HTTP headers in WS HTTP initial requests #3237

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

vladpaiu
Copy link
Member

Summary
Support overlapping SIP&HTTP headers in WS HTTP initial requests

Details
HTTP proxies can sometimes add VIA header to the HTTP request switching from WS to SIP. Since we are using the SIP parser to go over a HTTP request, that will fail with :

Oct 30 13:08:01 vlad-x1carbon ./opensips[31764]: ERROR:core:parse_via: bad char on state 100
Oct 30 13:08:01 vlad-x1carbon ./opensips[31764]: ERROR:core:parse_via: <HTTP/1.1 test@localhost#015#012#015#012>
Oct 30 13:08:01 vlad-x1carbon ./opensips[31764]: ERROR:core:parse_via: via parse failed
Oct 30 13:08:01 vlad-x1carbon ./opensips[31764]: ERROR:core:get_hdr_field: bad via
Oct 30 13:08:01 vlad-x1carbon ./opensips[31764]: INFO:core:parse_headers: bad header field

Steps to reproduce :

  • have OpenSIPS running with a ws listener
  • run curl http://127.0.0.1:5060 -H "Host: 127.0.0.1" -H "Origin: 127.0.0.1" -H "Upgrade: websocket" -H "Connection: Upgrade" -H "Sec-Websocket-Protocol: sip" -H "Sec-Websocket-Key: ONdMbIDTYRA5rFwlmqhK5Q==" -H "Sec-Websocket-Version: 13" -H "Via: HTTP/1.1 test@localhost"

Solution
Introduce a flag in parser to decide if we want to internally implicit parse well-known SIP headers ( current list is VIA / TO / Content-Length / CSEQ ) & add a define to always parse ( aka. assume SIP context ). From WS HTTP contexts, do not parse those.

Compatibility
Should not affect anything

Closing issues
Opening PR to make sure we are all in agreement with the solution here.

Copy link
Member

@razvancrainea razvancrainea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me :)

@vladpaiu vladpaiu merged commit 139b158 into master Oct 31, 2023
88 checks passed
@vladpaiu vladpaiu deleted the ws_support_via_headers_in_http branch October 31, 2023 08:06
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