Message272121
Yes when I say “text” I mean str objects, as opposed to byte strings, etc.
I wonder if it would be safer to test for TextIOBase. With the read(0) hack, the zero size may be unexpected. A file object may need special handling to generate an empty result, or to not interpret it as an EOF indicator. See e.g. Issue 23804, about zero-sized SSL reads, where both of these problems happened. As long as we document that the text file logic has changed, I think it would be okay. IMO it is better to keep things simple and robust for byte files than add more hacks for text files.
A common base class like HTTPException is useful when a caller may need to handle a set of exceptions in a common way. But a problem with Content-Length or Transfer-Encoding seems like a programming error, so ValueError seems fine IMO.
However I still don’t see why the checks are worthwhile. Sure having both is a technical violation of the current HTTP protocols. But if a caller is likely to end up in one of these situations, it may indicate a problem with the API that we should fix instead. If the checks are unlikely to ever fail, then drop them. Otherwise, I don’t think any of the bugs I pointed out have been addressed.
I don’t like converting bytes-like to bytes. I understand one of the main purposes of accepting bytes-like objects is to avoid copying them, but converting to bytes defeats that. Also, the copying is inconsistent with the lack of copying in _read_iterable().
I am wondering if it would be best to keep new logic out of send(); just put it into _send_output() instead. Once upon a time, send() was a simple wrapper around socket.sendall() without much bloat. But with the current patch, in order to send a single buffer, we have to jump through a bunch of checks, create a dummy lambda and singleton tuple, loop over the singleton, call the lambda, realize there is no chunked encoding, etc. IMO the existing support for iterables should have been put in _send_output() too (when that was an option). There is no need to use send() to send the whole body when endheaders() already supports that. |
|
Date |
User |
Action |
Args |
2016-08-07 14:27:03 | martin.panter | set | recipients:
+ martin.panter, orsenthil, pitrou, harobed, petri.lehtinen, piotr.dobrogost, demian.brecht, whitemice, Rotkraut |
2016-08-07 14:27:03 | martin.panter | set | messageid: <1470580023.31.0.266102920969.issue12319@psf.upfronthosting.co.za> |
2016-08-07 14:27:03 | martin.panter | link | issue12319 messages |
2016-08-07 14:27:02 | martin.panter | create | |
|