Fix handling of http header processing for better operability with Samsung DTVs #79
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…msung DTVs, which require 2xCRLF after header
ghost
mentioned this pull request
ghost
mentioned this pull request
mrjimenez
pushed a commit
that referenced
this pull request
Jan 27, 2019
… with Samsung DTVs, which require 2xCRLF after header" This reverts commit 6d0e485. This commit (pull request #79) arbitrarily adds a second empty line to HTTP 1.0 messages at the place where the CONNECTION: header would be if the message was HTTP 1.1 (it leaves 1.1 messages unchanged). This breaks standard-compliant HTTP 1.0 clients. The second empty line is taken as part of the body: this is really basic, and true with ALL HTTP versions, The syntax of ANY HTTP message of ANY version: Request or status line Headers Single empty-line Body If there is a second empty line, it is taken as part of the body. This has two consequences: - If an XML document is following, there will be a blank line before <?xml. Some parsers don't like it. - As the message content-length header was not adjusted when adding the second cr/lf, there will be data missing at the end of the document. Again, for example with XML, if there was no extra white space after the closing tag, the document is now incomplete (e.g. ending with </root). In many cases the change had no visible consequences, because the client is using HTTP/1.1, or because the XML parser is lenient, and the document can be truncated with no consequences (for example if there is extra whitespace at the end). But, for example, upnp-inspector does not discover upmpdcli instances when run with this change, because it uses HTTP 1.0, and the XML description document it receives lacks a closing '>' Also: adding the spurious empty line was done as part of processing the 'C' directive (Connection: close). There is no guarantee that this header is the last (currently: it is always last as far as I can see). If further headers are added, of course, they will be ignored. This change was performed to accomodate Samsung TVs. This should be done instead at the application level by adding an empty line in the content itself. (cherry picked from commit 33144fa)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Many Samsung DTVs will not recognize UPnP/DLNA media servers, because they are picky about HTTP imlementation. E.g. they loop until double CRLF is found after HTTP header. If not found, then there is no processing done. -> connection failed.
A snippet out of B series TV:
`
.text:0145DCF0 0A 00 A0 E1 MOV R0, R10 ; haystack
.text:0145DCF4 E8 12 9F E5 LDR R1, =asc_215F5E0 ; "\r\n\r\n"
.text:0145DCF8 52 4C C3 EB BL strstr
.text:0145DCFC 00 00 50 E3 CMP R0, #0
.text:0145DD00 E1 FF FF 0A BEQ loc_145DC8C ; loop until EOF found
.text:0145DD04 0A 00 A0 E1 MOV R0, R10
.text:0145DD08 1F FF FF EB BL _Z15cpParseHTTPRespPc ; cpParseHTTPResp(char *)
`
In addition they send "HTTP/1.0" requests, which need to be handled properly, too.
The proposed patch fixes mentioned issues.
The patch was discussed here:
gerbera/gerbera#111