Merge pull request #860 from christianlouis/fix-httpx-asyncclient-syntax-error-15225710494108486471

Fix SyntaxError caused by duplicate `event_hooks` in `httpx.AsyncClient` instantiation

app/api/url_upload.py

@@ -28,6 +28,10 @@ logger = logging.getLogger(__name__)
 router = APIRouter()
 
 
+class UnsafeRedirectError(httpx.RequestError):
+    """Raised when a redirect target fails URL safety checks."""
+
+
 class URLUploadRequest(BaseModel):
     """Request model for URL-based file upload"""
 
@@ -120,9 +124,10 @@ async def verify_redirect(response: httpx.Response) -> None:
             try:
                 validate_url_safety(new_url)
             except HTTPException as e:
-                # Map the validation error to an httpx exception so it can be handled
-                # properly by the caller, avoiding raw HTTPExceptions escaping the client scope
-                raise httpx.RequestError(f"Redirect to unsafe URL blocked: {e.detail}", request=response.request) from e
+                raise UnsafeRedirectError(
+                    f"Redirect to unsafe URL blocked: {e.detail}",
+                    request=response.request,
+                ) from e
 
 
 @router.post("/process-url")
@@ -170,19 +175,6 @@ async def process_url(
     if not safe_filename:
         safe_filename = "download"
 
-    # Hook to validate redirects and prevent SSRF
-    async def validate_redirect(response: httpx.Response):
-        if response.is_redirect:
-            location = response.headers.get("Location")
-            if location:
-                # Resolve relative URLs
-                next_url = urllib.parse.urljoin(str(response.url), location)
-                try:
-                    validate_url_safety(next_url)
-                except HTTPException as e:
-                    # Reraise as a RequestError so httpx aborts the request
-                    raise httpx.RequestError(f"Unsafe redirect target: {e.detail}", request=response.request)
-
     # Download file with security measures
     # Initialize target_path to None to prevent UnboundLocalError in exception handlers
     # that may execute before target_path is assigned during error cases
@@ -194,11 +186,10 @@ async def process_url(
         async with httpx.AsyncClient(
             timeout=settings.http_request_timeout,
             follow_redirects=True,
-            event_hooks={"response": [validate_redirect]},
+            event_hooks={"response": [verify_redirect]},
             headers={
                 "User-Agent": "DocuElevate/1.0",  # Identify ourselves
             },
-            event_hooks={"response": [verify_redirect]},
         ) as client:
             async with client.stream("GET", url) as response:
                 response.raise_for_status()
@@ -285,6 +276,10 @@ async def process_url(
         logger.error(f"HTTP error while downloading file from URL: {url} - {str(e)}")
         raise HTTPException(status_code=e.response.status_code, detail=f"HTTP error: {str(e)}")
 
+    except UnsafeRedirectError as e:
+        logger.warning(f"Unsafe redirect blocked while downloading file from URL: {url} - {str(e)}")
+        raise HTTPException(status_code=400, detail=str(e))
+
     except httpx.RequestError as e:
         logger.error(f"Error downloading file from URL: {url} - {str(e)}")
         raise HTTPException(status_code=500, detail=f"Failed to download file: {str(e)}")

tests/test_url_upload.py

@@ -465,6 +465,19 @@ class TestURLUploadEndpoint:
         data = response.json()
         assert "Failed to download file" in data["detail"]
 
+    @patch("app.api.url_upload.httpx.AsyncClient.stream")
+    def test_process_url_unsafe_redirect_returns_400(self, mock_stream, client):
+        """Test unsafe redirects are reported as a client error instead of HTTP 500."""
+        from app.api.url_upload import UnsafeRedirectError
+
+        mock_stream.side_effect = UnsafeRedirectError("Redirect to unsafe URL blocked: Unsafe URL")
+
+        response = client.post("/api/process-url", json={"url": "https://example.com/file.pdf"})
+
+        assert response.status_code == 400
+        data = response.json()
+        assert "Redirect to unsafe URL blocked" in data["detail"]
+
     @patch("app.api.url_upload.httpx.AsyncClient.stream")
     def test_process_url_oserror_during_save(self, mock_stream, client, tmp_path, monkeypatch):
         """Test handling of OSError when saving file"""