This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: http.server.BaseHTTPRequestHandler inconsistence with Content-Length value
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, ezio.melotti, m.xhonneux, r.david.murray, rhettinger, vstinner, xiang.zhang
Priority: normal Keywords: patch

Created on 2016-06-29 13:34 by m.xhonneux, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue27414.patch xiang.zhang, 2016-08-15 04:31 review
issue27414_v2.patch xiang.zhang, 2016-11-22 08:09 review
Messages (14)
msg269488 - (view) Author: Mathieu Xhonneux (m.xhonneux) Date: 2016-06-29 13:34
With Python 3.5, when I subclass SimpleHTTPRequestHandler, which itself subclasses BaseHTTPRequestHandler, and I try to access a non-existing file, the server responds with a 404 code, but send_error (see Lib/http/server.py, line 473) adds the Content-Length header with an int value, whereas all others functions convert this value to str (see lines 699, 761).

For consistency, all header values should be str.
msg269491 - (view) Author: Mathieu Xhonneux (m.xhonneux) Date: 2016-06-29 13:36
It's line 469 in the latest Lib/http/server.py (and not 473).
msg269501 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-06-29 14:30
For reference, the changeset that introduced that was ee34cb049a10.  An interesting feature of that changset is the trace command in the test...I guess the wrong patch was comitted and no one noticed.  The trace was obvious fixed but no one appears to have noticed the int problem, presumably because it gets converted to string before being emitted.

Would you be interested in trying to write a test that will detect this, to go along with the trivial fix?
msg269502 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-06-29 14:33
Sorry once again for the infelicitous phrasing.  I meant to say "sorry, what I was unclearly asking for was what you were calling that..."  Too bad I don't drink coffee or I could blame it on not having had any yet today...
msg272715 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-08-15 04:31
I write a patch for this trivial fix along with a corresponding test. Hope it helps. :)
msg273572 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-08-24 15:02
Ping. :)
msg273580 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-08-24 17:24
It would be better to use support.swap_attr().
msg273582 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-08-24 17:36
Thanks for the reply berker. It's nice to know this helper. :) But I'd like to keep it as now since I don't see any real advantage to use it and swap_attr will set the attribute even when it's absent, this is not wanted.
msg273589 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-08-24 20:27
In which case would the send_header method be absent?
msg273797 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-08-27 19:32
Delete by other test cases without restoring? Which is rather impossible to happen. But I'd like it to stay like now. It is not complex and straight when reading. I don't want to change it to swap_attr.
msg281398 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-21 22:33
issue27414.patch is not good: see my review.
msg281441 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-11-22 08:09
v2 applies the suggestions.
msg281442 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-22 08:31
Mathieu: "For consistency, all header values should be str."

Xiang: issue27414_v2.patch: Doc/library/http.server.rst
"*keyword* and *value* should both be :class:`str`."

I'm sorry, but I don't understand the whole point of this issue. The current code works, I don't see any good reason to change it.

The unit test is a little bit complex for a little tiny implementation detail. Usually, we try to avoid unit tests on implementation details in the Python test suite.

It's perfectly fine to pass an integer to an HTTP value, it's just super convenient.

I checked the HTTP client: the type of the 'argument' parameter of putheader() is not documented neither, so I think that it's just fine to leave the doc unchanged.

I close now this issue NOTABUG.
msg281443 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-22 08:43
Ah, on IRC Xiang told me that send_header("Connection", 1) raises an
error. Right, *this* specific case raises an error. But I don't think
that it's worth to require text for HTTP header values.

If you consider that it's a shame, Python should handle this corner
case (sorry, what is use case for "Connection: 1" ? :-D), please open
a new issue.

If you consider that the doc the HTTP server must be more explicit on
the accepted types, please mention that integers are accepted for
values, not only text.
History
Date User Action Args
2022-04-11 14:58:33adminsetgithub: 71601
2016-11-22 08:43:07vstinnersetmessages: + msg281443
2016-11-22 08:31:36vstinnersetstatus: open -> closed
resolution: not a bug
messages: + msg281442
2016-11-22 08:09:21xiang.zhangsetpriority: low -> normal
files: + issue27414_v2.patch
messages: + msg281441

versions: + Python 3.7
2016-11-21 22:33:00vstinnersetnosy: + vstinner
messages: + msg281398
2016-08-27 19:32:09xiang.zhangsetmessages: + msg273797
2016-08-25 16:33:22pitrousetnosy: - pitrou
2016-08-24 20:27:15berker.peksagsetmessages: + msg273589
2016-08-24 17:36:01xiang.zhangsetmessages: + msg273582
2016-08-24 17:24:04berker.peksagsetpriority: normal -> low
versions: + Python 3.6, - Python 3.5
nosy: + berker.peksag

messages: + msg273580

stage: needs patch -> patch review
2016-08-24 15:02:48xiang.zhangsetmessages: + msg273572
2016-08-15 04:31:28xiang.zhangsetfiles: + issue27414.patch
keywords: + patch
messages: + msg272715
2016-08-14 17:50:30xiang.zhangsetnosy: + xiang.zhang
2016-08-10 01:15:01martin.pantersetstage: needs patch
2016-06-29 14:33:14r.david.murraysetmessages: + msg269502
2016-06-29 14:30:29r.david.murraysetnosy: + r.david.murray
messages: + msg269501
2016-06-29 14:17:21xiang.zhangsetnosy: + rhettinger, pitrou, ezio.melotti
2016-06-29 13:36:38m.xhonneuxsetmessages: + msg269491
2016-06-29 13:34:36m.xhonneuxcreate