Skip to content

Conversation

MaelPic
Copy link
Contributor

@MaelPic MaelPic commented May 22, 2025

What do these changes do?

For IPv6 URL, if brackets are set in reversed order (closing bracket before open bracket), the raised exception is now ValueError("Invalid IPv6 URL"), which is more convenient that the reported one (IndexError: string index out of range).

Are there changes in behavior for the user?

If url contains brackets in opposite order:
Previous behavior: exception IndexError is raised
New behavior: exception ValueError is raised

Related issue number

Fixes #1485

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes

Copy link

codspeed-hq bot commented May 22, 2025

CodSpeed Performance Report

Merging #1512 will not alter performance

Comparing MaelPic:bugfix-ipv6-parsing (e3049dc) with master (d1bbd7a)

Summary

✅ 101 untouched benchmarks

Copy link

codecov bot commented May 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.63%. Comparing base (dcc3b15) to head (e3049dc).
Report is 1 commits behind head on master.

❌ Your project status has failed because the head coverage (98.08%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1512   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files          29       29           
  Lines        5795     5806   +11     
  Branches      265      265           
=======================================
+ Hits         5774     5785   +11     
  Misses         18       18           
  Partials        3        3           
Flag Coverage Δ
CI-GHA 99.63% <100.00%> (+<0.01%) ⬆️
MyPy 98.08% <100.00%> (+<0.01%) ⬆️
OS-Linux 99.80% <100.00%> (+<0.01%) ⬆️
OS-Windows 99.48% <100.00%> (+<0.01%) ⬆️
OS-macOS 99.68% <100.00%> (+<0.01%) ⬆️
Py-3.10.11 99.45% <100.00%> (+<0.01%) ⬆️
Py-3.10.17 99.70% <100.00%> (+<0.01%) ⬆️
Py-3.11.12 99.70% <100.00%> (+<0.01%) ⬆️
Py-3.11.9 99.45% <100.00%> (+<0.01%) ⬆️
Py-3.12.10 99.70% <100.00%> (+<0.01%) ⬆️
Py-3.13.3 99.75% <100.00%> (+<0.01%) ⬆️
Py-3.13.4t 99.75% <100.00%> (+<0.01%) ⬆️
Py-3.9.13 99.41% <100.00%> (+<0.01%) ⬆️
Py-3.9.22 99.65% <100.00%> (+<0.01%) ⬆️
Py-pypy7.3.16 99.33% <100.00%> (+<0.01%) ⬆️
Py-pypy7.3.19 99.36% <100.00%> (+<0.01%) ⬆️
VM-macos-latest 99.68% <100.00%> (+<0.01%) ⬆️
VM-ubuntu-latest 99.80% <100.00%> (+<0.01%) ⬆️
VM-windows-latest 99.48% <100.00%> (+<0.01%) ⬆️
pytest 99.80% <100.00%> (+<0.01%) ⬆️

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@webknjaz webknjaz added the bug label May 22, 2025
@MaelPic MaelPic force-pushed the bugfix-ipv6-parsing branch from e3af387 to 5001d56 Compare May 22, 2025 21:49
@MaelPic MaelPic changed the title 🐛 more meaningful exception if reversed brackets in ipv6 URL 🐛 Raises meaningful exception when IPv6 URL is malformed May 22, 2025
@webknjaz
Copy link
Member

note: we agreed with Mael that he'll add a change note later on, while traveling

@MaelPic MaelPic force-pushed the bugfix-ipv6-parsing branch from 5001d56 to f89d97b Compare May 23, 2025 00:47
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label May 23, 2025
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MaelPic thanks for your contribution during the PyCon US Sprints last week!

Would you me able to make the following cosmetic changes? Or do you need us to pick up the PR and complete it instead?

@webknjaz webknjaz requested review from bdraco and Dreamsorcerer May 28, 2025 16:49
…ersed

Problem: If brackets are reversed or content inside brackets is empty,
`bracketed_host` will be an empty string. In consequence, reaching the
first element of `bracketed_host` returns an `IndexError`.
Solution: Use `startswith` because it does not raise anything if the string is empty
and then it fallbacks into the following check verifying the presence of `:`.
Also, the raised message was made more generic but still keeps its meaning.
@MaelPic MaelPic force-pushed the bugfix-ipv6-parsing branch from f89d97b to 63ba8c5 Compare May 29, 2025 14:52
@bdraco
Copy link
Member

bdraco commented Jun 9, 2025

Thanks @MaelPic

@bdraco bdraco merged commit bf9f5ba into aio-libs:master Jun 9, 2025
52 of 54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided There is a change note present in this PR bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unhandled exception (IndexError) in URL parsing
4 participants