gh-85162: Add HTTPSServer to http.server #129607
Conversation
There was a problem hiding this comment.
Sorry but I can't review what is new and necessary from what is not (even with separate commits). While the style is inconsistent, let's address it in a separate PR if it's really that disturbing.
Note that since we sqash-merge at the end, the style changes will be part of the diff as well making the history harder to review.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
This reverts commit b382985.
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
There was a problem hiding this comment.
The interface doesn't look the nicest to me. Is there an obvious reason why we can't add SSL support to HTTPServer even though it's named HTTPServer? Or should we really make HTTPSServer inherit from HTTPServer?
I am travelling and it's hard to review on mobile but @gpshead may have some preliminary comments on the design. I've left some comments but they can be disregarded for now until the design is fixed.
|
@picnixz I don't want to change the HTTP server interface and overcomplicate it. I also think this matches the principle of separation of responsibilities. but I'm open to discussion |
|
Ok for a separate class but whether it should be a subclass or not is something I don't know if we should do. I don't know which layout will be the best one in the long term (for now let's keep it as a subclass). I don't think I have thought enough about the broader implications so I will hold future reviews for now. I think I need another opinion on this interface (btw, I'm not sure tiran is still active actually, hence the reason why I requested gpshead's opinion). |
There was a problem hiding this comment.
regardless of review, I still wonder if we actually want this in the stdlib? But the implementation is not complicated so I don't think it'll be a maintenance burden in that regard.
The challenge is it may add more reasons for people to file security issues. Despite our existing bold warning up top in https://docs.python.org/3.14/library/http.server.html that http.server is not a production quality secure server.
|
I think it's important to add this because there are many useful use cases. And I don't see any reason why they would open an issue if they were already familiar with the concept of |
|
@gpshead what do you think now about the merge of this PR? |
Misc/NEWS.d/next/Library/2025-02-02-00-30-09.gh-issue-85162.BNF_aJ.rst
Outdated
Show resolved
Hide resolved
|
I'll let it open a few days so that @gpshead can have a look, else I'll merge this before the end of the week. |
|
Thanks a lot everyone |
…r HTTPS (python#129607) The `http.server` module now supports serving over HTTPS using the `http.server.HTTPSServer` class. This functionality is also exposed by the command-line interface (`python -m http.server`) through the `--tls-cert`, `--tls-key` and `--tls-password-file` options.
I decided to resume the work on the implementation of HTTPS server in
http.servermodule.I am ready to respond in time and make changes :-)
I'm opening a new PR because this #20923 had been clearly abandoned.
I took the liberty of defining a single style in
Lib/http/server.pyfile, otherwise everything would have been completely confusing.cc @tiran @remilapeyre @jaswdr
(who was involved with the previous PR)
📚 Documentation preview 📚: https://cpython-previews--129607.org.readthedocs.build/