alirezamika / alirezamika/autoscraper
Missing default timeout and HTTP status check in _fetch_html can hang builds and mask fetch errors
- Lingua principale
- Python
- Stelle
- 8k
- Fork
- 826
- Metriche di merge delle PR
- Nessuna PR unita negli ultimi 30g
Descrizione
## Summary
`AutoScraper._fetch_html` performs `requests.get(url, ...)` with no default timeout and never checks the HTTP status code. A slow or unresponsive server can therefore hang `build()` / `get_result_*()` indefinitely, and HTTP error pages (404/500/etc.) are silently parsed as if they were valid content.
## Location
- File: `autoscraper/auto_scraper.py`
- Class: `AutoScraper`
- Method: `_fetch_html` (called by `_get_soup`, which is called by `build`, `get_result`, `get_result_similar`, `get_result_exact`)
Relevant code path (static analysis of current `master`):
```python
res = requests.get(url, headers=headers, **request_args)
...
html = res.text
return html
```
No `timeout=` is supplied unless the caller happens to pass one inside `request_args`, and there is no `res.raise_for_status()` / status-code check.
## Problem
1. **No default timeout:** `requests.get` without `timeout` blocks indefinitely by default. Since `_fetch_html` only forwards `timeout` if the user explicitly puts it in `request_args`, the common call `AutoScraper().build(url=...)` has no upper bound on connection/read wait.
2. **HTTP errors silently accepted:** `res.text` is returned for any status code. A 404/403/500 page (often a short error template) is then fed to BeautifulSoup and rule matching, producing confusing empty results instead of a clear error.
## Trigger / Reproduction
- Call `AutoScraper().build(url="https://example.com/slow-endpoint", wanted_list=[...])` where the server accepts the connection but never sends a response body, without passing `request_args={"timeout": ...}`. Based on static analysis, the call will block on `requests.get` with no timeout.
- Call `build()` / `get_result_similar()` against a URL that returns HTTP 404/500. Based on static analysis, the error-page HTML is parsed normally and typically yields `[]`, with no exception or warning indicating the fetch failed.
Note: this is a static-analysis finding; I did not execute the code against a live hanging server.
## Expected Behavior
- Network fetch should have a sane default timeout (e.g. 10-30s) that the caller can override via `request_args`.
- Non-2xx responses should either raise a clear exception or be surfaced to the caller, rather than being parsed as valid pages.
## Actual Behavior
- No timeout is enforced unless the caller knows to inject `request_args={"timeout": ...}`.
- Any HTTP response body, regardless of status, is treated as page content.
## Impact
- Applications using autoscraper in batch jobs / web services can hang indefinitely on one unresponsive host, tying up workers.
- Debugging is harder because a 404/block page looks identical to "no rules matched" (`[]`).
## Suggested Direction
- Add an explicit default, e.g. `request_args.setdefault("timeout", 10)` (or similar) in `_fetch_html` before calling `requests.get`, while still allowing caller override.
- After the GET, check `res.status_code` / call `res.raise_for_status()` (or raise a library-specific error with URL + status) so fetch failures are distinguishable from zero matches. Document the behavior in `build` / `get_result_*` docstrings.
## Evidence
- Source: `autoscraper/auto_scraper.py`, `AutoScraper._fetch_html` — the only `requests.get` call site in the package.
- Search of repo issues for `timeout` shows only incidental user-supplied `timeout` usage (e.g. issue #90), not a report of the missing default or missing status check, so this does not appear to be a duplicate.
- No `timeout`, `raise_for_status`, or `status_code` handling exists anywhere in `autoscraper/` per inspection of `auto_scraper.py` and `utils.py`.
## Classification
- FACT: `requests.get` is called without a default timeout and without status validation (verified in source via API).
- INFERENCE: this permits indefinite blocking and silent error-page parsing.
- HYPOTHESIS: a modest default timeout + status check would fix both without breaking the `request_args` escape hatch.
Guida per i contributori
Nessuna guida per i contributori indicizzata per questo repository
Valutazione
Questa issue non è ancora stata valutata.