Author Rotkraut
Recipients Rotkraut, demian.brecht, harobed, martin.panter, matrixise, orsenthil, petri.lehtinen, piotr.dobrogost, pitrou, vstinner, whitemice
Date 2016-08-11.07:52:52
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1470901973.35.0.435108001522.issue12319@psf.upfronthosting.co.za>
In-reply-to
Content
Martin,

> _is_textIO(): I’m sorry but I still prefer the TextIOBase check over
> read(0). Each option has a disadvantage, but with TextIOBase, the
> disadvantage only affects text files, not byte files.

Ok, this is a valid argument.

> Maybe another option is to stick with the current checking of the
> “mode” attribute, or are its failings significant for chunked
> encoding?  It seems to me that the most important use cases would be
> reading bytes from a pipe file, custom bytes file object reader,
> generator, etc, but not a text file without a “mode” attribute.

No, this is not significant for chunked encoding.  As I said, I am happy to accept any method.

But I would say, it's a fairly save bet that any file-like that has the mode attribute is in fact derived from the base classes in the io module.  If this assumption holds, then the TextIOBase check will always work in all cases where the mode attribute check would.  The inverse is not true.  So I'll use the TextIOBase check, hope you agree.

> Checking Content-Length and Transfer-Encoding: I would prefer to
> remove this, unless there is a “good” reason to keep them. I want to
> understand why the three specific checks were added, and what made
> them more special than other potential checks (e.g. the format of
> the Content-Length value, or that the resulting body matches it). If
> the reason is something like “it is too easy for the caller to
> accidentally trigger the problem”, then there may be another way to
> fix it. Or maybe it is a mitigation for a security problem? But at
> the moment, it just seems to me like code being too smart for its
> own good.

Ok, I'll drop them.

> I mentioned a few possible bugs with parsing Transfer-Encoding at
> <https://bugs.python.org/review/12319/diff/14900/Lib/http/client.py#newcode1210>.
> The format of the Transfer-Encoding value is specified at
> <https://tools.ietf.org/html/rfc7230#section-3.3.1> and
> <https://tools.ietf.org/html/rfc7230#section-4>. In Issue 23498, I
> identified some parts of Python that parse header values like this,
> although it looks like http.cookiejar.split_header_words() may be
> the only one usable for Transfer-Encoding.

I admit, I didn't read all comments on Demian's versions of the patch.  Since I'll drop the checks, I guess, these problems are gone now.

> Some of the header field validation was included in Demian’s first
> patch, raising HTTPEncodingError. But I don’t know why he included
> it. Maybe it was more appropriate then; it looks like that patch
> took a different approach than the current encode_chunked=True
> mechanism.

I don't think this is in the current versions of the patch any more.
History
Date User Action Args
2016-08-11 07:52:53Rotkrautsetrecipients: + Rotkraut, orsenthil, pitrou, vstinner, harobed, petri.lehtinen, martin.panter, piotr.dobrogost, demian.brecht, matrixise, whitemice
2016-08-11 07:52:53Rotkrautsetmessageid: <1470901973.35.0.435108001522.issue12319@psf.upfronthosting.co.za>
2016-08-11 07:52:53Rotkrautlinkissue12319 messages
2016-08-11 07:52:52Rotkrautcreate