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: protocol_version in http.server.test can be ignored
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.11
process
Status: open Resolution:
Dependencies: 46436 Superseder:
Assigned To: Nosy List: JelleZijlstra, eric.araujo, maggyero, matrixise, openalmeida
Priority: normal Keywords: patch

Created on 2022-01-06 18:39 by openalmeida, last changed 2022-04-11 14:59 by admin.

Files
File name Uploaded Description Edit
my_http.py openalmeida, 2022-01-08 19:41
Pull Requests
URL Status Linked Edit
PR 30999 open maggyero, 2022-01-28 23:49
Messages (11)
msg409894 - (view) Author: Hugo Almeida (openalmeida) Date: 2022-01-06 18:39
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.server` and test by: `curl http://127.0.0.1:8000 2>/dev/null| head -n 1`

Result
======

The response head line will always been a fixed HTTP Version refer to `BaseHTTPRequestHandler.protocol_version` defined, 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.
msg409899 - (view) Author: Hugo Almeida (openalmeida) Date: 2022-01-06 19:09
update
======

It seems I've found the problem, http/server.py#L1277-L1288:

```
        handler_class = partial(SimpleHTTPRequestHandler,
                                directory=args.directory)
```

Because of `partial` (provide by the functools module), there comes a closure like stuff ?
msg410012 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2022-01-07 19:30
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.
msg410015 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2022-01-07 19:38
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)
msg410107 - (view) Author: Hugo Almeida (openalmeida) Date: 2022-01-08 19:41
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) will
# make the parameter `protocol` of the function `test` useless.
# So, specify a handler class directly.
msg410161 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2022-01-09 19:02
I am a bit confused!

The script you attached is to show a problem, but you’re saying there is no problem?
msg410163 - (view) Author: Hugo Almeida (openalmeida) Date: 2022-01-09 20:33
Hi, buddy, there is no problem if invoke the http.server.test function as its designed, I mean the function iteself is okay, thus http/server.py invoked it via the functools.partial wrapper (handler_class) only will case this issue, which technically ignored its protocol parameter's specify. User of http/server.py::test should specify a handler class directly.

If this is a bug, maybe it is about the usage/desire of `functools.partial` ... I am not sure, closure model programming (the lambda etc.) is not eary for me and I closed this issue by courtesy yesterday, if reopen this issue will help/valueable a bit, please tell me.

Improve my English seems too nessary, thank you so much for your warm hearted.
msg411310 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2022-01-23 00:01
I think we have a valid bug, and you correctly attributed it to the use of partial!

(No worry about your English — thank you for reporting the problem here!)


To answer your questions:

1) partial:

Imagine we have a function with many parameters:

  def frob(source, target, backup=False): ...

and we want to call it many times without passing backup=True every time; we can make a lambda:

  frob_backup = lambda source, target: frob(source, target, backup=True)
  # later in the file
  frob_backup(src1, tgt1)
  frob_backup(src2, tgt2)

or a partial:

  frob_backup = partial(frob, backup=True)
  # then
  frob_backup(src3, tgt3)

As you can see, they both work in the same way.  They are equivalent ways of making a shortcut to call a function with some parameters pre-defined.  When called, the lambda will call the frob function and return its return value.  When called, the partial object will call the frob function and return its return value.


2) closures:

Closures are variables that are resolved in the parent scope (namespace) of the normal scope.
A function (including a function created by a lambda) can have closures.

  def make_callback(backup_default):  
      callback = lambda source, target: frob(source, target, backup_default)
      return callback

Here when callback is called, the backup_default variable is not found in the local variables of the callback function, but in the scope of make_callback.  It will be true or false depending on how make_callback was called.


3) http.server.test

There aren’t any closures in http.server.test; the issue is that HandlerClass (defined in the `if name is main` block and passed to `test`) is a proper handler class in one case, or a partial object!  So inside test, setting attributes on the passed handler class will do nothing if that class is actually a partial instance.  When it is called, it will return an instance of SimpleHttpRequestHandler, which won’t see the protocol attribute that we wanted.


Conclusion:
- maybe we should pass all parameters to test instead of smuggling through params
- setting class attributes seems fishy to me
- cgi handler and regular handler should have the same features (see #46436)
msg411392 - (view) Author: Géry (maggyero) * Date: 2022-01-23 18:05
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:

    def finish_request(self, request, client_address):
        """Finish one request by instantiating RequestHandlerClass."""
        self.RequestHandlerClass(request, client_address, self)

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: https://github.com/python/cpython/pull/30701/commits/fc7f95f9d270a8a83cb2fd6d51eb0f904b85e0d9

It fixes both #46285 and #46436.
msg411841 - (view) Author: Hugo Almeida (openalmeida) Date: 2022-01-27 07:32
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:

1) partial is not working as its feature or design said, assume I guessed right, thus partial object behaviors should be totally equal to its wrapper/inside class called with properties applied in a standalone/outside way which we usually used, the use of partical at http.server.test should be okey.

"""
import functools

class Obj:
    pass

obj = Obj(); obj.foo = bar  # usually used
jbo = functools.partial(Obj, foo=bar)

# 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
"""

2) solve this issue itself, to pass handler class directly to ignore partial object caused problem, this is a coding logic shelter/fixing because the way we used functools.partical is not as it expected. This is what cpython@github PR30701 already done. (so the use of partical in http.server is not ok, not welcome at least, so based what I said above, the feature/design of partical now confused me, unless its a bug of partical itself which I am not sure.)
msg411887 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2022-01-27 15:04
There is no bug with partial itself.  I have tried to explain this and I can’t write it in another way.
History
Date User Action Args
2022-04-11 14:59:54adminsetgithub: 90443
2022-02-03 15:57:34eric.araujosetdependencies: + Pass the -d/--directory command-line option to http.server.CGIHTTPRequestHandler
versions: - Python 3.9, Python 3.10
2022-01-28 23:49:38maggyerosetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request29181
2022-01-27 15:04:27eric.araujosetmessages: + msg411887
2022-01-27 07:32:59openalmeidasetmessages: + msg411841
2022-01-23 18:05:00maggyerosetmessages: + msg411392
2022-01-23 00:01:37eric.araujosetnosy: + matrixise, maggyero
2022-01-23 00:01:07eric.araujosetstatus: closed -> open


title: http/server.py wont respect its protocol_version -> protocol_version in http.server.test can be ignored
nosy: + JelleZijlstra
versions: + Python 3.9, Python 3.10
messages: + msg411310
resolution: not a bug ->
stage: resolved -> needs patch
2022-01-09 20:33:09openalmeidasetmessages: + msg410163
2022-01-09 19:02:43eric.araujosetmessages: + msg410161
2022-01-08 19:42:00openalmeidasetstatus: open -> closed
files: + my_http.py
messages: + msg410107

resolution: remind -> not a bug
stage: resolved
2022-01-07 19:38:40eric.araujosetmessages: + msg410015
2022-01-07 19:30:40eric.araujosetnosy: + eric.araujo
messages: + msg410012
2022-01-06 19:09:19openalmeidasetresolution: remind
messages: + msg409899
2022-01-06 18:39:21openalmeidacreate