Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

On incomplete HTTP reads and the requests library in Python

blog.petrzemek.net

Post reply on HN
Print view View original

21–30 of 32 posts

Re: On incomplete HTTP reads and the requests library in Python

#21

Post by greglindahl »

dagenix wrote: post #20
wumpus wrote: post #19

Earlier quoted context omitted.

Even after this is fixed the way you want it, webservers will still send you half of a PNG because it's a truncated file on the webserver. The webserver will give the truncated length as the length, the actual transfer will be the truncated length, requests will think all is well, and you'll have a problem.

We can't fix every problem. And fixing every problem shouldn't be a gating function for fixing some problems. Web servers generally want to send a complete response. And sometimes some middle box chops off the response halfway through. And if that origin server set a correct Content-Length header and the response isn't that long, we can be fairly certain that the response is probably incomplete. And I really can't co…

Er, that's not what I'm saying.

I'm saying that if you get a response from a webserver, you have to think about it being truncated. Period. No matter what the libraries you're using do for the many different cases of problem that can cause truncation.

You seem to have read a lot into my words that I didn't intend to put there.

Re: On incomplete HTTP reads and the requests library in Python

#22

Post by dagenix »

greglindahl wrote: post #21
dagenix wrote: post #20

Earlier quoted context omitted.

We can't fix every problem. And fixing every problem shouldn't be a gating function for fixing some problems. Web servers generally want to send a complete response. And sometimes some middle box chops off the response halfway through. And if that origin server set a correct Content-Length header and the response isn't that long, we can be fairly certain that the response is probably incomplete. And I really can't co…

Er, that's not what I'm saying. I'm saying that if you get a response from a webserver, you have to think about it being truncated. Period. No matter what the libraries you're using do for the many different cases of problem that can cause truncation. You seem to have read a lot into my words that I didn't intend to put there.

I fully agree that there can always be garbage coming back and that any robust client needs to be aware of that happening. I'm sorry if I read too much into what you were saying, but, it sounded like you were arguing that the change shouldn't be made because there are other ways that corruption could occur that this change wouldn't solve. But, it sounds like you were just warning that this change wouldn't be a panacea? Is that accurate?

Re: On incomplete HTTP reads and the requests library in Python

#23

Post by greglindahl »

dagenix wrote: post #22
greglindahl wrote: post #21

Earlier quoted context omitted.

Er, that's not what I'm saying. I'm saying that if you get a response from a webserver, you have to think about it being truncated. Period. No matter what the libraries you're using do for the many different cases of problem that can cause truncation. You seem to have read a lot into my words that I didn't intend to put there.

I fully agree that there can always be garbage coming back and that any robust client needs to be aware of that happening. I'm sorry if I read too much into what you were saying, but, it sounded like you were arguing that the change shouldn't be made because there are other ways that corruption could occur that this change wouldn't solve. But, it sounds like you were just warning that this change wouldn't be a panace…

I didn't want to express any opinion on the change; I use aiohttp for my python-powered web-scale crawling needs. So yes, it's accurate that I was warning that the change would be incremental and not a panacea.

BTW aiohttp has the problem of currently having a too-strict http protocol parser, so it throws errors for many rare cases of bad webservers, which browsers don't have a problem with. As a crawler, I need to be able to work with whatever a browser will display, ...

Re: On incomplete HTTP reads and the requests library in Python

#24

Post by TheSwordsman »

ubernostrum wrote: post #17
TheSwordsman wrote: post #14

Earlier quoted context omitted.

I'm not sure I agree with your assessment that sending an incorrect Content-Length is permitted. This specific post quotes a section in RFC-2616 that deals with this directly[1]: When a Content-Length is given in a message where a message-body is allowed, its field value MUST exactly match the number of OCTETs in the message-body. According to this, the Content-Length MUST match the data. Can you share an example of…

RFC2616 is obsoleted by RFC7230. Relevant section: https://tools.ietf.org/html/rfc7230#section-3.3.2 Occurrences of MUST NOT for the server in that section are: A sender MUST NOT send a Content-Length header field in any message that contains a Transfer-Encoding header field. (so can't send both Content-Length and Transfer-Encoding) A server MAY send a Content-Length header field in a response to a HEAD request (Sect…

I'd say the HEAD section is the best implication about the behavior they expect. Where in this section there is precedent set for the spec requiring it to be equal.

I'll agree and admit that implication is not explicit specification, but I think it's a reasonable example to follow.

Re: On incomplete HTTP reads and the requests library in Python

#25

Post by greglindahl »

ubernostrum wrote: post #17
TheSwordsman wrote: post #14

Earlier quoted context omitted.

I'm not sure I agree with your assessment that sending an incorrect Content-Length is permitted. This specific post quotes a section in RFC-2616 that deals with this directly[1]: When a Content-Length is given in a message where a message-body is allowed, its field value MUST exactly match the number of OCTETs in the message-body. According to this, the Content-Length MUST match the data. Can you share an example of…

