Skip to content

Update to handle Content-Length header without space after colon#102

Open
justin-lavelle wants to merge 1 commit intonils-ohlmeier:mainfrom
justin-lavelle:cl-header-check-fix
Open

Update to handle Content-Length header without space after colon#102
justin-lavelle wants to merge 1 commit intonils-ohlmeier:mainfrom
justin-lavelle:cl-header-check-fix

Conversation

@justin-lavelle
Copy link

@justin-lavelle justin-lavelle commented Nov 1, 2025

PR for Issue #101

Change to handle the Content-Length header when header does not have a space after the colon ":" e.g. "\nContent-Length:0"

Created new constants for use when searching for the headers, keeping the existing ones for the other uses to maintain use fo the preferred form of the headers and keep existing behavior.

If this is an acceptable way to address this issue then the other headers could be handled the same.

Built and tested locally, resolved the issue when using TCP.

@nils-ohlmeier
Copy link
Owner

Thanks for the report #101 and the PR!

Yes your PR appears to be the most straight forward solution to the issue you encountered. What I'm slightly worried about it that as you pointed to the spec it also allows "l : 5\n".
So I guess if we would want to make sipsak's parsers more robust we would need to search for keywords, then look for the ':' and ignore any white spaces. But then just looking for "\nl" might hit a lot of other headers.

I guess the proper way would be to build a full blown parser, which separates lines and then scans each line for ':' and then tries to identify the header name. But that would be a huge change, and not really what I had intended with sipsak.

I will think about this a little bit more.

@justin-lavelle
Copy link
Author

justin-lavelle commented Nov 6, 2025

While the spec does allow for space between the header name and the colon "Header :Value" I've never encountered that in practice across a wide variety of systems. While having the no spaces form is used quite commonly as part of the compact header usage to reduce size of payload, having this issue only gone unnoticed because it doesn't happen with UDP.

Given how long this has been a "hidden" bug and how much use sipsak gets until this came up it's likely not an at all common scenario to have that form with space before the colon.

I think addressing the form of no spaces after the colon is worth addressing in a simple way such as this and pushing off a full parser or more comprehensive "proper" solution is fine until that case comes up. Could always throw regex at it 😜

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