Skip to content

Commit fe4a6c8

Browse files
hussein-awalauranusjr
hussein-awala
and
uranusjr
authored
Add a check for trailing slash in webserver base_url (#31833)
* Remove right trailing / from webserver base_url Signed-off-by: Hussein Awala <[email protected]> * use url join instead of removing trailing slash Signed-off-by: Hussein Awala <[email protected]> * raise an exception when base_url contains a trailing slash Signed-off-by: Hussein Awala <[email protected]> * Update airflow/www/extensions/init_wsgi_middlewares.py Co-authored-by: Tzu-ping Chung <[email protected]> --------- Signed-off-by: Hussein Awala <[email protected]> Co-authored-by: Tzu-ping Chung <[email protected]>
1 parent 69bc90b commit fe4a6c8

3 files changed

Lines changed: 54 additions & 21 deletions

File tree

‎airflow/models/taskinstance.py‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
from pathlib import PurePath
3434
from types import TracebackType
3535
from typing import TYPE_CHECKING, Any, Callable, Collection, Generator, Iterable, Tuple
36-
from urllib.parse import quote
36+
from urllib.parse import quote, urljoin
3737

3838
import dill
3939
import jinja2
@@ -759,26 +759,26 @@ def log_url(self) -> str:
759759
"""Log URL for TaskInstance."""
760760
iso = quote(self.execution_date.isoformat())
761761
base_url = conf.get_mandatory_value("webserver", "BASE_URL")
762-
return (
763-
f"{base_url}/log"
764-
f"?execution_date={iso}"
762+
return urljoin(
763+
base_url,
764+
f"log?execution_date={iso}"
765765
f"&task_id={self.task_id}"
766766
f"&dag_id={self.dag_id}"
767-
f"&map_index={self.map_index}"
767+
f"&map_index={self.map_index}",
768768
)
769769

770770
@property
771771
def mark_success_url(self) -> str:
772772
"""URL to mark TI success."""
773773
base_url = conf.get_mandatory_value("webserver", "BASE_URL")
774-
return (
775-
f"{base_url}/confirm"
776-
f"?task_id={self.task_id}"
774+
return urljoin(
775+
base_url,
776+
f"confirm?task_id={self.task_id}"
777777
f"&dag_id={self.dag_id}"
778778
f"&dag_run_id={quote(self.run_id)}"
779779
"&upstream=false"
780780
"&downstream=false"
781-
"&state=success"
781+
"&state=success",
782782
)
783783

784784
@provide_session

‎airflow/www/extensions/init_wsgi_middlewares.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
from werkzeug.middleware.proxy_fix import ProxyFix
2626

2727
from airflow.configuration import conf
28+
from airflow.exceptions import AirflowConfigException
2829

2930
if TYPE_CHECKING:
3031
from _typeshed.wsgi import StartResponse, WSGIEnvironment
@@ -37,8 +38,11 @@ def _root_app(env: WSGIEnvironment, resp: StartResponse) -> Iterable[bytes]:
3738

3839
def init_wsgi_middleware(flask_app: Flask) -> None:
3940
"""Handle X-Forwarded-* headers and base_url support."""
41+
webserver_base_url = conf.get_mandatory_value("webserver", "BASE_URL", fallback="")
42+
if webserver_base_url.endswith("/"):
43+
raise AirflowConfigException("webserver.base_url conf cannot have a trailing slash.")
4044
# Apply DispatcherMiddleware
41-
base_url = urlsplit(conf.get("webserver", "base_url"))[2]
45+
base_url = urlsplit(webserver_base_url)[2]
4246
if not base_url or base_url == "/":
4347
base_url = ""
4448
if base_url:

‎tests/www/test_app.py‎

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from __future__ import annotations
1919

2020
import hashlib
21+
import re
2122
import runpy
2223
import sys
2324
from datetime import timedelta
@@ -86,15 +87,26 @@ def debug_view():
8687
assert b"success" == response.get_data()
8788
assert response.status_code == 200
8889

89-
@conf_vars(
90-
{
91-
("webserver", "base_url"): "http://localhost:8080/internal-client",
92-
}
90+
@pytest.mark.parametrize(
91+
"base_url, expected_exception",
92+
[
93+
("http://localhost:8080/internal-client", None),
94+
(
95+
"http://localhost:8080/internal-client/",
96+
AirflowConfigException("webserver.base_url conf cannot have a trailing slash."),
97+
),
98+
],
9399
)
94100
@dont_initialize_flask_app_submodules
95-
def test_should_respect_base_url_ignore_proxy_headers(self):
96-
app = application.cached_app(testing=True)
97-
app.url_map.add(Rule("/debug", endpoint="debug"))
101+
def test_should_respect_base_url_ignore_proxy_headers(self, base_url, expected_exception):
102+
with conf_vars({("webserver", "base_url"): base_url}):
103+
if expected_exception:
104+
with pytest.raises(expected_exception.__class__, match=re.escape(str(expected_exception))):
105+
app = application.cached_app(testing=True)
106+
app.url_map.add(Rule("/debug", endpoint="debug"))
107+
return
108+
app = application.cached_app(testing=True)
109+
app.url_map.add(Rule("/debug", endpoint="debug"))
98110

99111
def debug_view():
100112
from flask import request
@@ -126,9 +138,18 @@ def debug_view():
126138
assert b"success" == response.get_data()
127139
assert response.status_code == 200
128140

141+
@pytest.mark.parametrize(
142+
"base_url, expected_exception",
143+
[
144+
("http://localhost:8080/internal-client", None),
145+
(
146+
"http://localhost:8080/internal-client/",
147+
AirflowConfigException("webserver.base_url conf cannot have a trailing slash."),
148+
),
149+
],
150+
)
129151
@conf_vars(
130152
{
131-
("webserver", "base_url"): "http://localhost:8080/internal-client",
132153
("webserver", "enable_proxy_fix"): "True",
133154
("webserver", "proxy_fix_x_for"): "1",
134155
("webserver", "proxy_fix_x_proto"): "1",
@@ -138,9 +159,17 @@ def debug_view():
138159
}
139160
)
140161
@dont_initialize_flask_app_submodules
141-
def test_should_respect_base_url_when_proxy_fix_and_base_url_is_set_up_but_headers_missing(self):
142-
app = application.cached_app(testing=True)
143-
app.url_map.add(Rule("/debug", endpoint="debug"))
162+
def test_should_respect_base_url_when_proxy_fix_and_base_url_is_set_up_but_headers_missing(
163+
self, base_url, expected_exception
164+
):
165+
with conf_vars({("webserver", "base_url"): base_url}):
166+
if expected_exception:
167+
with pytest.raises(expected_exception.__class__, match=re.escape(str(expected_exception))):
168+
app = application.cached_app(testing=True)
169+
app.url_map.add(Rule("/debug", endpoint="debug"))
170+
return
171+
app = application.cached_app(testing=True)
172+
app.url_map.add(Rule("/debug", endpoint="debug"))
144173

145174
def debug_view():
146175
from flask import request

0 commit comments

Comments
 (0)