Repository navigation
protocol_version in http.server.test can be ignored #90443
Description
| BPO | 46285 |
|---|---|
| Nosy | @merwok, @matrixise, @JelleZijlstra, @maggyero, @openalmeida |
| PRs | |
| Dependencies | |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = None
created_at = <Date 2022-01-06.18:39:21.504>
labels = ['type-feature', 'library', '3.11']
title = 'protocol_version in http.server.test can be ignored'
updated_at = <Date 2022-02-03.15:57:34.891>
user = 'https://github.com/openalmeida'bugs.python.org fields:
activity = <Date 2022-02-03.15:57:34.891>
actor = 'eric.araujo'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2022-01-06.18:39:21.504>
creator = 'openalmeida'
dependencies = ['46436']
files = ['50551']
hgrepos = []
issue_num = 46285
keywords = ['patch']
message_count = 11.0
messages = ['409894', '409899', '410012', '410015', '410107', '410161', '410163', '411310', '411392', '411841', '411887']
nosy_count = 5.0
nosy_names = ['eric.araujo', 'matrixise', 'JelleZijlstra', 'maggyero', 'openalmeida']
pr_nums = ['30999']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue46285'
versions = ['Python 3.11']
Activity
openalmeida commentedon Jan 6, 2022
Hi,
Sorry for my poor English, this is not a spam issue.
How to reproduce
================
File about
http/server.py, line 1235 at main branch.1st, change
protocol_version, e.g. from "HTTP/1.0" to "HTTP/1.1":--- protocol="HTTP/1.0", port=8000, ...
+++ protocol="HTTP/1.1", port=8000, ...
2ed, run with
python -m http.serverand test by:curl http://127.0.0.1:8000 2>/dev/null| head -n 1Result
======
The response head line will always been a fixed HTTP Version refer to
BaseHTTPRequestHandler.protocol_versiondefined, thus "HTTP/1.0 200 OK" currently.Expected
========
It should equal to
http.server.test(protocol="...")which specified like above, for this issue, it is expected to be "HTTP/1.1 200 OK".P.S.
====
I know it is just locate in a test code area (http.servers::test), but what I submit here is about a Python Variable Scope issue maybe.
openalmeida commentedon Jan 6, 2022
update
======
It seems I've found the problem, http/server.py#L1277-L1288:
Because of
partial(provide by the functools module), there comes a closure like stuff ?merwok commentedon Jan 7, 2022
Hello and thanks for the report!
Could you tell more about what you’re trying to achieve?
Some notes:
You are not meant to change the code of provided modules, but to instantiate classes with the right parameters, or subclass in your own code to change how some methods work.
Supporting HTTP 1/1 is not just changing the protocol string, but also about supporting the actual behaviours defined in the 1.1 spec!
If the module has a parameter for version and it doesn’t work, then you can report a bug. If there is no such parameter, you can open a feature request to ask for HTTP 1.1 support.
merwok commentedon Jan 7, 2022
I understand your report better after looking at the code.
There is indeed a protocol_version parameter in the test function (which is really a main function, not test), that sets the protocol attribute on the passed handler class. (The class attribute is changed, which seems like a bug!) The BaseHTTPRequestHandler class does have support for HTTP 1.1 (adds automatic keep-alive).
So if we ignore the distraction that OP changed the source code, and imaging instead that they called
test(protocol="HTTP/1.1"), then there would be a bug if the requests went out as HTTP/1.0.Hugo, can you attach a minimal reproducer? (a python script as small as possible to show the problem — don’t edit code in http.server but call the http.server.test function with your own code)
openalmeida commentedon Jan 8, 2022
The short story is, everything is okay,
its my bad to taken the test function out of context,
sorry about that of issue report.
just for details review (related file attached):
check line 1277 to line 1278 (main branch of Python currently):
https://github.com/python/cpython/blob/17b16e1/Lib/http/server.py#L1277-L1278
thus,
functools.partial(closure/wrapper) willmake the parameter
protocolof the functiontestuseless.So, specify a handler class directly.
8 remaining items
[-]http/server.py wont respect its protocol_version[/-][+]protocol_version in http.server.test can be ignored[/+][-]http/server.py wont respect its protocol_version[/-][+]protocol_version in http.server.test can be ignored[/+]maggyero commentedon Jan 23, 2022
Thanks Hugo for opening this issue and Éric for inviting me.
As you guys pointed out, function test in module http.server expects a real handler class argument (SimpleHTTPRequestHandler or CGIHTTPRequestHandler), not a partial object partial(SimpleHTTPRequestHandler, directory=args.directory) or partial(CGIHTTPRequestHandler, directory=args.directory), so that the assignment of protocol_version class attribute in test is not ignored.
The partial object in the if __name__ == '__main__' branch of module http.server was introduced in the first place to pass the directory argument to the handler class’s __init__ method called in method BaseServer.finish_request:
But finish_request is a factory method of BaseServer (the abstract creator) so it is DESIGNED to be overridden in subclasses to customize the instantiation of the handler class BaseRequestHandler (the abstract product). So the proper way to instantiate SimpleHTTPRequestHandler and CGIHTTPRequestHandler with the directory argument is to override BaseServer.finish_request.
That is what I have just did by updating my PR here: fc7f95f
It fixes both bpo-46285 and bpo-46436.
openalmeida commentedon Jan 27, 2022
Hi Éric, thank you so much.
I know only a little usage of closure and functools.partial but not the historical/relative knowledge of their design/feature, I mean this issue have 2 visual, the partical object not working as it expected or we are not calling the partical as it expected:
"""
import functools
# if jbo totally equal to obj in anywhere
# (the so called Duck Type or
# maybe the LSP rule, Liskov
# Substitution Principle)
# the use of partical in http.server
# should also be ok
# I used the partical times the same
# as http.server's author did
# and I told myself I know its usage
# but now I do not think so
"""
merwok commentedon Jan 27, 2022
There is no bug with partial itself. I have tried to explain this and I can’t write it in another way.
merwok commentedon May 3, 2022
#30999 was merged. Thanks to all!