alirezamika / alirezamika/autoscraper

Missing default timeout and HTTP status check in _fetch_html can hang builds and mask fetch errors

オープン
#120 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Python
スター
8k
フォーク
826
PR マージ指標
30日以内にマージされた PR はありません

説明

## 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.

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。