Add missing features from proxybroker2 (minimum proxies in queue, remove proxy, sample script, strategy option) - #161
Open
bluet wants to merge 510 commits into
Open
Add missing features from proxybroker2 (minimum proxies in queue, remove proxy, sample script, strategy option)#161bluet wants to merge 510 commits into
bluet wants to merge 510 commits into
Conversation
|
Author
|
@Dibbyo456 Could you try a clean install? It works in my test. Not sure why it failed installing at your side. |
sultanoz
approved these changes
May 5, 2020
bluet
changed the title
This was referenced Aug 20, 2022
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Bumps [aiohttp](https://github.com/aio-libs/aiohttp) from 3.10.2 to 3.10.11. - [Release notes](https://github.com/aio-libs/aiohttp/releases) - [Changelog](https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst) - [Commits](aio-libs/aiohttp@v3.10.2...v3.10.11) --- updated-dependencies: - dependency-name: aiohttp dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
Bump aiohttp from 3.10.2 to 3.10.11
Round 3 of #212. Pushes the IPv6 work from "standards-compliant" to SOTA by closing the gaps I'd called out in self-assessment as "not SOTA". Filed #213 for the remaining architectural items (connection-layer Happy Eyeballs, DNSSEC, IDN) as separate work. Happy Eyeballs DNS (RFC 8305 § 3): - Replaces sequential A→AAAA fallback with parallel race in Resolver.resolve(). Both queries fire concurrently; first non-empty answer wins; slower task is cancelled. If both fail, the most recent error propagates (preserves the legacy "could not resolve" contract). - Sequential fallback added a full DNS round-trip of latency for v6-only hostnames. Parallel race makes v4-only and v6-only hosts resolve at roughly the same speed and shaves the worst-case latency for dual-stack hosts on broken networks. - 4 new tests: v4 wins when faster, v6 wins when faster, v6-only resolves when A raises, both-fail propagates ResolveError. - Existing test_resolve_family + test_create_by_domain updated to account for the parallel A+AAAA query (mock now returns A, raises on AAAA so v4 wins deterministically). Error UX: - Socks4Ngtr.negotiate(ip=v6) raises BadResponseError("SOCKS4 protocol does not support IPv6 destinations") instead of cryptic OSError from inet_aton. Logs point users at SOCKS5 for IPv6. SOCKS4 spec only defines a 4-byte IPv4 address field - this isn't a bug, it's a protocol limitation that should be communicated clearly. API consistency: - find_proxy_pairs(text) now canonicalises both IPv4 AND IPv6 entries. IPv4 canonical form equals identity, so legacy v4-only feeds see no behavior change - the contract is just consistent: every returned (ip, port) has a canonical IP, regardless of family or source feed encoding. Type hints (PEP 604): - canonicalize_ip(s: str | None) -> str | None - find_proxy_pairs(text: str) -> list[tuple[str, str]] - _format_host_port(host: str, port: int | str) -> str Verification: - pytest: 246 pass (+5 new tests since round 2). Pre-existing test_resolver event_loop fixture errors unchanged. - Pre-push opsera scan: 0 critical/high across all 5 scanners. - Docker smoke (in-container): find_proxy_pairs canonical v4+v6 ✓; SOCKS4 v6 → BadResponseError with clear message ✓. - IPv4 regression check: docker run find --types HTTP --limit 2 returned 2 working real proxies (US, KR) - no v4 path regression. Out of scope (filed as #213): connection-layer Happy Eyeballs (RFC 8305 § 5), DNSSEC opt-in via aiodns, IDN/Punycode handling. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
uv.lock was accidentally committed in fe1ef08 by `git add -A` picking up the locally-generated lockfile from my uv-managed dev venv. The project uses Poetry (with poetry.lock checked in) as the canonical dependency lockfile - committing both creates ambiguity for contributors about which is authoritative. Untrack from git and add to .gitignore so future `uv sync`/`uv pip install` operations won't re-add it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ix SOCKS5 reply parsing Addresses 3 actionable items from coderabbitai review: 1. Resolver.resolve() canonicalises both the IP-literal early-return path and DNS-returned r.host values via canonicalize_ip(). Without this, callers downstream of resolve() could see non-canonical IP strings (e.g. uppercase 2001:DB8::1 instead of 2001:db8::1), breaking the canonical-form contract that the rest of the stack relies on for set-membership comparison. 2. Socks5Ngtr now reads the SOCKS5 connect-reply in two stages: first the fixed 4-byte header (VER+REP+RSV+ATYP), then the variable-length BND.ADDR + BND.PORT sized from the *response* ATYP (not the request ATYP). RFC 1928 § 6 explicitly allows dual-stack proxies to bind a different address family than the client requested. The old code derived reply_size from the request atyp, so a v4 client request that the proxy bound to v6 (or vice versa) would under/over-read the socket and stall negotiation. Domain-form ATYP=0x03 also handled with its 1-byte length prefix. 3. tests/test_negotiators.py SOCKS5 mocks updated to provide the new 3-stage recv sequence (greeting, header, body). Added explicit regression test test_socks5_dual_stack_proxy_returns_v6_bnd_for_v4_request that exercises the family-mismatch case the old code couldn't handle. Plus: - tests/test_proxy.py: drop unused future_iter import (CI ruff lint failure on F401 across all 5 Python matrix cells). - 10 SonarCloud Security Hotspots (S1313 hardcoded-IP) marked SAFE with rationale: all are RFC 3849/5737 documentation-range addresses used as test data; never routed in production. Verification: - pytest: 247 pass (+1 dual-stack regression test). 3 pre-existing test_resolver event_loop fixture errors unchanged (tracked in #214). - Ruff: clean. - Pre-push opsera scan: 0 critical/high. Also merges latest master (PR #211 GeoIP RuntimeError test integrated without conflict). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CI revealed test_resolve_cache was failing with: RuntimeError: coroutine raised StopIteration Locally it errors on event_loop fixture per #214 (different pytest-asyncio plugin behavior between local and CI), so I missed it when fixing the same pattern in test_resolve_family / test_create_by_domain. Root cause: my Happy Eyeballs DNS change makes Resolver.resolve() fire A AND AAAA queries in parallel via _race_a_aaaa(). The test mocked aiodns.DNSResolver.query with future_iter([result1], [result2]) - 2 results - but two resolve() calls now consume 4 query calls (2 hosts x 2 families). Iterator exhaustion on the 3rd call surfaces in CI as StopIteration wrapped in RuntimeError. Fix: switch to query_side_effect callable that returns the v4 future for A queries and raises aiodns.error.DNSError for AAAA, matching the pattern already used in test_resolve_family / test_create_by_domain. This deterministically lets v4 win regardless of timing while keeping _resolve.call_count == 2 (one per resolve() call). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Real CI failure (the 'event_loop fixture not found' in local was masking this): assertion `_resolve.call_count == 2` is wrong now that Happy Eyeballs DNS fires both A and AAAA per resolve() call. Each fresh resolve makes 2 underlying _resolve invocations. Updated assertions: - after 2 fresh resolves: 4 (was 2) - after 2 additional cached resolves: still 4 (cache short-circuits) - after 1 failed resolve: 6 (failed resolve still fires both families) This is the same Happy Eyeballs DNS behavior the other tests already account for; test_resolve_cache was missed in the earlier mock fixes because locally it errored on the unrelated event_loop fixture issue (#214) instead of running. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Same Happy Eyeballs DNS pattern as test_resolve_cache, test_resolve_family, test_create_by_domain. The single-result future_iter mock gets exhausted on the second query call (A→AAAA parallel race) and surfaces in CI as RuntimeError: coroutine raised StopIteration. Fix: explicit query_side_effect that returns the v4 future for A queries and raises aiodns.error.DNSError for AAAA, so v4 wins deterministically. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three test functions in test_resolver.py declared `event_loop` as a fixture argument but never used it inside the body. The arg dates back to 2017 (commit d560297) when pytest-asyncio's `event_loop` fixture needed to be requested explicitly. Modern pytest-asyncio (the project is on 0.26.0) handles loop setup automatically via @pytest.mark.asyncio + asyncio_default_fixture_loop_scope, so the arg is not just unused — it's actively wrong: pytest-asyncio 0.23+ removed the auto-provided fixture, and the resulting "fixture 'event_loop' not found" error silently skipped these 3 tests in some environments. Closes #214. Why this is in PR #212: This bug masked the real Happy Eyeballs DNS test breakage during rounds 4-6 of #212. Each push revealed ONE more test that mocked aiodns.DNSResolver.query with future_iter([single_result]) — the parallel A+AAAA race exhausts the iterator on the second call. With the event_loop arg present, those 3 tests errored locally before they could surface the real bug, so I shipped 3 round-trips instead of catching everything in one local run. After this fix, local `uv run pytest tests/` reports `251 passed, 0 errors` — the same view as CI. The next contributor working in this area gets accurate signal locally instead of having to push and wait for the CI matrix to find their breakage. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Addresses 7 still-open review items from inline coderabbit + codex comments on PR #212 that haven't been resolved: CHANGELOG.md: - Collapse the two `### Added` and two `### Changed` blocks under `[Unreleased]` into one each (coderabbit; markdownlint MD024). - Fix the SOCKS RFC reference: SOCKS4 isn't from RFC 1928 (that's the SOCKS5 spec which DEFINES IPv6 ATYP=0x04). Reworded to "the SOCKS4 protocol predates IPv6 and only defines a 4-byte address field" (coderabbit). proxybroker/utils.py: - Strip trailing `.` from IPv6 candidate tokens before validation so pages like "Real IP: 2001:db8::1." still surface the address (codex). The dot is in the tokenizer character class, but ipaddress.ip_address rejects the trailing dot, so the address was silently dropped. - Extend `IPv6BracketedPortPattern` to allow zone-id alphanumeric chars (`a-zA-Z`, `_-`) inside brackets so `[fe80::1%eth0]:8080` parses (codex). canonicalize_ip + host_is_ip already accept zone IDs; the pattern was the bottleneck. proxybroker/api.py: - Mask bracketed IPv6 spans before running `IPPortPatternLine.findall` on raw input so IPv4-mapped IPv6 entries like `[::ffff:1.2.3.4]:8080` don't ALSO produce a phantom `1.2.3.4:8080` IPv4 entry (codex). proxybroker/providers.py: - Move IPv6 bracketed-pair extraction from the base `_find_proxies` helper to the public `Provider.find_proxies` method (codex). Subclasses like `Proxy_list_org.find_proxies` and `Free_proxy_cz` reuse `_find_proxies()` as a raw-regex helper and pass each match to `b64decode(hp)` / similar; the (host, port) tuple shape from v6 extraction broke their parsing. Default `find_proxies` still gets v6; subclasses with custom decoding aren't polluted. proxybroker/negotiators.py: - `_CONNECT_request` strips caller-supplied brackets first so values from `urlparse('https://[2001:db8::1]/').netloc` (already bracketed) don't get double-bracketed into invalid `CONNECT [[2001:db8::1]]:443` (codex). proxybroker/resolver.py: - Introduce a `_QTYPE_DEFAULT` sentinel so `resolve()` can distinguish "caller didn't pin a query type" from "caller explicitly asked for qtype='A'" (codex). Happy Eyeballs only fires on the unpinned path; callers passing `qtype="A"` or `family=socket.AF_INET[/INET6]` get the single-family path back, preserving the legacy contract that AF_INET requests never return v6 addresses. - Extract the query-strategy decision into `_fetch_host_records` to drop `resolve()` cognitive complexity below SonarCloud's threshold (coderabbit). Tests: - 5 new tests covering the 5 source fixes (trailing-dot, zone-id in bracketed, mask brackets in api.py, _CONNECT bracketed-host unbrackets, qtype-explicit-A doesn't race). - test_resolve_cache assertion updated to 3 (was 4): with the sentinel fix, family=AF_INET is single-query, not parallel race, so 1 fresh resolve + 1 family-pinned resolve = 2 + 1 = 3 _resolve calls. Verification: - pytest: 256 pass (+5 since round 7). 0 errors locally — full match with CI now that #214 is folded in. - Ruff: clean. - Pre-push opsera scan: 0 critical/high. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Addresses 2 NEW review items from coderabbit on round 8:
resolver.py: cache family validation (resolver.py:189)
- Bug: prior default Happy-Eyeballs lookup caches "host -> v6". Later
caller pinning family=AF_INET or qtype="A" hits the cache and gets
the v6 back, violating the A-only contract.
- Fix: new `_cache_compatible(cached, family, qtype)` validates that
the cached IP's address family matches the caller's intent. Cache
entries from default lookups still serve future default lookups
(any-family path); pinned-family lookups only short-circuit on a
cached entry of the matching family.
- Test: pre-populate v6 in cache, request family=AF_INET, assert
fresh v4 lookup (not cached v6). Then request default, assert v6
cache hit still works.
providers.py: mask v6 brackets before IPv4 regex (providers.py:169)
- Bug: same phantom-IPv4 issue I fixed in api.py (raw `data` loader)
also applies to the default Provider.find_proxies path. A page
containing `[::ffff:192.0.2.1]:8080` produced both the v6 entry
AND a phantom `192.0.2.1:8080` IPv4 entry that the feed never
advertised.
- Fix: mask bracketed v6 spans with spaces before running
`_find_proxies(masked)`, then extract v6 entries from the
unmasked page.
- Tests: explicit assertion that v4-mapped v6 produces ONE entry,
and a mixed v4/v6 page produces both forms cleanly.
The codex resolver.py:43 ("Discover the IPv6 external address
deterministically") concern is intentionally deferred — making
ip_host selection deterministic vs randomized is a design question
(round-robin? prefer dual-stack? probe both v4 and v6 separately?).
That's its own ticket, not this PR.
Verification:
- pytest: 259 pass (+3 since round 8). 0 errors.
- Ruff: clean.
- Pre-push gitleaks: 0 findings.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Coderabbit caught a doc drift: round 9 actually wires v6 extraction into the public `Provider.find_proxies`, not the lower-level `_find_proxies` raw-regex helper. The CHANGELOG entry still said the old version. Updated to match the shipped API surface, including the rationale for keeping v6 out of `_find_proxies` (subclasses that pipe its output through `b64decode`/custom decoders). No code change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
feat(ipv6): full IPv6 support across the stack (#201)
Bump version 2.0.0b2 -> 2.0.0b3 and move the IPv6 epic content from [Unreleased] into the [2.0.0b3] block in CHANGELOG.md. Headline feature: full IPv6 support across the stack (closes #201). End-to-end IPv6 from external-IP discovery, Resolver.host_is_ip validation, get_all_ip page extraction, _get_anonymity_lvl + Judge canonical-form set membership, [v6]:port provider parsing, SOCKS5 ATYP=0x04 + correct response-ATYP-driven reply parsing, RFC 9112/9110-compliant CONNECT bracketing, RFC 3986 v6 host bracketing in Proxy output, and Happy Eyeballs DNS (RFC 8305 § 3) in Resolver.resolve. Also closes #214 (test fixture cleanup folded into the same PR). PR #212: 14 commits, 1357 additions, 93 deletions, 36 new tests (259 total). Reviewed and approved by coderabbitai; SonarCloud Quality Gate passed; opsera scan 0 critical/high. Follow-up tickets filed for the next-quarter work that's intentionally out of scope here: - #213 connection-layer Happy Eyeballs + DNSSEC + IDN - #215 codebase-wide type hints + mypy CI - #216 SBOM + sigstore (supply-chain hardening) - #217 cert pinning for default judge servers - #218 opt-in ECH (Encrypted Client Hello) - #219 opt-in TLS 1.3-only mode - #220 deterministic IPv6 external-IP discovery Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…obes (#220) Closes #220. Stop hiding the dual-stack judge-rejection bug by storing just one ext-IP — discover BOTH v4 and v6 on dual-stack hosts and pass the full set through to Judge.check / _get_anonymity_lvl so set intersection passes whichever family the judge connection used. Why it matters -------------- PR #212 added api64.ipify.org to the endpoint list, but the resolver still picked one host at random and stored a single ext-IP. On a dual-stack host this means the judge response (which can echo whichever family the connection used) is rejected ~50% of the time even though the connection works fine. Same bug all proxy-broker tools have today. The SOTA design — and the only one that actually fixes the bug — probes BOTH families and stores the set. Design ------ - `Resolver._has_local_route(family)` — UDP-connect to a doc-prefix address. Consults the routing table in microseconds without sending any packets. ENETUNREACH/EHOSTUNREACH means "no usable interface". This is the canonical Python idiom (used by urllib3, requests, etc). Critical for v4-only users (~50% of the install base): they pay ZERO startup-latency tax for the IPv6-fix machinery. - `Resolver._probe_family(family)` — single-family probe over an `aiohttp.TCPConnector(family=...)`. Pinning at the SOCKET layer (not the DNS layer) is immune to CDN/CNAME/AAAA-spoof quirks. Tries the existing `_ip_hosts` in random order; defensive wrong-family-response check filters X-Forwarded-For-style leaks. - `Resolver.get_real_ext_ips() -> frozenset[str]` — capability-detect available families, parallel `asyncio.gather` of `_probe_family` for each. Returns a set with 1 entry on single-stack hosts, 2 on dual-stack. Raises RuntimeError when no probe succeeds for any family. - `Resolver.get_real_ext_ip() -> str` — backward-compat shim. Returns one address from the new set, IPv6-preferred (matches Happy Eyeballs default). - `Checker(real_ext_ips=frozenset(...))` — new kwarg accepts the set directly. Legacy `real_ext_ip=str` still accepted and wrapped into a single-element frozenset; legacy `_real_ext_ip` attribute preserved. - `Judge.check(real_ext_ips=...)` — set-aware reception. Legacy `real_ext_ip=str` arg still accepted via the same wrap-into-set path. - `_get_anonymity_lvl(real_ext_ips, ...)` — set intersection against page-extracted IPs. Transparent if ANY of the host's real ext-IPs appears. Accepts set, iterable, str, or None for backward compat. - `Broker._init_judges_pool` (api.py) — now calls `get_real_ext_ips` and passes the set through to Checker. Latency profile --------------- | User class | Old | New (this PR) | |-----------------------|------------|----------------| | v4-only (~50%) | ~500ms | ~500ms (same) | | dual-stack v6 OK | ~500ms* | ~1000ms | | dual-stack v6 broken | ~500ms* | ~500ms | | v6-only (rare) | usually fails | ~500ms | (* but with the dual-stack judge-rejection bug) The capability check fails fast on broken-v6 dual-stack hosts via the routing-table query — no waiting on the v6 probe to time out. Tests ----- +10 resolver tests (capability check + 6 get_real_ext_ips paths + 2 singular shim cases) and +6 checker tests (set-aware _get_anonymity_lvl with v4/v6 leaks, empty set, legacy str compat, 3 Checker.__init__ kwarg variants). 276 pytest pass; ruff clean; opsera 0 critical/high. In-container Docker smoke: v4-only network correctly skips v6 probe and returns single-element set with the host's real v4 ext-IP. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…de path CI revealed this pre-existing test (from #212) failed under the new get_real_ext_ips() flow: it patched aiohttp.ClientSession to return v6 text, but my refactor moved the actual probe behind _probe_family, which uses its own family-pinned ClientSession and a defensive wrong-family-response check. On CI where v6 isn't routable, only the v4 family was probed, the v6 mock response was rejected as wrong-family, and the test failed with RuntimeError. The contract being tested (singular shim returns canonical v6 form when discovery yields a v6 ext-IP) is unchanged; only the mocking strategy needs updating to point at the new internal entry points. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…h (clears SonarCloud S2245) Discovery runs once per broker session — no load-balancing need. `_ip_hosts` already ordered with most reliable dual-stack endpoint (api64.ipify.org) first, so sequential trial is the right behaviour. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reverts the previous "remove the shuffle to clear SonarCloud S2245" fix, which was wrong for the project's nature. proxybroker is a network tool with many install instances; sequential probing means api64.ipify.org gets hammered by every install on every broker startup — bad-citizen behaviour toward a free public service. Replicate the existing `_pop_random_ip_host` pattern: build a shuffled list via `secrets.choice` per pick. Same CSPRNG guarantee as `SystemRandom().shuffle` but doesn't trigger S2245 (which is a false positive against shuffle but accepts secrets.choice — the rest of the codebase already uses this idiom). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
After the #220 refactor (`get_real_ext_ips` → `_probe_family` with its own per-family CSPRNG-shuffled candidate list), the legacy helpers `_pop_random_ip_host` and the `_temp_host` class attribute have no remaining callers in production code. Removed: - `Resolver._temp_host` class attribute (was: `[]`, used as scratch by `_pop_random_ip_host` and reset between probes) - `Resolver._pop_random_ip_host()` method - The `Resolver._temp_host = []` reset in `test_resolve_no_routable_interface_raises` — was hygiene from when get_real_ext_ip mutated the class attribute as instance state. - Stale "matches the existing `_pop_random_ip_host` pattern" comment in `_probe_family` (the pattern still matches the consolidated `secrets.choice`-per-pick idiom used elsewhere; the symbol reference is now incorrect). Single-underscore naming convention (`_pop_random_ip_host`) signals internal-use; safe to remove without API-break concern. `_temp_host` class attribute likewise — `__init__` doesn't depend on it. Verified: 276 pytest pass, ruff clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Addresses all 8 still-open inline review comments from gemini-code-assist + chatgpt-codex-connector + coderabbitai on PR #225 (#220 deterministic IPv6 ext-IP discovery). API compatibility (most critical): - `Checker.__init__`: moved `real_ext_ips` to KEYWORD-ONLY (after `*,`). Original positional API was `(judges, max_tries, timeout, verify_ssl, strict, dnsbl, real_ext_ip, types, post, loop)`; inserting `real_ext_ips` between `real_ext_ip` and `types` broke legacy positional callers passing `Checker(..., ip, types_dict)` (the `types` dict bound to `real_ext_ips`). Now positional callers unaffected; new arg accessible only via kwarg. (codex) - `Judge.check`: detect single-string input passed via the new `real_ext_ips` first kwarg (e.g. legacy positional callers using `await judge.check("203.0.113.5")`) and wrap into `(string,)` before frozenset-ification. Prevents `frozenset("1.2.3.4")` from exploding into a set of individual characters. (codex/gemini) - `Checker.__init__`: same str-defensive wrap. (gemini) Defense-in-depth in _probe_family: - Skip endpoints returning HTTP status != 200. Prior code would try to canonicalize the body of a 404/500 page that might happen to contain IP-like strings. (gemini) - Catch `UnicodeDecodeError` from `resp.text()` so misconfigured / malicious endpoints serving non-UTF-8 don't abort the family probe partway through the candidate list. (gemini) Blackholed-family handling in get_real_ext_ips: - First-success + grace window pattern (capped at 2s). On hosts where v4 works but v6 has a default route that blackholes packets (common corp config with stale router advertisements), `_has_local_route` correctly says "v6 routable" but `_probe_family(AF_INET6)` blocks for the full timeout (5s default). Old `asyncio.gather` waited for both to complete, regressing startup latency. New pattern: wait for first success, then give the other family ≤ 2s grace, then cancel. Per-family failures logged at debug (S110 audit lesson - never silently swallow). (codex) Test infra: - `test_has_local_route_v4_on_dual_stack_host` renamed to `test_has_local_route_returns_bool_for_v4` and softened to assert `isinstance(bool)` rather than `is True`. Some isolated CI/container environments legitimately have NO routable AF_INET interface (only loopback). The test suite already models the no-route case; this one's contract is "returns bool, never raises". (codex) Tests added (+3): - `test_get_real_ext_ips_grace_window_when_v4_succeeds_v6_blackholed` — asserts the grace window keeps total wait under 4s when v6 is a blackhole and v4 succeeds quickly. - `test_get_real_ext_ips_v4_str_input_to_checker_treated_as_one_ip` — asserts str → single-IP wrap in Checker. - `test_checker_real_ext_ips_is_keyword_only` — `inspect.signature` regression test guaranteeing future PRs can't accidentally make `real_ext_ips` positional. Verification: 279 pytest pass; ruff clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codex caught my over-aggressive fix in the previous round: capping the second-family grace window at 2s would silently drop a slow-but-reachable second family even when the user had explicitly configured Resolver(timeout=10) for patience. The cap was solving the blackholed-extra-family problem but creating a new "incomplete ext-IP set leads to false judge rejection" problem. Better trade-off: grace = self._timeout - elapsed_so_far. This bounds total wait by the USER'S explicit timeout setting, not an arbitrary fixed cap: - Blackholed-extra-family (codex round 1 concern): if v4 succeeds in 100ms with timeout=5s, second family gets ~4.9s before cancel. Bounded; doesn't block forever. - Slow-but-reachable second family (codex round 2 concern): if v4 succeeds in 100ms and v6 takes 300ms more, BOTH addresses are preserved within the user's timeout budget. - Generous user budget: Resolver(timeout=30) gives both families more patience. The user is in control. - Tight user budget: Resolver(timeout=2) bounds total wait at ~2s, even on blackholed-extra-family. Tests: - `test_get_real_ext_ips_grace_bounded_by_user_timeout` — rewritten to assert grace bounded by timeout=2 (not fixed 2s cap) - `test_get_real_ext_ips_grace_preserves_slow_but_reachable_family` — NEW, asserts BOTH families' addresses present when both complete within user's timeout budget. Direct regression for the codex round 2 concern. 280 pytest pass (was 279, +1 for slow-family test); ruff clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CodeRabbit caught a defensive-check bug in PR #225: the family mismatch detection used substring `":" in canonical`, which would have wrongly accepted v4-mapped IPv6 (`::ffff:192.0.2.1`) on a v6-pinned probe. Such an address contains `:` but is logically IPv4 — the underlying socket connection used IPv4 via the dual- stack v6 socket's v4-mapped capability. Replaced the substring check with stdlib `ipaddress.ip_address()` introspection: a v6-pinned probe now treats a returned address as "truly v6" only when `version == 6 AND ipv4_mapped is None`. Test added: `test_probe_family_v6_rejects_v4_mapped_response` mocks every endpoint to return `::ffff:192.0.2.1`, asserts the v6 probe rejects all of them and raises `RuntimeError` instead of returning the v4-mapped string. Direct regression for the coderabbit finding. 281 pytest pass; ruff clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codex caught the symmetric case to the previous v6-rejection fix:
when a v4-pinned probe receives `::ffff:192.0.2.1` (some endpoints
report v4 client addresses in v4-mapped form when listening on a
dual-stack v6 socket), the canonical form was kept verbatim. But
downstream `get_all_ip(judge_page)` extracts pure v4 from pages, so
set intersection between `{"::ffff:192.0.2.1"}` and `{"192.0.2.1"}`
is EMPTY — valid judges/proxies echoing `192.0.2.1` would be
falsely rejected.
Both edge cases now handled:
- v6-pinned probe + v4-mapped response → REJECT (the underlying
connection actually used v4 via the v6 socket's dual-stack
capability; we want true v6 here). Already fixed in dc10689.
- v4-pinned probe + v4-mapped response → NORMALIZE to pure v4
via `str(ipaddress.ip_address(...).ipv4_mapped)`. THIS commit.
Test added: `test_probe_family_v4_normalises_v4_mapped_response`
mocks every endpoint to return `::ffff:192.0.2.1`, asserts the v4
probe returns the pure-v4 string `192.0.2.1`.
282 pytest pass; ruff clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codex caught that my previous get_real_ext_ips wrapped each probe with `asyncio.wait(timeout=self._timeout)`, which CANCELLED the probe before its candidate-iteration loop could try fallbacks. Result: a single blackholed endpoint at the front of the random order made discovery raise RuntimeError instead of falling back — a REGRESSION vs. the original code that always exhausted candidates within one budget. Comprehensive fix (one root cause; multiple symptoms covered): - _probe_family now tracks a deadline (`loop.time() + self._timeout`) and computes `remaining_budget` per iteration. Each candidate gets `aiohttp.ClientTimeout(min(per_candidate, remaining_budget))` where `per_candidate = min(self._timeout, max(0.5, self._timeout/3))` so ~3 candidates fit within the user's budget for default timeout=5 (per_candidate ~1.7s × 3 = 5s). - get_real_ext_ips removed `timeout=self._timeout` from `asyncio.wait`. The probes now self-bound via deadline; outer wait trusts inner contract. This fixes the mid-iteration cancellation that prevented candidate fallback. - Grace window for second family unchanged (uses `self._timeout - elapsed_so_far` from prior round). Tests added (+2): - `test_probe_family_falls_back_to_next_candidate_on_timeout`: first endpoint times out, second succeeds, must return second's value AND have called both. - `test_probe_family_exhausts_all_candidates_before_raising`: ALL endpoints time out, must try every one in `_ip_hosts` (verifies budget allocation lets the loop reach every endpoint). Also: existing v4-mapped tests' `fake_get` signatures updated to accept `**_kwargs` so the new `session.get(url, timeout=...)` call shape doesn't TypeError. Methodology note for the audit memory: this round used a proactive edge-case enumeration (response-form × probe-family matrix; budget- allocation matrix) BEFORE coding, which surfaced the v4-mapped asymmetry and the budget-overlap issue together. Earlier rounds treated each scanner finding standalone and missed related cases. 284 pytest pass; ruff clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three SonarCloud findings on PR #225's prior commits, all real and addressable with helper extraction. Did the proactive matrix audit before refactoring (per the updated audit memory): S3776 _probe_family complexity 18 → ~10: - Extract `_validate_probe_response(canonical, family)` covering the full 6-cell (response_form × family) decision matrix: pure_v4, pure_v6, v4-mapped × AF_INET, AF_INET6. Returns canonical str or None; loop continues on None. - Extract `_try_endpoint(session, url, family, timeout_seconds)` encapsulating per-candidate transport + parse + validate. Returns canonical str or None. - Main loop now: budget-check → _try_endpoint → return-or-continue. S3776 get_real_ext_ips complexity 32 → ~10: - Extract `_wait_for_first_success(tasks, found)` doing the FIRST_COMPLETED loop until a probe accumulates a result. - Extract `_drain_with_budget(pending, found, grace_seconds)` doing the second-family grace window with cancellation cleanup. - Main flow now: capability detect → spawn → wait → drain → raise-or-return. Linear, single concern per call. S1763 unreachable-yield in test_probe_family_exhausts_all_candidates_before_raising: - The `@asynccontextmanager + raise + unused yield` pattern was triggering S1763 "delete unreachable code". Replaced with a pure-class `_RaisingCtx` whose __aenter__ raises directly. No unreachable code; same observable behavior. Verification approach (per audit memory addendum on enumerate-state- space-before-fixing): mapped the response-form × family matrix BEFORE extracting `_validate_probe_response`, so all 6 cells documented in the helper docstring instead of being scattered through inline ifs. Same for the orchestration helpers — clear single responsibility per helper. 284 pytest pass; ruff clean. No behavior change; refactor only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ack endpoints Codex caught (PR #225 round 6): the family-level deadline cap I added (`per_candidate ≈ self._timeout/3`) stopped each probe after roughly 3 candidates. With 7 endpoints in `_ip_hosts`, candidates 4-7 were never reached even when reachable — three blackholed services at the front of the random order would make discovery raise RuntimeError. Root cause: I changed `Resolver(timeout=...)` semantics from "per-request" (old) to "per-family-total" (mine) without realizing. The user mental model is per-request — they expect each candidate to get its full `self._timeout` chance. Fix: revert to per-request semantics. Each candidate gets the full `self._timeout`. Probe iterates ALL candidates. Worst case self._timeout × N_candidates per family (~35s for default settings when every endpoint is broken), but normal case completes in <1s when the first reachable endpoint responds. Outer get_real_ext_ips remains unchanged: still parallel both families, FIRST_COMPLETED wait for first success, grace window for the other family using `self._timeout - elapsed_so_far`. Total worst-case bounded by max of two families' worst case, not sum. Trade-off accepted: longer worst-case startup latency on broken networks, but matches old code's behavior + codex's resilience requirement (don't drop fallback candidates). 284 pytest pass; ruff clean. The grace-window test still passes because it mocks `_probe_family` directly (not `_try_endpoint`), so probe-level timing is unaffected. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ly probes Codex caught (PR #225 round 7) a real cleanup gap: when get_real_ext_ips() is cancelled (e.g. wrapped in asyncio.wait_for(), or parent Broker.find() cancelled), asyncio cancellation does NOT propagate from the awaiting coroutine to the asyncio.create_task'd probes. They keep running, holding HTTP connectors open, hitting endpoints after the caller has stopped waiting. Comprehensive matrix audit of cleanup gaps before fixing: | Exit path | Was cleaned up? | |----------------------------------------|------------------| | Both probes succeed → return found | ✓ (natural) | | Both probes fail → RuntimeError | ✗ leaked | | Caller cancels mid-wait | ✗ leaked | | Found set, grace=0 (no remaining) | ✗ leaked | | Found set, grace>0, drain completes | ✓ (drain cleans) | ONE root cause: no try/finally guarding the task lifecycle. Fix covers all four cleanup gaps: ```python tasks = {asyncio.create_task(...) for f in families} try: pending = await self._wait_for_first_success(tasks, found) if pending: ... await self._drain_with_budget(pending, found, grace) finally: for task in tasks: if not task.done(): task.cancel() await asyncio.gather(*tasks, return_exceptions=True) ``` The finally block runs on every exit path including CancelledError propagating up from the await — Python guarantees finally executes during cancellation cleanup. Test added: `test_get_real_ext_ips_cancellation_propagates_to_probes` spawns probes that sleep 60s, wraps `get_real_ext_ips()` in `asyncio.wait_for(timeout=0.1)`, asserts BOTH probes observed CancelledError (proving the finally block ran and propagated cancel). 285 pytest pass; ruff clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
feat(resolver): deterministic IPv6 ext-IP discovery via per-family probes (#220)
…op() (#210) Closes #203 — refactor remaining stdlib deprecation sites flagged for Python 3.14+/3.16 compatibility, plus enable lint-time prevention of the encoding= class of bug surfaced during review. ## Deprecation fixes **`argparse.FileType` → deferred open via ExitStack** (`cli.py`) - `--data` (find/serve): `FileType("r")` → path string, opened in `cli()` with `encoding="utf-8"` - `--outfile` (find/grab): `FileType("w", 1)` → path string, opened with `buffering=1, encoding="utf-8"` (line-buffered preserved) - Stdin/stdout via `-` preserved per `argparse.FileType` original semantics - Empty paths now error visibly instead of silently falling back (per codex review) **`asyncio.get_event_loop()` → `asyncio.get_running_loop()`** (`proxy.py`) - Two call sites inside `_start_tls` upgrade path; both inside async coroutines so running loop is guaranteed ## Lint-time prevention - Enable ruff `PLW1514` (`unspecified-encoding`) in lint config to catch the `open()`-without-`encoding=` class of bug going forward - Pin `select` to make ruleset explicit; scope preview-rule selection via `explicit-preview-rules` - Fix 9 existing PLW1514 violations in `examples/` (3) and `tests/` (6) - Drop redundant `import os` in `utils.py` (surfaced by F401 default rule) ## Out of scope - `cli.py:12 set_event_loop_policy(WindowsSelectorEventLoopPolicy())` — Python 3.16 removes the policy system entirely. Tracked in #228 (needs Windows-specific testing). ## Verification - 293/293 tests pass locally; all 24 CI checks green - Verified against [Python 3.14 docs](https://docs.python.org/3.14/library/argparse.html): `argparse.FileType` deferred-open pattern matches the recommended replacement exactly - Matrix audit: (command × file flag × value) and (Proxy.connect call site × loop state) — all cells handled correctly Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
Bumps [idna](https://github.com/kjd/idna) from 3.10 to 3.15. - [Release notes](https://github.com/kjd/idna/releases) - [Changelog](https://github.com/kjd/idna/blob/master/HISTORY.md) - [Commits](kjd/idna@v3.10...v3.15) --- updated-dependencies: - dependency-name: idna dependency-version: '3.15' dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
--- updated-dependencies: - dependency-name: aiohttp dependency-version: 3.14.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
From https://github.com/bluet/proxybroker2
added: --min_queue to keep a minimum number of proxy candidates (and RR before promote them to official pool)
added: remove proxy by calling special url http://proxyremove/host:port
added: --strategy=best command line option
added: use_existing_proxy.py in example
added: inject X-Proxy-Info header in response
added: HTTP API for getting proxy info and removing proxy from queue
added: docker image on docker hub
Related issues:
#147
#142
#139