h11 icon indicating copy to clipboard operation
h11 copied to clipboard

write_any_response not sending optional causes failure on Samsung TV

Open shaolo1 opened this issue 7 years ago • 6 comments

I'm trying to work on an app that communicates with my Samsung TV. After some digging I figured out that the missing status text causes it to not work.

def write_any_response(response, write):
    if response.http_version != b"1.1":
        raise LocalProtocolError("I only send HTTP/1.1")
    status_bytes = str(response.status_code).encode("ascii")
    # We don't bother sending ascii status messages like "OK"; they're
    # optional and ignored by the protocol. (But the space after the numeric
    # status code is mandatory.)
    #
    # XX FIXME: could at least make an effort to pull out the status message
    # from stdlib's http.HTTPStatus table. Or maybe just steal their enums
    # (either by import or copy/paste). We already accept them as status codes
    # since they're of type IntEnum < int.
    write(bytesmod(b"HTTP/1.1 %s %s\r\n", (status_bytes, response.reason)))
write_headers(response.headers, write)

Seeing as how there is a FIXME here already. Would it be possible to get the following added?

from http import HTTPStatus
response.reason = HTTPStatus(response.status_code).name.encode('ascii')

shaolo1 avatar Aug 20 '18 19:08 shaolo1

That comment is out of date... As you can see from the code, h11 does send whatever "reason" text the user gives it.

Can you give more details on how you're using h11, and how you concluded that Samsung's http implementation is broken?

njsmith avatar Aug 20 '18 20:08 njsmith

I'm using Quart, which uses h11. The app I'm working on is not quite ready to be published. I've got it working with Flask, but the tv won't respond to the Quart version unless I populate the response.reason. By your comment, I'm assuming the response.reason should be populated before it gets to write_any_response().

I see asgi_send() being called with the message, but then it builds a new response using only the status. self.send(h11.Response(status_code=self.response['status'], headers=headers))

Where is the correct place for me to supply the reason?

shaolo1 avatar Aug 20 '18 20:08 shaolo1

The quick fix would be to pass the reason phrase into the h11.Response constructor, like: h11.Response(status_code=self.response['status'], headers=headers, reason="blah blah")

Out of curiosity, does your TV require a specific reason phrase, or does it just insist that it be non-empty?

Given that this is breaking things (wtf is wrong with you, samsung TV), maybe we should also start defaulting the reason inside h11 when it's not specified. Probably the way to do this would be in the Reponse/InformationalResponse constructor, if the reason argument isn't passed, set a default based on the status code.

njsmith avatar Aug 20 '18 23:08 njsmith

Interestingly enough its completely happy with b'HTTP/1.1 200 foo\r\n' or b'HTTP/1.1 206 foo\r\n'

shaolo1 avatar Aug 20 '18 23:08 shaolo1

Heh. Well, I guess that's another option: if reason isn't given, default to foo.

(I'm joking!)

njsmith avatar Aug 21 '18 00:08 njsmith

I finally got around to posting the app I mentioned that has the problem on my tv. https://github.com/shaolo1/VideoServer. I've temporarily monkey patched the h11 issue to get it working.

shaolo1 avatar Aug 22 '18 16:08 shaolo1