Skip to content

bpo-35989: Forbid a netmask > 32 for ipaddress.IPv4Network#11844

Open
matrixise wants to merge 3 commits into
python:mainfrom
matrixise:bpo-35989
Open

bpo-35989: Forbid a netmask > 32 for ipaddress.IPv4Network#11844
matrixise wants to merge 3 commits into
python:mainfrom
matrixise:bpo-35989

Conversation

@matrixise

@matrixise matrixise commented Feb 13, 2019 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

jflorian
jflorian reviewed Feb 13, 2019
View reviewed changes
Comment thread Lib/ipaddress.py Outdated
mask = address[1] if len(address) > 1 else self._max_prefixlen
if len(address) > 1:
if isinstance(address[1], int) and 0 < address[1] > 32:
raise NetmaskValueError("The netmask should be isn't valid")

@jflorian jflorian Feb 13, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This error message doesn't read well.

@tirkarthi tirkarthi Feb 14, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, this can be reworded and could include the max value (32) in the error so that users know the limit.

@matrixise matrixise Feb 14, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

really strange, that was not my original message. Maybe a bad paste :/

@matrixise

matrixise commented Feb 15, 2019

Copy link
Copy Markdown
Member Author

@tirkarthi @jflorian maybe we could validate this PR and ping a core-dev.

remilapeyre
remilapeyre suggested changes Mar 5, 2019
View reviewed changes
Comment thread Lib/ipaddress.py Outdated
addr = address[0]
mask = address[1] if len(address) > 1 else self._max_prefixlen
if len(address) > 1:
if isinstance(address[1], int) and 0 < address[1] > 32:

@remilapeyre remilapeyre Mar 5, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
if isinstance(address[1], int) and 0 < address[1] > 32:
if isinstance(address[1], int) and address[1] > 32:

if address is larger than 32, it is larger than 0.

@matrixise

matrixise commented Mar 23, 2019

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again

@bedevere-bot

bedevere-bot commented Mar 23, 2019

Copy link
Copy Markdown

Thanks for making the requested changes!

: please review the changes made to this pull request.

@tirkarthi

tirkarthi commented May 18, 2019

Copy link
Copy Markdown
Member

I believe this is fixed in master with https://bugs.python.org/issue36845 and backported to 3.7 .

./python.exe
Python 3.8.0a4+ (heads/master:73934b9da0, May 18 2019, 17:28:10)
[Clang 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import ipaddress
>>> ipaddress.IPv4Network(('192.0.2.1', 33))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/ipaddress.py", line 1450, in __init__
    self.netmask, self._prefixlen = self._make_netmask(mask)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/ipaddress.py", line 1112, in _make_netmask
    cls._report_invalid_netmask(prefixlen)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/ipaddress.py", line 474, in _report_invalid_netmask
    raise NetmaskValueError(msg) from None
ipaddress.NetmaskValueError: 33 is not a valid netmask

@remilapeyre

remilapeyre commented Jun 10, 2020

Copy link
Copy Markdown

Yes, this seems to have been fixed and is not an issue on master anymore:

➜  cpython git:(master) ./python      
Python 3.10.0a0 (heads/master:07d8112812, Jun 10 2020, 23:50:22) 
[Clang 11.0.3 (clang-1103.0.32.62)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import ipaddress
>>> ipaddress.IPv4Network(('192.0.2.1', 33))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/remi/src/cpython/Lib/ipaddress.py", line 1500, in __init__
    self.netmask, self._prefixlen = self._make_netmask(mask)
  File "/Users/remi/src/cpython/Lib/ipaddress.py", line 1162, in _make_netmask
    cls._report_invalid_netmask(prefixlen)
  File "/Users/remi/src/cpython/Lib/ipaddress.py", line 474, in _report_invalid_netmask
    raise NetmaskValueError(msg) from None
ipaddress.NetmaskValueError: 33 is not a valid netmask

@matrixise

matrixise commented Jun 22, 2020

Copy link
Copy Markdown
Member Author

hi all, I will follow this PR and the associated BPO. Thanks for the reminder.

@matrixise matrixise self-assigned this Jun 22, 2020
@JohnFlorian JohnFlorian mannequin mentioned this pull request May 13, 2023
Open
@github-actions

github-actions Bot commented Jun 15, 2025

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Jun 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tirkarthi tirkarthi tirkarthi left review comments
+2 more reviewers
@jflorian jflorian jflorian left review comments
@remilapeyre remilapeyre remilapeyre requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

@matrixise matrixise

Labels

awaiting change review stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@matrixise @bedevere-bot @tirkarthi @remilapeyre @jflorian @the-knights-who-say-ni @ezio-melotti