Skip to content

gh-134168: http.server with HTTPS fails to bind IPv6 addresses and ignores --directory flag #134169

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 24, 2025

Conversation

ggqlq
Copy link
Contributor

@ggqlq ggqlq commented May 18, 2025

I add a new HTTPSDualStackServer class(like 'DualStackServer') in _main function to handle IPv6 addresses and the --directory flag.

@bedevere-app
Copy link

bedevere-app bot commented May 18, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@ggqlq ggqlq changed the title Fix http cli gh134168: http.server with HTTPS fails to bind IPv6 addresses and ignores --directory flag May 18, 2025
@ggqlq ggqlq changed the title gh134168: http.server with HTTPS fails to bind IPv6 addresses and ignores --directory flag gh-134168: http.server with HTTPS fails to bind IPv6 addresses and ignores --directory flag May 18, 2025
@picnixz

This comment was marked as resolved.

@picnixz
Copy link
Member

picnixz commented May 18, 2025

And please add tests. I don't understand why --directory is being ignored in the first place.

@bedevere-app
Copy link

bedevere-app bot commented May 18, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

ggqlq and others added 2 commits May 18, 2025 21:44
@ggqlq
Copy link
Contributor Author

ggqlq commented May 19, 2025

And please add tests. I don't understand why --directory is being ignored in the first place.

Should we move the runtime tests from PR #132540 to this one?

@picnixz
Copy link
Member

picnixz commented May 19, 2025

Oh I thought that the other PR was merged. NVM, i'll merge the other one so that you can extend the existing tests.

@picnixz
Copy link
Member

picnixz commented May 20, 2025

Ok, because we have some failures on the build bots, I've reverted the runtime tests but kept the static tests. Since this fix requires serving real files to check whether it works or not, I'll just assume that it locally works and be happy with it (I mean, what could go wrong?) I'm still planning to fix the runtime tests so that we can have a good coverage but it won't be a priority.

@picnixz
Copy link
Member

picnixz commented May 24, 2025

I couldn't reproduce the IPv6 issue because I have disabled IPv6 and I'm too lazy to re-enable it. However, I can confirm that this fixes the --directory issue. I can probably confirm though that I don't have the same error (I have an OSError, meaning that the call to self.socket.bind was still ok but my OS didn't want to serve IPv6, while your error was during the parsing of the arguments, so before).

@picnixz
Copy link
Member

picnixz commented May 24, 2025

@ggqlq Can you manually confirm that IPv6 + SSL works, IPv6 + other directory works, IPv6 + SSL + other directory works. I confirmed with IPv4 o my side.

@ggqlq
Copy link
Contributor Author

ggqlq commented May 24, 2025

I think they can all work properly.

IPv6 + SSL

$ ./python -m http.server -b ::1 --tls-cert ~/ssl/localhost.crt --tls-key ~/ssl/localhost.key
Serving HTTPS on ::1 port 8000 (https://[::1]:8000/) ...

$ curl -k https://[::1]:8000
# ...
<ul>
<li><a href=".azure-pipelines/">.azure-pipelines/</a></li>
<li><a href=".coveragerc">.coveragerc</a></li>
# ...

IPv6 + other directory

tree ~/test
~/test
├── 1
├── 2
└── 3

1 directory, 3 files

$ ./python -m http.server -b ::1 -d ~/test
Serving HTTP on ::1 port 8000 (http://[::1]:8000/) ...

$ curl http://[::1]:8000
# ...
<ul>
<li><a href="1">1</a></li>
<li><a href="2">2</a></li>
<li><a href="3">3</a></li>
</ul>

IPv6 + SSL + other directory

$ ./python -m http.server -b ::1 -d ~/test --tls-cert ~/ssl/localhost.crt --tls-key ~/ssl/localhost.key
Serving HTTPS on ::1 port 8000 (https://[::1]:8000/) ...

$ curl -k https://[::1]:8000
# ...
<ul>
<li><a href="1">1</a></li>
<li><a href="2">2</a></li>
<li><a href="3">3</a></li>
</ul>

@picnixz
Copy link
Member

picnixz commented May 24, 2025

Great!

@picnixz picnixz enabled auto-merge (squash) May 24, 2025 12:14
@picnixz picnixz added the needs backport to 3.14 bugs and security fixes label May 24, 2025
@picnixz picnixz disabled auto-merge May 24, 2025 12:15
@picnixz picnixz enabled auto-merge (squash) May 24, 2025 12:16
@picnixz picnixz merged commit 2fd09b0 into python:main May 24, 2025
41 checks passed
@miss-islington-app
Copy link

Thanks @ggqlq for the PR, and @picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @ggqlq and @picnixz, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 2fd09b011031f3c00c342b44e02e2817010e507c 3.14

@picnixz
Copy link
Member

picnixz commented May 24, 2025

I'll take care of the bp.

@ggqlq
Copy link
Contributor Author

ggqlq commented May 24, 2025

@picnixz Thank you!

picnixz pushed a commit to picnixz/cpython that referenced this pull request May 24, 2025
…ory` when serving over HTTPS (python#134169)

---------
(cherry picked from commit 2fd09b0)

Co-authored-by: ggqlq <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented May 24, 2025

GH-134630 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label May 24, 2025
picnixz pushed a commit to picnixz/cpython that referenced this pull request May 24, 2025
…-directory` when serving over HTTPS (pythonGH-134169)

(cherry picked from commit 2fd09b0)

Co-authored-by: ggqlq <[email protected]>
picnixz added a commit that referenced this pull request May 24, 2025
…tory` when serving over HTTPS (GH-134169) (#134630)

[3.14] gh-134168: fix `http.server` CLI support for IPv6 and `--directory` when serving over HTTPS (GH-134169)
(cherry picked from commit 2fd09b0)

Co-authored-by: ggqlq <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants