-
Notifications
You must be signed in to change notification settings - Fork 159
refactor!: use origin instead of base_url in configuration #979
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
Open
iisakkirotko
wants to merge
1
commit into
12-23-feat_change_mutation_detection_and_allow_reactive_boolean_defaults
Choose a base branch
from
01-20-refactor_use_origin_instead_of_base_url_in_configuration
base: 12-23-feat_change_mutation_detection_and_allow_reactive_boolean_defaults
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
import sys | ||
import uuid | ||
import warnings | ||
import logging | ||
from enum import Enum | ||
from pathlib import Path | ||
from typing import Optional, List | ||
|
@@ -178,8 +179,8 @@ class MainSettings(BaseSettings): | |
mode: str = "production" | ||
tracer: bool = False | ||
timing: bool = False | ||
root_path: Optional[str] = None # e.g. /myapp (without trailing slash) | ||
base_url: str = "" # e.g. https://myapp.solara.run/myapp/ | ||
root_path: str = "" # e.g. /myapp (without trailing slash) | ||
origin: Optional[str] = None # e.g. https://myapp.solara.run (without trailing slash) | ||
platform: str = sys.platform | ||
host: str = HOST_DEFAULT | ||
experimental_performance: bool = False | ||
|
@@ -245,5 +246,20 @@ class Config: | |
session.https_only = False | ||
|
||
|
||
# Make sure origin and root_path are correctly configured | ||
if "SOLARA_BASE_URL" in os.environ: | ||
from urllib.parse import urlparse | ||
|
||
base_url = urlparse(os.environ["SOLARA_BASE_URL"]) | ||
|
||
if not main.origin: | ||
main.origin = base_url.scheme + "://" + base_url.netloc | ||
main.root_path = base_url.path.rstrip("/") | ||
logging.warning( | ||
"SOLARA_BASE_URL is deprecated, please use SOLARA_ORIGIN and SOLARA_ROOT_PATH instead. " | ||
"SOLARA_BASE_URL will be removed in a future major version of Solara.", | ||
Comment on lines
+259
to
+260
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! |
||
) | ||
|
||
|
||
# call early so a misconfiguration fails early | ||
assets.extra_paths() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -399,19 +399,17 @@ async def root(request: Request, fullpath: str = ""): | |
""") | ||
if settings.oauth.private and not has_auth_support: | ||
raise RuntimeError("SOLARA_OAUTH_PRIVATE requires solara-enterprise") | ||
root_path = settings.main.root_path or "" | ||
if not settings.main.base_url: | ||
root_path = settings.main.root_path | ||
if not settings.main.origin: | ||
# Note: | ||
# starlette does not respect x-forwarded-host, and therefore | ||
# base_url and expected_origin below could be different | ||
# x-forwarded-host should only be considered if the same criteria in | ||
# uvicorn's ProxyHeadersMiddleware accepts x-forwarded-proto | ||
settings.main.base_url = str(request.base_url) | ||
# if not explicltly set, | ||
configured_root_path = settings.main.root_path | ||
settings.main.origin = request.base_url.scheme + "://" + request.base_url.netloc | ||
scope = request.scope | ||
root_path_asgi = scope.get("route_root_path", scope.get("root_path", "")) | ||
if settings.main.root_path is None: | ||
if settings.main.root_path == "": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we want to keep this to None, to make sure we only do this when it is not explicitly configured. |
||
# use the default root path from the app, which seems to also include the path | ||
# if we are mounted under a path | ||
root_path = root_path_asgi | ||
|
@@ -425,44 +423,16 @@ async def root(request: Request, fullpath: str = ""): | |
if script_name: | ||
logger.debug("override root_path using x-script-name header from %s to %s", root_path, script_name) | ||
root_path = script_name | ||
root_path = root_path.rstrip("/") | ||
settings.main.root_path = root_path | ||
|
||
# lets be flexible about the trailing slash | ||
# TODO: maybe we should be more strict about the trailing slash | ||
naked_root_path = settings.main.root_path.rstrip("/") | ||
naked_base_url = settings.main.base_url.rstrip("/") | ||
if not naked_base_url.endswith(naked_root_path): | ||
msg = f"""base url {naked_base_url!r} does not end with root path {naked_root_path!r} | ||
|
||
This could be a configuration mismatch behind a reverse proxy and can cause issues with redirect urls, and auth. | ||
|
||
See also https://solara.dev/documentation/getting_started/deploying/self-hosted | ||
""" | ||
if "script-name" in request.headers: | ||
msg += f"""It looks like the reverse proxy sets the script-name header to {request.headers["script-name"]!r} | ||
""" | ||
if "x-script-name" in request.headers: | ||
msg += f"""It looks like the reverse proxy sets the x-script-name header to {request.headers["x-script-name"]!r} | ||
""" | ||
if configured_root_path: | ||
msg += f"""It looks like the root path was configured to {configured_root_path!r} in the settings | ||
""" | ||
if root_path_asgi: | ||
msg += f"""It looks like the root path set by the asgi framework was configured to {root_path_asgi!r} | ||
""" | ||
warnings.warn(msg) | ||
if host and forwarded_host and forwarded_proto: | ||
port = request.base_url.port | ||
ports = {"http": 80, "https": 443} | ||
expected_origin = f"{forwarded_proto}://{forwarded_host}" | ||
if port and port != ports[forwarded_proto]: | ||
expected_origin += f":{port}" | ||
starlette_origin = settings.main.base_url | ||
# strip off trailing / because we compare to the naked root path | ||
starlette_origin = starlette_origin.rstrip("/") | ||
if naked_root_path: | ||
# take off the root path | ||
starlette_origin = starlette_origin[: -len(naked_root_path)] | ||
starlette_origin = settings.main.origin | ||
if starlette_origin != expected_origin: | ||
warnings.warn(f"""Origin as determined by starlette ({starlette_origin!r}) does not match expected origin ({expected_origin!r}) based on x-forwarded-proto ({forwarded_proto!r}) and x-forwarded-host ({forwarded_host!r}) headers. | ||
|
||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used the None option to signal it's not configured, which then in starlette.py will recognize it (put a comment there where we do that)