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

♻️ Update root_path handling (from --root-path CLI option) to include the root path prefix in the full ASGI path as per the ASGI spec #2213

Merged
merged 8 commits into from
Jan 16, 2024

Conversation

tiangolo
Copy link
Sponsor Member

@tiangolo tiangolo commented Jan 13, 2024

Summary

♻️ Update root_path handling (from --root-path CLI option) to include the root path prefix in the full ASGI path as per the ASGI spec, equivalent to Starlette 0.35.0

Why

Uvicorn doesn't receive the scope from another ASGI component on top, it creates it from the raw HTTP protocol.

When it receives the CLI option --root-path it means that Uvicorn is behind another HTTP frontend proxy (like Traefik or Nginx) that is receiving that "root path" prefix and stripping it.

More or less like this:

  • The client sends a request to: https://example.com/api/v1/items/42
  • The HTTP proxy (e.g. Traefik, Nginx, HAProxy) is listening on the server at example.com, so it takes this request
    • The HTTP proxy handles HTTPS with the certificates it has installed (e.g. with some Let's Encrypt) and handles the HTTPS communication
    • The HTTP proxy takes the path /api/v1/items/42 and removes the prefix it has configured of /api/v1
  • The HTTP proxy sends the pure and unencrypted HTTP request (after handling the TLS/HTTPS part) to the local Uvicorn server
    • Uvicorn is listening on http://127.0.0.1:8000, notice that it's not HTTPS, only HTTP, and it's only on localhost, not the open web IP, it's also on port 8000, not in the standard HTTP port 80 nor HTTPS port 443
    • the HTTP proxy sends the request with the removed path prefix configured, so, the request URL goes to http://127.0.0.1/items/42
    • Uvicorn is handling an ASGI app (e.g. Starlette, FastAPI) that knows of the path /items/42, but doesn't know of the prefix /api/v1, but the app might need to create some url_for in templates or such, so it needs to also know the original path prefix it should use.
    • Uvicorn is running with the option --root-path=/api/v1, the same path prefix used by the HTTP proxy, to let it know that it is being served to the external clients at that path prefix.
    • Uvicorn creates an ASGI scope that has as the root_path the value in the CLI option: /api/v1. But the path it received by the pure HTTP protocol was only /items/42 because the HTTP proxy sent the request to the local Uvicorn running on localhost.
    • So, to be compliant with the ASGI spec and make the path have the full path used by the clients, including any path prefix, it has to prepend that path prefix provided by the CLI --root-path to the ASGI scope key path.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@tiangolo tiangolo marked this pull request as ready for review January 13, 2024 12:45
@tiangolo
Copy link
Sponsor Member Author

Note, @Kludex has a question about how the access log works with this, we'll check that later, but DO NOT MERGE before that is resolved.

@Kludex
Copy link
Sponsor Member

Kludex commented Jan 14, 2024

The access logs are fine with this PR. 👍

@Kludex Kludex added the hold Don't merge yet label Jan 14, 2024
@tiangolo
Copy link
Sponsor Member Author

Thanks @Kludex! 🚀

Then I think this is ready for review. 🤓

tests/protocols/test_http.py Outdated Show resolved Hide resolved
uvicorn/protocols/http/httptools_impl.py Outdated Show resolved Hide resolved
tiangolo and others added 2 commits January 16, 2024 14:22
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
@Kludex Kludex changed the title ♻️ Update root_path handling (from --root-path CLI option) to include the root path prefix in the full ASGI path as per the ASGI spec, equivalent to Starlette 0.35.0 ♻️ Update root_path handling (from --root-path CLI option) to include the root path prefix in the full ASGI path as per the ASGI spec Jan 16, 2024
@Kludex Kludex merged commit 4af46c9 into encode:master Jan 16, 2024
15 checks passed
@tiangolo tiangolo deleted the root-path branch January 16, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hold Don't merge yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants