Skip to content
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

Fix server crash on request.server_port when bound to IPv6. #1640

Merged
merged 1 commit into from
Jul 25, 2019

Conversation

Tronic
Copy link
Member

@Tronic Tronic commented Jul 22, 2019

If no X-Forwarded-Port nor Host headers are present, Sanic uses "sockname"
to determine the port. This expected (host, port) tuple to be returned but
for IPv6 a 4-tuple is returned instead. Changed code so that port is picked
up in either case. Handling of "peername" was already correct in this regard.

_get_address and server_port both still return incorrect data or crash for
other socket types (e.g unix). Socket type should checked before any queries.

If no X-Forwarded-Port nor Host headers are present, Sanic uses "sockname"
to determine the port. This expected (host, port) tuple to be returned but
for IPv6 a 4-tuple is returned instead. Changed code so that port is picked
up in either case. Handling of "peername" was already correct in this regard.

_get_address and server_port both still return incorrect data or crash for
other socket types (e.g unix). Socket type should checked before any queries.
@codecov
Copy link

codecov bot commented Jul 22, 2019

Codecov Report

Merging #1640 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1640   +/-   ##
=======================================
  Coverage   91.61%   91.61%           
=======================================
  Files          19       19           
  Lines        2099     2099           
  Branches      391      391           
=======================================
  Hits         1923     1923           
  Misses        138      138           
  Partials       38       38
Impacted Files Coverage Δ
sanic/request.py 97.58% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84b4112...7c7bedf. Read the comment docs.

@Tronic
Copy link
Member Author

Tronic commented Jul 22, 2019

import sanic
app = sanic.Sanic()
app.get("/")(lambda req: sanic.response.text(
    f"server_name={req.server_name}, server_port={req.server_port}\n"
))
app.run("localhost")

Provided that localhost resolves to ::1, run server and test by: curl [::1]:8000
Current git master: Internal Server Error
This pull request: server_name=[, server_port=8000
Pull request #1638: server_name=[::1], server_port=8000

@yunstanford yunstanford merged commit 2363c06 into sanic-org:master Jul 25, 2019
@Tronic Tronic deleted the sockaddrfix branch July 26, 2019 12:12
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