diff --git a/airflow/models/taskinstance.py b/airflow/models/taskinstance.py index 28dc168ec3a25..f429e27e1546f 100644 --- a/airflow/models/taskinstance.py +++ b/airflow/models/taskinstance.py @@ -33,7 +33,7 @@ from pathlib import PurePath from types import TracebackType from typing import TYPE_CHECKING, Any, Callable, Collection, Generator, Iterable, Tuple -from urllib.parse import quote +from urllib.parse import quote, urljoin import dill import jinja2 @@ -759,26 +759,26 @@ def log_url(self) -> str: """Log URL for TaskInstance.""" iso = quote(self.execution_date.isoformat()) base_url = conf.get_mandatory_value("webserver", "BASE_URL") - return ( - f"{base_url}/log" - f"?execution_date={iso}" + return urljoin( + base_url, + f"log?execution_date={iso}" f"&task_id={self.task_id}" f"&dag_id={self.dag_id}" - f"&map_index={self.map_index}" + f"&map_index={self.map_index}", ) @property def mark_success_url(self) -> str: """URL to mark TI success.""" base_url = conf.get_mandatory_value("webserver", "BASE_URL") - return ( - f"{base_url}/confirm" - f"?task_id={self.task_id}" + return urljoin( + base_url, + f"confirm?task_id={self.task_id}" f"&dag_id={self.dag_id}" f"&dag_run_id={quote(self.run_id)}" "&upstream=false" "&downstream=false" - "&state=success" + "&state=success", ) @provide_session diff --git a/airflow/www/extensions/init_wsgi_middlewares.py b/airflow/www/extensions/init_wsgi_middlewares.py index 3ea47e92c86f6..37d4a074b4d18 100644 --- a/airflow/www/extensions/init_wsgi_middlewares.py +++ b/airflow/www/extensions/init_wsgi_middlewares.py @@ -25,6 +25,7 @@ from werkzeug.middleware.proxy_fix import ProxyFix from airflow.configuration import conf +from airflow.exceptions import AirflowConfigException if TYPE_CHECKING: from _typeshed.wsgi import StartResponse, WSGIEnvironment @@ -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.") # Apply DispatcherMiddleware - base_url = urlsplit(conf.get("webserver", "base_url"))[2] + base_url = urlsplit(webserver_base_url)[2] if not base_url or base_url == "/": base_url = "" if base_url: diff --git a/tests/www/test_app.py b/tests/www/test_app.py index 61bfae3e12a87..8dd3b57b2e394 100644 --- a/tests/www/test_app.py +++ b/tests/www/test_app.py @@ -18,6 +18,7 @@ from __future__ import annotations import hashlib +import re import runpy import sys from datetime import timedelta @@ -86,15 +87,26 @@ def debug_view(): assert b"success" == response.get_data() assert response.status_code == 200 - @conf_vars( - { - ("webserver", "base_url"): "http://localhost:8080/internal-client", - } + @pytest.mark.parametrize( + "base_url, expected_exception", + [ + ("http://localhost:8080/internal-client", None), + ( + "http://localhost:8080/internal-client/", + AirflowConfigException("webserver.base_url conf cannot have a trailing slash."), + ), + ], ) @dont_initialize_flask_app_submodules - def test_should_respect_base_url_ignore_proxy_headers(self): - app = application.cached_app(testing=True) - app.url_map.add(Rule("/debug", endpoint="debug")) + def test_should_respect_base_url_ignore_proxy_headers(self, base_url, expected_exception): + with conf_vars({("webserver", "base_url"): base_url}): + if expected_exception: + with pytest.raises(expected_exception.__class__, match=re.escape(str(expected_exception))): + app = application.cached_app(testing=True) + app.url_map.add(Rule("/debug", endpoint="debug")) + return + app = application.cached_app(testing=True) + app.url_map.add(Rule("/debug", endpoint="debug")) def debug_view(): from flask import request @@ -126,9 +138,18 @@ def debug_view(): assert b"success" == response.get_data() assert response.status_code == 200 + @pytest.mark.parametrize( + "base_url, expected_exception", + [ + ("http://localhost:8080/internal-client", None), + ( + "http://localhost:8080/internal-client/", + AirflowConfigException("webserver.base_url conf cannot have a trailing slash."), + ), + ], + ) @conf_vars( { - ("webserver", "base_url"): "http://localhost:8080/internal-client", ("webserver", "enable_proxy_fix"): "True", ("webserver", "proxy_fix_x_for"): "1", ("webserver", "proxy_fix_x_proto"): "1", @@ -138,9 +159,17 @@ def debug_view(): } ) @dont_initialize_flask_app_submodules - def test_should_respect_base_url_when_proxy_fix_and_base_url_is_set_up_but_headers_missing(self): - app = application.cached_app(testing=True) - app.url_map.add(Rule("/debug", endpoint="debug")) + def test_should_respect_base_url_when_proxy_fix_and_base_url_is_set_up_but_headers_missing( + self, base_url, expected_exception + ): + with conf_vars({("webserver", "base_url"): base_url}): + if expected_exception: + with pytest.raises(expected_exception.__class__, match=re.escape(str(expected_exception))): + app = application.cached_app(testing=True) + app.url_map.add(Rule("/debug", endpoint="debug")) + return + app = application.cached_app(testing=True) + app.url_map.add(Rule("/debug", endpoint="debug")) def debug_view(): from flask import request