[GitHub] [superset] villebro commented on a diff in pull request #22038: fix: datasource save, improve data validation

[GitBox <gi...@apache.org>]

villebro commented on code in PR #22038:
URL: https://github.com/apache/superset/pull/22038#discussion_r1014603466


##########
tests/unit_tests/utils/urls_tests.py:
##########
@@ -33,3 +35,24 @@ def test_convert_chart_link() -> None:
 def test_convert_dashboard_link() -> None:
     test_url = modify_url_query(EXPLORE_DASHBOARD_LINK, standalone="0")
     assert test_url == "http://localhost:9000/superset/dashboard/3/?standalone=0"
+
+
+@pytest.mark.parametrize(
+    "url,is_safe",
+    [
+        ("http://localhost/", True),
+        ("http://localhost/superset/1", True),
+        ("localhost/superset/1", False),
+        ("ftp://localhost/superset/1", False),
+        ("http://external.com", False),
+        ("external.com", False),
+        ("///localhost", False),
+        ("xpto://localhost:[3/1/", False),
+    ],

Review Comment:
   Could we also add an internal and external `https://` here?



##########
tests/integration_tests/datasource_tests.py:
##########
@@ -297,6 +297,44 @@ def test_save(self):
                 print(k)
                 self.assertEqual(resp[k], datasource_post[k])
 
+    def test_save_default_endpoint_validation_fail(self):
+        self.login(username="admin")
+        tbl_id = self.get_table(name="birth_names").id
+
+        datasource_post = get_datasource_post()
+        datasource_post["id"] = tbl_id
+        datasource_post["owners"] = [1]
+        datasource_post["default_endpoint"] = "http://www.google.com"
+        data = dict(data=json.dumps(datasource_post))
+        resp = self.client.post("/datasource/save/", data=data)
+        assert resp.status_code == 500

Review Comment:
   Should we really be expecting 500? This may just be me, but when I see 500, I immediately think "the system borked". Could 422 work better here?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org