RFC2616 is obsoleted by RFC7230. Relevant section: https://tools.ietf.org/html/rfc7230#section-3.3.2 Occurrences of MUST NOT for the server in that section are: A sender MUST NOT send a Content-Length header field in any message that contains a Transfer-Encoding header field. (so can't send both Content-Length and Transfer-Encoding) A server MAY send a Content-Length header field in a response to a HEAD request (Sect…

It's worth noting that browsers allow a lot of things that either look like errors to a human or are forbidden by the RFCs. Some people want defacto behavior, others want dejure.

Re: On incomplete HTTP reads and the requests library in Python

#26

Post by dagenix »

greglindahl wrote: post #23
dagenix wrote: post #22

Earlier quoted context omitted.

I fully agree that there can always be garbage coming back and that any robust client needs to be aware of that happening. I'm sorry if I read too much into what you were saying, but, it sounded like you were arguing that the change shouldn't be made because there are other ways that corruption could occur that this change wouldn't solve. But, it sounds like you were just warning that this change wouldn't be a panace…

I didn't want to express any opinion on the change; I use aiohttp for my python-powered web-scale crawling needs. So yes, it's accurate that I was warning that the change would be incremental and not a panacea. BTW aiohttp has the problem of currently having a too-strict http protocol parser, so it throws errors for many rare cases of bad webservers, which browsers don't have a problem with. As a crawler, I need to b…

Well, it sounds like we are in fierce agreement then. Sorry for misunderstanding your comment earlier.

I've been interesting lately in exploring at aiohttp, and your comment about it being too strict is certainly very enlightening. And I want to second your comment about libraries following the lead of what browsers will do. My strong feeling is that the battle over what HTTP libraries should do has been fought and been decided by browsers, and that whatever browsers do is what HTTP libraries should do as well, regardless of how ugly it is (IIRC, when I last checked, browsers did pay attention to the Content-Length header and wouldn't display results that were shorter than it - but if I'm misremembering, I would happily change my position with respect to honoring this header). The purist in me hates to say that, but, the pragmatist wants to get things done and fighting against browser behavior feels counter-productive at this stage.

Re: On incomplete HTTP reads and the requests library in Python

#27

Post by icebraining »

llccbb wrote: post #11
Twirrim wrote: post #10

Earlier quoted context omitted.

Honestly? I'm with the author here. "Response.ok" should tell me that the response is ok. It's the most natural expectation. If it's only going to check status code, Response.status_code.ok would be more reasonable.

I agree with your point that there should be some baked in capability for a more comprehensive check on the HTTP response. There are probably some edge cases around this regarding stream=True (load header only, not content) and iter_content() which pulls chunks from a buffer so the entire content isn't loaded into memory at once. Probably surmountable. FWIW Response.status_code is an int, so it might need to be a mem…

Response.status_code is an int

No, it acts like an int; whether it is one (should be) irrelevant :)

    class StatusCode(int):
        @property
        def ok(self):
            return 200

Re: On incomplete HTTP reads and the requests library in Python

#28

Post by icebraining »

ubernostrum wrote: post #13
Twirrim wrote: post #10

Earlier quoted context omitted.

Honestly? I'm with the author here. "Response.ok" should tell me that the response is ok. It's the most natural expectation. If it's only going to check status code, Response.status_code.ok would be more reasonable.

It's the most natural expectation. If it's only going to check status code, Response.status_code.ok would be more reasonable. Except that the status line is literally "HTTP 200 OK", and the 'ok' check is simply using the domain terminology. This is not a bad thing. And having 'request.ok' be a general "I have examined every possible aspect of this response for problems and found none" is probably impossible, despite…

Except that the status line is literally "HTTP 200 OK", and the 'ok' check is simply using the domain terminology.

Well, not exactly. From the documentation of .ok: "This is not a check to see if the response code is 200 OK."

Technically, it checks if status code is not between 400 and 600.

Re: On incomplete HTTP reads and the requests library in Python

#29

Post by Latty »

llccbb wrote: post #5

I am confused about the test used here. The `requests` docs very plainly state that `Response.ok` only checks the status code. Looking into the codebase proves that as well. Is there a status code for "I am going to send bad-length content"? This might be a UX problem and not a technical one. The author seems to think that `Response.ok` should be an all-purpose check.

To be fair, basically the whole reason requests exists is to try and have good UX for devs.

Re: On incomplete HTTP reads and the requests library in Python

#30

Post by crdoconnor »

>Secondly, if we throw an exception we irrevocably destroy the data we read. It becomes impossible to access. This means that situations where the user might want to ‘muddle through’, taking as much of the data as they were able to read and keeping hold of it, becomes a little bit harder.

This is an awful justification. They should do this by raising an exception and attaching the response data to it, so the user who wants to "muddle through" can catch the exception and "muddle through" explicitly.

Regardless, this is a clear violation of https://en.wikipedia.org/wiki/Fail-fast and disappointing behavior from a package that prides itself on clear, obvious API behavior.

Post reply on HN