Skip to content

Numeric ipv4 resolver bypass - #12827

Merged
bdraco merged 4 commits into
masterfrom
numeric-ipv4-resolver-bypass
Jun 7, 2026
Merged

bdraco merged 4 commits into
masterfrom
numeric-ipv4-resolver-bypass

Conversation

@bdraco

@bdraco bdraco commented Jun 7, 2026

Copy link
Copy Markdown
Member

What do these changes do?

is_ip_address() treats any digit-and-dot host as an IP literal, so TCPConnector._resolve_host() treated legacy numeric forms like 2130706433, 017700000001 and 127.1 as IP literals and handed the raw string straight to the socket layer instead of the configured resolver.

This adds is_canonical_ipv4_address(), which accepts only a standard dotted-quad with four decimal octets in range and no leading zeros, cross-checked against ipaddress.IPv4Address over a broad corpus. _resolve_host() now treats only canonical IPv4 literals (and IPv6) as IP addresses; any other numeric form is rejected with InvalidUrlClientError, and every non-literal host goes through the configured resolver as before.

Are there changes in behavior for the user?

A request to a non-canonical numeric IPv4 host now raises InvalidUrlClientError instead of being treated as an IP literal. Canonical dotted-quad addresses, IPv6 literals, and ordinary hostnames are unchanged.

Is it a substantial burden for the maintainers to support this?

No.

Related issue number

N/A

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes N/A
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt N/A, already listed
  • Add a new news fragment into the CHANGES/ folder

@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Jun 7, 2026
@bdraco bdraco added backport-3.14 Trigger automatic backporting to the 3.14 release branch by Patchback robot backport-3.15 Trigger automatic backporting to the 3.15 release branch by Patchback robot and removed bot:chronographer:provided There is a change note present in this PR labels Jun 7, 2026
@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Jun 7, 2026
@codecov

codecov Bot commented Jun 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.14%. Comparing base (69344c6) to head (aded875).
⚠️ Report is 516 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12827      +/-   ##
==========================================
+ Coverage   98.94%   99.14%   +0.20%     
==========================================
  Files         131      128       -3     
  Lines       47099    46539     -560     
  Branches     2435     2449      +14     
==========================================
- Hits        46600    46142     -458     
+ Misses        376      274     -102     
  Partials      123      123              
Flag Coverage Δ
Autobahn 22.37% <22.44%> (-0.02%) ⬇️
CI-GHA 98.91% <100.00%> (+<0.01%) ⬆️
OS-Linux 98.66% <100.00%> (+<0.01%) ⬆️
OS-Windows 97.04% <100.00%> (+<0.01%) ⬆️
OS-macOS 97.93% <100.00%> (+<0.01%) ⬆️
Py-3.10 98.15% <100.00%> (+<0.01%) ⬆️
Py-3.11 98.41% <100.00%> (+<0.01%) ⬆️
Py-3.12 98.50% <100.00%> (+<0.01%) ⬆️
Py-3.13 98.47% <100.00%> (+<0.01%) ⬆️
Py-3.14 98.49% <100.00%> (+<0.01%) ⬆️
Py-3.14t 97.56% <100.00%> (-0.01%) ⬇️
Py-pypy-3.11 97.42% <100.00%> (+<0.01%) ⬆️
VM-macos 97.93% <100.00%> (+<0.01%) ⬆️
VM-ubuntu 98.66% <100.00%> (+<0.01%) ⬆️
VM-windows 97.04% <100.00%> (+<0.01%) ⬆️
cython-coverage ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@aiolibsbot

aiolibsbot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Previous review — superseded by a newer review below.

@codspeed

codspeed Bot commented Jun 7, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 72 untouched benchmarks
⏩ 72 skipped benchmarks1


Comparing numeric-ipv4-resolver-bypass (aded875) with master (60b85e9)

Open in CodSpeed

Footnotes

  1. 72 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@bdraco
