Rolf Krahl added the comment:

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.

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue12319>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to