Spaces:
Running
Bound the list inputs and the outbound fan-out they cause
Browse files`n_results` was defended by a validator and a seven-case parametrized test
while the list fields alongside it were undefended entirely. Two
consequences, both reachable from an unauthenticated request:
An empty query list crashed every search endpoint. `_shape_serp_results`
reads `results[-1]` to build its error message, so `{"queries": []}`
raised IndexError - an unhandled 500 on all six endpoints, from a
two-character body.
The bulk endpoints fanned out without any limit. `patent_ids` had no
maximum and both bulk helpers gathered the whole list at once, so 3000 ids
opened 3000 concurrent scrapes; the OPS variant is three times worse again,
since each id costs a biblio, a claims and a description request. The
Playwright scrapers have had a concurrency semaphore from the start - this
brings the HTTP paths in line with them.
- min_length/max_length on SerpQuery.queries and ScrapPatentsRequest.patent_ids
- semaphores in scrap_patent_bulk_async and ops_scrap_patent_bulk
- _shape_serp_results is now total for an empty list, so it doesn't depend
on its caller having validated anything
The concurrency tests assert the observed peak rather than the limit
constant, so they fail if the semaphore is removed rather than just
re-stating the configuration.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MSNSYnceqvdVz7Csis4K9e
- app.py +14 -0
- ops.py +12 -1
- scrap.py +15 -2
- serp.py +10 -0
- tests/test_input_bounds.py +148 -0
|
@@ -126,6 +126,12 @@ def _shape_serp_results(results: list) -> SerpResults:
|
|
| 126 |
`return_exceptions=True` gather) into one SerpResults, surfacing the
|
| 127 |
last error only when every query failed.
|
| 128 |
"""
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 129 |
filtered_results = [r for r in results if not isinstance(r, Exception)]
|
| 130 |
flattened_results = [
|
| 131 |
item for sublist in filtered_results for item in sublist]
|
|
@@ -307,9 +313,17 @@ async def scrap_patent(patent_id: PatentId) -> PatentScrapResult:
|
|
| 307 |
detail=f"EPO OPS request failed for '{patent_id}': {e}")
|
| 308 |
|
| 309 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 310 |
class ScrapPatentsRequest(BaseModel):
|
| 311 |
"""Request model for scrapping multiple patents."""
|
| 312 |
patent_ids: list[PatentId] = Field(...,
|
|
|
|
|
|
|
| 313 |
description="List of patent IDs to scrap")
|
| 314 |
|
| 315 |
|
|
|
|
| 126 |
`return_exceptions=True` gather) into one SerpResults, surfacing the
|
| 127 |
last error only when every query failed.
|
| 128 |
"""
|
| 129 |
+
if not results:
|
| 130 |
+
# SerpQuery bounds the query list, so this is unreachable from the
|
| 131 |
+
# endpoints; keep the helper total anyway rather than indexing into
|
| 132 |
+
# an empty list, since every search endpoint shares it.
|
| 133 |
+
return SerpResults(results=[], error="No queries were provided.")
|
| 134 |
+
|
| 135 |
filtered_results = [r for r in results if not isinstance(r, Exception)]
|
| 136 |
flattened_results = [
|
| 137 |
item for sublist in filtered_results for item in sublist]
|
|
|
|
| 313 |
detail=f"EPO OPS request failed for '{patent_id}': {e}")
|
| 314 |
|
| 315 |
|
| 316 |
+
# Same reasoning as MAX_QUERIES_PER_REQUEST in serp.py: one accepted
|
| 317 |
+
# request must not be able to queue unbounded outbound work. The OPS
|
| 318 |
+
# variant is the expensive one - three requests per id.
|
| 319 |
+
MAX_BULK_PATENT_IDS = 50
|
| 320 |
+
|
| 321 |
+
|
| 322 |
class ScrapPatentsRequest(BaseModel):
|
| 323 |
"""Request model for scrapping multiple patents."""
|
| 324 |
patent_ids: list[PatentId] = Field(...,
|
| 325 |
+
min_length=1,
|
| 326 |
+
max_length=MAX_BULK_PATENT_IDS,
|
| 327 |
description="List of patent IDs to scrap")
|
| 328 |
|
| 329 |
|
|
@@ -312,9 +312,20 @@ class OPSBulkResponse(BaseModel):
|
|
| 312 |
failed_ids: list[str]
|
| 313 |
|
| 314 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 315 |
async def ops_scrap_patent_bulk(client: AsyncClient, numbers: list[str]) -> OPSBulkResponse:
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 316 |
results = await asyncio.gather(
|
| 317 |
-
*[
|
| 318 |
patents = [r for r in results if not isinstance(r, Exception)]
|
| 319 |
failed = [numbers[i] for i, r in enumerate(
|
| 320 |
results) if isinstance(r, Exception)]
|
|
|
|
| 312 |
failed_ids: list[str]
|
| 313 |
|
| 314 |
|
| 315 |
+
# Lower than the Google Patents equivalent because each id here costs three
|
| 316 |
+
# OPS requests (biblio + claims + description) rather than one page fetch.
|
| 317 |
+
BULK_OPS_CONCURRENCY_LIMIT = 5
|
| 318 |
+
|
| 319 |
+
|
| 320 |
async def ops_scrap_patent_bulk(client: AsyncClient, numbers: list[str]) -> OPSBulkResponse:
|
| 321 |
+
semaphore = asyncio.Semaphore(BULK_OPS_CONCURRENCY_LIMIT)
|
| 322 |
+
|
| 323 |
+
async def scrap_one(number: str):
|
| 324 |
+
async with semaphore:
|
| 325 |
+
return await ops_scrap_patent(client, number)
|
| 326 |
+
|
| 327 |
results = await asyncio.gather(
|
| 328 |
+
*[scrap_one(n) for n in numbers], return_exceptions=True)
|
| 329 |
patents = [r for r in results if not isinstance(r, Exception)]
|
| 330 |
failed = [numbers[i] for i, r in enumerate(
|
| 331 |
results) if isinstance(r, Exception)]
|
|
@@ -131,11 +131,24 @@ class PatentScrapBulkResponse(BaseModel):
|
|
| 131 |
failed_ids: list[str]
|
| 132 |
|
| 133 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 134 |
async def scrap_patent_bulk_async(client: AsyncClient, patent_ids: list[str]) -> PatentScrapBulkResponse:
|
| 135 |
-
"""Scrape multiple patents asynchronously."""
|
| 136 |
urls = [
|
| 137 |
f"https://patents.google.com/patent/{pid}/en" for pid in patent_ids]
|
| 138 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 139 |
|
| 140 |
filtered_results = [
|
| 141 |
res for res in results if not isinstance(res, Exception)]
|
|
|
|
| 131 |
failed_ids: list[str]
|
| 132 |
|
| 133 |
|
| 134 |
+
# Cap how many patent pages are fetched at once. Without this a single
|
| 135 |
+
# accepted request opened one outbound scrape per id, all at the same time;
|
| 136 |
+
# the Playwright scrapers have had an equivalent limit from the start and
|
| 137 |
+
# this brings the HTTP path in line with them.
|
| 138 |
+
BULK_SCRAP_CONCURRENCY_LIMIT = 10
|
| 139 |
+
|
| 140 |
+
|
| 141 |
async def scrap_patent_bulk_async(client: AsyncClient, patent_ids: list[str]) -> PatentScrapBulkResponse:
|
| 142 |
+
"""Scrape multiple patents asynchronously, a bounded number at a time."""
|
| 143 |
urls = [
|
| 144 |
f"https://patents.google.com/patent/{pid}/en" for pid in patent_ids]
|
| 145 |
+
semaphore = asyncio.Semaphore(BULK_SCRAP_CONCURRENCY_LIMIT)
|
| 146 |
+
|
| 147 |
+
async def scrap_one(url: str):
|
| 148 |
+
async with semaphore:
|
| 149 |
+
return await scrap_patent_async(client, url)
|
| 150 |
+
|
| 151 |
+
results = await asyncio.gather(*[scrap_one(url) for url in urls], return_exceptions=True)
|
| 152 |
|
| 153 |
filtered_results = [
|
| 154 |
res for res in results if not isinstance(res, Exception)]
|
|
@@ -16,8 +16,18 @@ from asyncio import Semaphore
|
|
| 16 |
PLAYWRIGHT_CONCURRENCY_LIMIT = 10
|
| 17 |
|
| 18 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 19 |
class SerpQuery(BaseModel):
|
| 20 |
queries: list[str] = Field(...,
|
|
|
|
|
|
|
| 21 |
description="The list of queries to search for")
|
| 22 |
n_results: int = Field(
|
| 23 |
10, description="Number of results to return for each query. Valid values are 10, 25, 50 and 100")
|
|
|
|
| 16 |
PLAYWRIGHT_CONCURRENCY_LIMIT = 10
|
| 17 |
|
| 18 |
|
| 19 |
+
# One request must not be able to fan out without limit. Every search
|
| 20 |
+
# endpoint issues one outbound call per query (and `/serp/search` may try
|
| 21 |
+
# three backends for each), so the length of this list is a direct
|
| 22 |
+
# multiplier on outbound load - `n_results` was clamped from the start
|
| 23 |
+
# while the list holding the queries was not bounded at all.
|
| 24 |
+
MAX_QUERIES_PER_REQUEST = 50
|
| 25 |
+
|
| 26 |
+
|
| 27 |
class SerpQuery(BaseModel):
|
| 28 |
queries: list[str] = Field(...,
|
| 29 |
+
min_length=1,
|
| 30 |
+
max_length=MAX_QUERIES_PER_REQUEST,
|
| 31 |
description="The list of queries to search for")
|
| 32 |
n_results: int = Field(
|
| 33 |
10, description="Number of results to return for each query. Valid values are 10, 25, 50 and 100")
|
|
@@ -0,0 +1,148 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
"""Bounds on request-shaped input.
|
| 2 |
+
|
| 3 |
+
`n_results` was defended by a validator and a parametrized test while the
|
| 4 |
+
list fields on the same models were undefended entirely: an empty query
|
| 5 |
+
list crashed every search endpoint with an IndexError, and the bulk
|
| 6 |
+
endpoints would fan out one concurrent outbound request per id with no
|
| 7 |
+
upper limit at all.
|
| 8 |
+
"""
|
| 9 |
+
|
| 10 |
+
import httpx
|
| 11 |
+
import pytest
|
| 12 |
+
from httpx import ASGITransport
|
| 13 |
+
from pydantic import ValidationError
|
| 14 |
+
|
| 15 |
+
import app as app_module
|
| 16 |
+
import scrap as scrap_module
|
| 17 |
+
from app import MAX_BULK_PATENT_IDS, ScrapPatentsRequest
|
| 18 |
+
from serp import MAX_QUERIES_PER_REQUEST, SerpQuery
|
| 19 |
+
|
| 20 |
+
|
| 21 |
+
@pytest.fixture
|
| 22 |
+
async def client():
|
| 23 |
+
transport = ASGITransport(app=app_module.app)
|
| 24 |
+
async with httpx.AsyncClient(transport=transport, base_url="http://test") as c:
|
| 25 |
+
yield c
|
| 26 |
+
|
| 27 |
+
|
| 28 |
+
# --------------------------------- query list bounds ---------------------------------
|
| 29 |
+
|
| 30 |
+
|
| 31 |
+
def test_empty_query_list_is_rejected_by_the_model():
|
| 32 |
+
with pytest.raises(ValidationError):
|
| 33 |
+
SerpQuery(queries=[])
|
| 34 |
+
|
| 35 |
+
|
| 36 |
+
def test_too_many_queries_are_rejected_by_the_model():
|
| 37 |
+
with pytest.raises(ValidationError):
|
| 38 |
+
SerpQuery(queries=["q"] * (MAX_QUERIES_PER_REQUEST + 1))
|
| 39 |
+
|
| 40 |
+
|
| 41 |
+
async def test_empty_query_list_returns_422_not_500(client):
|
| 42 |
+
"""`_shape_serp_results` read `results[-1]` to build its error message,
|
| 43 |
+
so an empty list raised IndexError - an unhandled 500 on all six search
|
| 44 |
+
endpoints, reachable with a two-character request body.
|
| 45 |
+
"""
|
| 46 |
+
resp = await client.post("/serp/search_arxiv", json={"queries": []})
|
| 47 |
+
|
| 48 |
+
assert resp.status_code == 422
|
| 49 |
+
|
| 50 |
+
|
| 51 |
+
async def test_oversized_query_list_returns_422(client):
|
| 52 |
+
resp = await client.post(
|
| 53 |
+
"/serp/search_arxiv", json={"queries": ["q"] * (MAX_QUERIES_PER_REQUEST + 1)})
|
| 54 |
+
|
| 55 |
+
assert resp.status_code == 422
|
| 56 |
+
|
| 57 |
+
|
| 58 |
+
def test_shape_serp_results_is_total_for_an_empty_list():
|
| 59 |
+
"""Defence in depth: the helper must not depend on its caller having
|
| 60 |
+
validated the query list, since it is shared by every search endpoint.
|
| 61 |
+
"""
|
| 62 |
+
result = app_module._shape_serp_results([])
|
| 63 |
+
|
| 64 |
+
assert result.results == []
|
| 65 |
+
assert result.error is not None
|
| 66 |
+
|
| 67 |
+
|
| 68 |
+
# ---------------------------------- bulk id bounds ----------------------------------
|
| 69 |
+
|
| 70 |
+
|
| 71 |
+
def test_empty_patent_id_list_is_rejected():
|
| 72 |
+
with pytest.raises(ValidationError):
|
| 73 |
+
ScrapPatentsRequest(patent_ids=[])
|
| 74 |
+
|
| 75 |
+
|
| 76 |
+
def test_too_many_patent_ids_are_rejected():
|
| 77 |
+
with pytest.raises(ValidationError):
|
| 78 |
+
ScrapPatentsRequest(patent_ids=["US1234567"] * (MAX_BULK_PATENT_IDS + 1))
|
| 79 |
+
|
| 80 |
+
|
| 81 |
+
async def test_oversized_bulk_request_returns_422(client):
|
| 82 |
+
resp = await client.post(
|
| 83 |
+
"/scrap/scrap_patents_bulk",
|
| 84 |
+
json={"patent_ids": ["US1234567"] * (MAX_BULK_PATENT_IDS + 1)})
|
| 85 |
+
|
| 86 |
+
assert resp.status_code == 422
|
| 87 |
+
|
| 88 |
+
|
| 89 |
+
# ------------------------------- bulk fan-out is bounded -------------------------------
|
| 90 |
+
|
| 91 |
+
|
| 92 |
+
async def test_bulk_scrape_limits_concurrent_outbound_requests(monkeypatch):
|
| 93 |
+
"""Playwright work is bounded by a semaphore; HTTP work was not bounded
|
| 94 |
+
at all, so a single accepted request could open one outbound scrape per
|
| 95 |
+
id. Track how many are in flight simultaneously.
|
| 96 |
+
"""
|
| 97 |
+
import asyncio
|
| 98 |
+
|
| 99 |
+
in_flight = 0
|
| 100 |
+
peak = 0
|
| 101 |
+
|
| 102 |
+
async def slow_scrape(client_arg, url):
|
| 103 |
+
nonlocal in_flight, peak
|
| 104 |
+
in_flight += 1
|
| 105 |
+
peak = max(peak, in_flight)
|
| 106 |
+
await asyncio.sleep(0.01)
|
| 107 |
+
in_flight -= 1
|
| 108 |
+
raise httpx.ConnectTimeout("nope")
|
| 109 |
+
|
| 110 |
+
monkeypatch.setattr(scrap_module, "scrap_patent_async", slow_scrape)
|
| 111 |
+
|
| 112 |
+
ids = [f"US{1000000 + i}" for i in range(40)]
|
| 113 |
+
result = await scrap_module.scrap_patent_bulk_async(None, ids)
|
| 114 |
+
|
| 115 |
+
assert peak <= scrap_module.BULK_SCRAP_CONCURRENCY_LIMIT, (
|
| 116 |
+
f"{peak} concurrent scrapes for {len(ids)} ids; "
|
| 117 |
+
f"limit is {scrap_module.BULK_SCRAP_CONCURRENCY_LIMIT}")
|
| 118 |
+
assert result.failed_ids == ids
|
| 119 |
+
|
| 120 |
+
|
| 121 |
+
async def test_bulk_ops_retrieval_limits_concurrent_requests(monkeypatch):
|
| 122 |
+
"""The OPS bulk path is the expensive one - three requests per id
|
| 123 |
+
(biblio, claims, description) - so it carries a tighter limit.
|
| 124 |
+
"""
|
| 125 |
+
import asyncio
|
| 126 |
+
|
| 127 |
+
import ops as ops_module
|
| 128 |
+
|
| 129 |
+
in_flight = 0
|
| 130 |
+
peak = 0
|
| 131 |
+
|
| 132 |
+
async def slow_ops_scrap(client_arg, number, *args, **kwargs):
|
| 133 |
+
nonlocal in_flight, peak
|
| 134 |
+
in_flight += 1
|
| 135 |
+
peak = max(peak, in_flight)
|
| 136 |
+
await asyncio.sleep(0.01)
|
| 137 |
+
in_flight -= 1
|
| 138 |
+
raise RuntimeError("nope")
|
| 139 |
+
|
| 140 |
+
monkeypatch.setattr(ops_module, "ops_scrap_patent", slow_ops_scrap)
|
| 141 |
+
|
| 142 |
+
numbers = [f"US{1000000 + i}" for i in range(30)]
|
| 143 |
+
result = await ops_module.ops_scrap_patent_bulk(None, numbers)
|
| 144 |
+
|
| 145 |
+
assert peak <= ops_module.BULK_OPS_CONCURRENCY_LIMIT, (
|
| 146 |
+
f"{peak} concurrent OPS retrievals; limit is "
|
| 147 |
+
f"{ops_module.BULK_OPS_CONCURRENCY_LIMIT}")
|
| 148 |
+
assert result.failed_ids == numbers
|