Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-85162: Add HTTPSServer to http.server#129607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
picnixz merged 54 commits intopython:mainfromdonBarbos:issue-85162
Apr 5, 2025

Conversation

donBarbos
Copy link
Contributor

@donBarbosdonBarbos commentedFeb 3, 2025•
edited
Loading

I decided to resume the work on the implementation of HTTPS server inhttp.server module.
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 inLib/http/server.py file, 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/

@donBarbosdonBarbos changed the titlegh-85162: Add HTTPS support to http.server.HTTPServergh-85162: Add HTTPSServer to http.serverFeb 3, 2025
picnixz
picnixz previously requested changesFeb 3, 2025
Copy link
Member

@picnixzpicnixz left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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.

donBarbos reacted with thumbs up emoji
@bedevere-app
Copy link

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 phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@donBarbos
Copy link
ContributorAuthor

I have made the requested changes; please review again

@bedevere-app
Copy link

Thanks for making the requested changes!

@picnixz: please review the changes made to this pull request.

Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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.

@donBarbos
Copy link
ContributorAuthor

donBarbos commentedFeb 3, 2025•
edited
Loading

@picnixz
Initially, I was guided by the comment left by@tiran in the previous PR:
#20923 (review)

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

@picnixz
Copy link
Member

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).

donBarbos reacted with thumbs up emoji

Copy link
Member

@gpsheadgpshead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

regardless of review, I still wonder if we actuallywant 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 inhttps://docs.python.org/3.14/library/http.server.html thathttp.server is not a production quality secure server.

@donBarbos
Copy link
ContributorAuthor

I think it's important to add this because there are many useful use cases.
all such people first go to the documentation before opening an issue.

And I don't see any reason why they would open an issue if they were already familiar with the concept ofhttp.server

@donBarbos
Copy link
ContributorAuthor

@gpshead what do you think now about the merge of this PR?

@donBarbosdonBarbos requested a review frompicnixzMarch 16, 2025 11:27
Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Sorry, but I missed one possible case

Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Just an improved blurb/what's new and I think it's good to go.

@picnixz
Copy link
Member

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.

donBarbos reacted with thumbs up emoji

@picnixzpicnixzenabled auto-merge (squash)April 5, 2025 08:26
@picnixzpicnixzdisabled auto-mergeApril 5, 2025 08:26
@picnixzpicnixzenabled auto-merge (squash)April 5, 2025 08:28
@picnixzpicnixz merged commit37bc386 intopython:mainApr 5, 2025
38 checks passed
@donBarbos
Copy link
ContributorAuthor

Thanks a lot everyone

seehwan pushed a commit to seehwan/cpython that referenced this pull requestApr 16, 2025
…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.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@picnixzpicnixzpicnixz approved these changes

@gpsheadgpsheadgpshead approved these changes

Assignees

@picnixzpicnixz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@donBarbos@picnixz@gpshead
[8]ページ先頭
©2009-2025 Movatter.jp