bdraco marked this pull request as ready for review June 7, 2026 00:50
@bdraco
bdraco requested review from asvetlov and webknjaz as code owners June 7, 2026 00:50
@aiolibsbot

Copy link
Copy Markdown
Contributor

PR Review — Numeric ipv4 resolver bypass

Solid, focused security fix. Merge-ready.

  • Correctly closes the resolver-bypass for legacy numeric IPv4 forms (2130706433, 017700000001, 127.1) by tightening what _resolve_host treats as an IP literal.
  • is_canonical_ipv4_address is conservative and correct: rejects unicode digits via isascii(), octal-style leading zeros, out-of-range octets, and non-4-part hosts.
  • IPv6 short-circuit (: in host) is preserved, so existing IPv6 and IPv4-mapped IPv6 paths are untouched.
  • Test coverage is excellent — parametrized accept/reject lists, an exhaustive itertools.product cross-check against ipaddress.IPv4Address, and an integration-style test confirming the resolver is never consulted for bad aliases (with match= already locking the error message).
  • Changelog fragment, error type (InvalidUrlClientError), and PR description are all appropriate.


Checklist

  • Input validation at boundaries (host before socket)
  • No bare except / swallowed errors
  • Untested branches covered
  • Edge case coverage (empty, unicode digits, boundary octets, octal, short-form)
  • No mutable defaults / unsafe eval / is-vs-== misuse
  • Changelog fragment present
  • Behavior change documented in PR description

Automated review by Kōan (Claude) HEAD=aded875

@bdraco
bdraco merged commit ccf218a into master Jun 7, 2026
44 of 46 checks passed
@bdraco
bdraco deleted the numeric-ipv4-resolver-bypass branch June 7, 2026 02:55
@patchback

patchback Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Backport to 3.14: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply ccf218a on top of patchback/backports/3.14/ccf218ab8c319faf166aff4d4c04f969fed7e341/pr-12827

Backporting merged PR #12827 into master

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/aio-libs/aiohttp.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/3.14/ccf218ab8c319faf166aff4d4c04f969fed7e341/pr-12827 upstream/3.14
  4. Now, cherry-pick PR Numeric ipv4 resolver bypass #12827 contents into that branch:
    $ git cherry-pick -x ccf218ab8c319faf166aff4d4c04f969fed7e341
    If it'll yell at you with something like fatal: Commit ccf218ab8c319faf166aff4d4c04f969fed7e341 is a merge but no -m option was given., add -m 1 as follows instead:
    $ git cherry-pick -m1 -x ccf218ab8c319faf166aff4d4c04f969fed7e341
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Numeric ipv4 resolver bypass #12827 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/3.14/ccf218ab8c319faf166aff4d4c04f969fed7e341/pr-12827
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@patchback

patchback Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Backport to 3.15: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply ccf218a on top of patchback/backports/3.15/ccf218ab8c319faf166aff4d4c04f969fed7e341/pr-12827

Backporting merged PR #12827 into master

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/aio-libs/aiohttp.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/3.15/ccf218ab8c319faf166aff4d4c04f969fed7e341/pr-12827 upstream/3.15
  4. Now, cherry-pick PR Numeric ipv4 resolver bypass #12827 contents into that branch:
    $ git cherry-pick -x ccf218ab8c319faf166aff4d4c04f969fed7e341
    If it'll yell at you with something like fatal: Commit ccf218ab8c319faf166aff4d4c04f969fed7e341 is a merge but no -m option was given., add -m 1 as follows instead:
    $ git cherry-pick -m1 -x ccf218ab8c319faf166aff4d4c04f969fed7e341
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Numeric ipv4 resolver bypass #12827 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/3.15/ccf218ab8c319faf166aff4d4c04f969fed7e341/pr-12827
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-3.14 Trigger automatic backporting to the 3.14 release branch by Patchback robot backport-3.15 Trigger automatic backporting to the 3.15 release branch by Patchback robot bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants