Cleanup dashboard test and header validation

python/ray/dashboard/http_server_head.py

@@ -305,12 +305,9 @@ async def browsers_no_post_put_middleware(request, handler):
                 return await handler(request)
 
             if (
-                # A best effort test for browser traffic. All common browsers
-                # start with Mozilla at the time of writing.
-                (
-                    dashboard_optional_utils.is_browser_request(request)
-                    or dashboard_optional_utils.has_sec_fetch_headers(request)
-                )
+                # Deny mutating requests from browsers.
+                # See `is_browser_request` for details of the check.
+                dashboard_optional_utils.is_browser_request(request)
                 and request.method in [hdrs.METH_POST, hdrs.METH_PUT]
             ):
                 return aiohttp.web.Response(

python/ray/dashboard/optional_utils.py

@@ -128,19 +128,17 @@ def _update_cache(task):
 
 
 def is_browser_request(req: Request) -> bool:
-    """Checks if a request is made by a browser like user agent.
-
-    This heuristic is very weak, but hard for a browser to bypass- eg,
-    fetch/xhr and friends cannot alter the user-agent, but requests made with
-    an http library can stumble into this if they choose to user a browser like
-    user agent.
+    """Best-effort detection if the request was made by a browser.
+
+    Uses two heuristics:
+        1) If the `User-Agent` header starts with 'Mozilla'. This heuristic is weak,
+        but hard for a browser to bypass e.g., fetch/xhr and friends cannot alter the
+        user agent, but requests made with an HTTP library can stumble into this if
+        they choose to user a browser-like user agent. At the time of writing, all
+        common browsers' user agents start with 'Mozilla'.
+        2) If any of the `Sec-Fetch-*` headers are present.
     """
-    return req.headers["User-Agent"].startswith("Mozilla")
-
-
-def has_sec_fetch_headers(req: Request) -> bool:
-    """Checks for the existance of any of the sec-fetch-* headers"""
-    return any(
+    return req.headers.get("User-Agent", "").startswith("Mozilla") or any(
         h in req.headers
         for h in (
             "Sec-Fetch-Mode",
@@ -152,21 +150,17 @@ def has_sec_fetch_headers(req: Request) -> bool:
 
 
 def deny_browser_requests() -> Callable:
-    """Reject any requests that appear to be made by a browser"""
+    """Reject any requests that appear to be made by a browser."""
 
     def decorator_factory(f: Callable) -> Callable:
         @functools.wraps(f)
         async def decorator(self, req: Request):
-            if has_sec_fetch_headers(req):
-                return Response(
-                    text="Browser requests not allowed",
-                    status=aiohttp.web.HTTPMethodNotAllowed.status_code,
-                )
             if is_browser_request(req):
                 return Response(
                     text="Browser requests not allowed",
                     status=aiohttp.web.HTTPMethodNotAllowed.status_code,
                 )
+
             return await f(self, req)
 
         return decorator

python/ray/dashboard/tests/test_dashboard.py

@@ -425,43 +425,32 @@ def dashboard_available():
         except Exception:
             return False
 
-    timeout_seconds = 30
-    start_time = time.time()
     wait_for_condition(dashboard_available)
-    while True:
-        try:
-            # Starting and getting jobs should be fine from API clients
-            response = requests.post(
-                webui_url + "/api/jobs/", json={"entrypoint": "ls"}
-            )
-            response.raise_for_status()
-            response = requests.get(webui_url + "/api/jobs/")
-            response.raise_for_status()
 
-            # Starting job should be blocked for browsers
-            response = requests.post(
-                webui_url + "/api/jobs/",
-                json={"entrypoint": "ls"},
-                headers={
-                    "User-Agent": (
-                        "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) "
-                        "AppleWebKit/537.36 (KHTML, like Gecko) "
-                        "Chrome/119.0.0.0 Safari/537.36"
-                    )
-                },
+    # Starting and getting jobs should be fine from API clients
+    response = requests.post(webui_url + "/api/jobs/", json={"entrypoint": "ls"})
+    response.raise_for_status()
+    response = requests.get(webui_url + "/api/jobs/")
+    response.raise_for_status()
+
+    # Starting job should be blocked for browsers
+    response = requests.post(
+        webui_url + "/api/jobs/",
+        json={"entrypoint": "ls"},
+        headers={
+            "User-Agent": (
+                "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) "
+                "AppleWebKit/537.36 (KHTML, like Gecko) "
+                "Chrome/119.0.0.0 Safari/537.36"
             )
-            with pytest.raises(HTTPError):
-                response.raise_for_status()
+        },
+    )
+    with pytest.raises(HTTPError):
+        response.raise_for_status()
 
-            # Getting jobs should be fine for browsers
-            response = requests.get(webui_url + "/api/jobs/")
-            response.raise_for_status()
-            break
-        except (AssertionError, requests.exceptions.ConnectionError) as e:
-            logger.info("Retry because of %s", e)
-        finally:
-            if time.time() > start_time + timeout_seconds:
-                raise Exception("Timed out while testing.")
+    # Getting jobs should be fine for browsers
+    response = requests.get(webui_url + "/api/jobs/")
+    response.raise_for_status()
 
 
 @pytest.mark.skipif(
@@ -479,40 +468,29 @@ def dashboard_available():
         except Exception:
             return False
 
-    timeout_seconds = 30
-    start_time = time.time()
     wait_for_condition(dashboard_available)
-    while True:
-        try:
-            # Starting and getting jobs should be fine from API clients
-            response = requests.post(
-                webui_url + "/api/jobs/", json={"entrypoint": "ls"}
-            )
-            response.raise_for_status()
-            response = requests.get(webui_url + "/api/jobs/")
-            response.raise_for_status()
 
-            # Starting job should be blocked for browsers
-            response = requests.post(
-                webui_url + "/api/jobs/",
-                json={"entrypoint": "ls"},
-                headers={
-                    "User-Agent": ("Spurious User Agent"),
-                    "Sec-Fetch-Site": ("cross-site"),
-                },
-            )
-            with pytest.raises(HTTPError):
-                response.raise_for_status()
+    # Starting and getting jobs should be fine from API clients
+    response = requests.post(webui_url + "/api/jobs/", json={"entrypoint": "ls"})
+    response.raise_for_status()
+    response = requests.get(webui_url + "/api/jobs/")
+    response.raise_for_status()
 
-            # Getting jobs should be fine for browsers
-            response = requests.get(webui_url + "/api/jobs/")
-            response.raise_for_status()
-            break
-        except (AssertionError, requests.exceptions.ConnectionError) as e:
-            logger.info("Retry because of %s", e)
-        finally:
-            if time.time() > start_time + timeout_seconds:
-                raise Exception("Timed out while testing.")
+    # Starting job should be blocked for browsers
+    response = requests.post(
+        webui_url + "/api/jobs/",
+        json={"entrypoint": "ls"},
+        headers={
+            "User-Agent": ("Spurious User Agent"),
+            "Sec-Fetch-Site": ("cross-site"),
+        },
+    )
+    with pytest.raises(HTTPError):
+        response.raise_for_status()
+
+    # Getting jobs should be fine for browsers
+    response = requests.get(webui_url + "/api/jobs/")
+    response.raise_for_status()
 
 
 @pytest.mark.skipif(