You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by ka...@apache.org on 2021/03/19 22:02:30 UTC
[airflow] 02/04: Fix tests for all urllib versions with only '&' as
separator (#14710)
This is an automated email from the ASF dual-hosted git repository.
kaxilnaik pushed a commit to branch v2-0-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 44b65e734f71632fa1158faeb78f4893e6a05f5a
Author: Kaxil Naik <ka...@gmail.com>
AuthorDate: Thu Mar 11 01:39:28 2021 +0000
Fix tests for all urllib versions with only '&' as separator (#14710)
Turns out #14698 did not fix the issue as Master failed again. After
digging a bit more I found that the CVE was fixed in all
Python versions: 3.6.13, 3.7.10 & 3.8.8
The solution in this PR/commit checks the `parse_qsl` behavior with
following tests:
```
❯ docker run -it python:3.8-slim bash
root@41120dfd035e:/# python
Python 3.8.8 (default, Feb 19 2021, 18:07:06)
>>> from urllib.parse import parse_qsl
>>> parse_qsl(";a=b")
[(';a', 'b')]
>>>
```
```
❯ docker run -it python:3.8.7-slim bash
root@68e527725610:/# python
Python 3.8.7 (default, Feb 9 2021, 08:21:15)
>>> from urllib.parse import parse_qsl
>>> parse_qsl(";a=b")
[('a', 'b')]
>>>
```
(cherry picked from commit 7bd9d477dd7c59b8efb7183050de58bcfd6fdd43)
---
tests/www/test_views.py | 88 ++++++++++++-------------------------------------
1 file changed, 21 insertions(+), 67 deletions(-)
diff --git a/tests/www/test_views.py b/tests/www/test_views.py
index 0d50144..ce4478c 100644
--- a/tests/www/test_views.py
+++ b/tests/www/test_views.py
@@ -32,7 +32,7 @@ from datetime import datetime as dt, timedelta
from typing import Any, Dict, Generator, List, NamedTuple
from unittest import mock
from unittest.mock import PropertyMock
-from urllib.parse import quote_plus
+from urllib.parse import parse_qsl, quote_plus
import jinja2
import pytest
@@ -2757,46 +2757,22 @@ class TestTriggerDag(TestBase):
("http://google.com", "/home"),
(
"%2Ftree%3Fdag_id%3Dexample_bash_operator';alert(33)//",
- "/tree?dag_id=example_bash_operator%27&alert%2833%29%2F%2F=",
- ),
- ("%2Ftree%3Fdag_id%3Dexample_bash_operator", "/tree?dag_id=example_bash_operator"),
- ("%2Fgraph%3Fdag_id%3Dexample_bash_operator", "/graph?dag_id=example_bash_operator"),
- ]
- )
- @pytest.mark.skipif(
- sys.version_info < (3, 8, 8),
- reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
- )
- def test_trigger_dag_form_origin_url_py_lte_387(self, test_origin, expected_origin):
- test_dag_id = "example_bash_operator"
-
- resp = self.client.get(f'trigger?dag_id={test_dag_id}&origin={test_origin}')
- self.check_content_in_response(
- '<button type="button" class="btn" onclick="location.href = \'{}\'; return false">'.format(
- expected_origin
- ),
- resp,
- )
-
- @parameterized.expand(
- [
- ("javascript:alert(1)", "/home"),
- ("http://google.com", "/home"),
- (
- "%2Ftree%3Fdag_id%3Dexample_bash_operator';alert(33)//",
"/tree?dag_id=example_bash_operator%27%3Balert%2833%29%2F%2F",
),
("%2Ftree%3Fdag_id%3Dexample_bash_operator", "/tree?dag_id=example_bash_operator"),
("%2Fgraph%3Fdag_id%3Dexample_bash_operator", "/graph?dag_id=example_bash_operator"),
]
)
- @pytest.mark.skipif(
- sys.version_info > (3, 8, 7),
- reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
- )
- def test_trigger_dag_form_origin_url_py_gt_387(self, test_origin, expected_origin):
+ def test_trigger_dag_form_origin_url(self, test_origin, expected_origin):
test_dag_id = "example_bash_operator"
+ # https://github.com/python/cpython/pull/24297/files
+ # Check if tests are running with a Python version containing the above fix
+ # where ";" is removed as a separator
+ if parse_qsl(";a=b") != [(';a', 'b')]:
+ expected_url = expected_origin.replace("%3B", "&")
+ expected_url += "="
+
resp = self.client.get(f'trigger?dag_id={test_dag_id}&origin={test_origin}')
self.check_content_in_response(
'<button type="button" class="btn" onclick="location.href = \'{}\'; return false">'.format(
@@ -3329,31 +3305,6 @@ class TestHelperFunctions(TestBase):
(
"http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag';alert(33)//",
"http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3F"
- "dag_id%25test_dag%27&alert%2833%29%2F%2F=",
- ),
- (
- "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag",
- "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%25test_dag",
- ),
- ]
- )
- @pytest.mark.skipif(
- sys.version_info < (3, 8, 8),
- reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
- )
- def test_get_safe_url_py_lte_387(self, test_url, expected_url):
- with mock.patch("airflow.www.views.url_for") as mock_url_for:
- mock_url_for.return_value = "/home"
- with self.app.test_request_context(base_url="http://localhost:8080"):
- assert get_safe_url(test_url) == expected_url
-
- @parameterized.expand(
- [
- ("", "/home"),
- ("http://google.com", "/home"),
- (
- "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag';alert(33)//",
- "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3F"
"dag_id%25test_dag%27%3Balert%2833%29%2F%2F",
),
(
@@ -3362,15 +3313,18 @@ class TestHelperFunctions(TestBase):
),
]
)
- @pytest.mark.skipif(
- sys.version_info > (3, 8, 7),
- reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
- )
- def test_get_safe_url_py_gt_387(self, test_url, expected_url):
- with mock.patch("airflow.www.views.url_for") as mock_url_for:
- mock_url_for.return_value = "/home"
- with self.app.test_request_context(base_url="http://localhost:8080"):
- assert get_safe_url(test_url) == expected_url
+ @mock.patch("airflow.www.views.url_for")
+ def test_get_safe_url(self, test_url, expected_url, mock_url_for):
+ # https://github.com/python/cpython/pull/24297/files
+ # Check if tests are running with a Python version containing the above fix
+ # where ";" is removed as a separator
+ if parse_qsl(";a=b") != [(';a', 'b')]:
+ expected_url = expected_url.replace("%3B", "&")
+ expected_url += "="
+
+ mock_url_for.return_value = "/home"
+ with self.app.test_request_context(base_url="http://localhost:8080"):
+ assert get_safe_url(test_url) == expected_url
@parameterized.expand(
[