uranusjr commented on code in PR #31833:
URL: https://github.com/apache/airflow/pull/31833#discussion_r1282696916
##########
airflow/www/extensions/init_wsgi_middlewares.py:
##########
@@ -37,8 +38,11 @@ def _root_app(env: WSGIEnvironment, resp: StartResponse) ->
Iterable[bytes]:
def init_wsgi_middleware(flask_app: Flask) -> None:
"""Handle X-Forwarded-* headers and base_url support."""
+ webserver_base_url = conf.get_mandatory_value("webserver", "BASE_URL",
fallback="")
+ if webserver_base_url.endswith("/"):
+ raise AirflowConfigException("webserver.base_url conf cannot have a
trailing slash.")
Review Comment:
(Sorry, forgot to reply.)
Python’s `urljoin` implements the logic in `<a href="...">` tags; I don’t
recall the correct term for this, but it is the URL resolution logic when you
click on links in a browser. URLs have two kinds, folder and file (not the
right terms, but easier to understand). The URL would not have a trailing slash
if you are in a file view; the trailing slash indicates a folder view. When
you’re in a file, a relative path like `foo` indicates another file in the same
folder. When you’re in a folder view, however, `foo` means the foo entry _in_
the folder. This means the resolution logic would change depending on whether
the URL has a trailing slash or not.
Note that Python’s local path joining logic (both pathlib and os.path) is
different and does not consider the trailing slash; it instead matches the
common logic used to join format in shell scripts.
But the prefix is an entirely different thing, and is intended to only be
joined with simple string concatenation and skips all the folder/file logic
because it is not semantically possible to support combining the prefix with
absolute paths and relative paths. The only proper way to prepend a prefix is
`prefix + path` (or other string concatenation methods such as
`f"{prefix}{path}"`, of course).
##########
airflow/www/extensions/init_wsgi_middlewares.py:
##########
@@ -37,8 +38,11 @@ def _root_app(env: WSGIEnvironment, resp: StartResponse) ->
Iterable[bytes]:
def init_wsgi_middleware(flask_app: Flask) -> None:
"""Handle X-Forwarded-* headers and base_url support."""
+ webserver_base_url = conf.get_mandatory_value("webserver", "BASE_URL",
fallback="")
+ if webserver_base_url.endswith("/"):
+ raise AirflowConfigException("webserver.base_url conf cannot have a
trailing slash.")
Review Comment:
(Sorry, forgot to reply.)
Python’s `urljoin` implements the logic in `<a href="...">` tags; I don’t
recall the correct term for this, but it is the URL resolution logic when you
click on links in a browser. URLs have two kinds, folder and file (not the
right terms, but easier to understand). The URL would not have a trailing slash
if you are in a file view; the trailing slash indicates a folder view. When
you’re in a file, a relative path like `foo` indicates another file in the same
folder. When you’re in a folder view, however, `foo` means the foo entry _in_
the folder. This means the resolution logic would change depending on whether
the URL has a trailing slash or not.
Note that Python’s local path joining logic (both pathlib and os.path) is
different and does not consider the trailing slash; it instead matches the
common logic used to join format in shell scripts.
But the prefix is an entirely different thing, and is intended to only be
joined with simple string concatenation and skips all the folder/file logic
because it is not semantically possible to support combining the prefix with
absolute paths and relative paths. The only proper way to prepend a prefix is
`prefix + path` (or other string concatenation methods such as
`f"{prefix}{path}"`, of course).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
