Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#17751 closed Bug (fixed)

GenericIPAddressField allows an ipv6 address to start with a space

Reported by: federico.capoano@… Owned by: Erik Romijn
Component: Forms Version: master
Severity: Normal Keywords: GenericIPAddressField, ipv6
Cc: net147 Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

GenericIPAddressField allows an ipv6 address to start with a space, it should raise a ValidationError instead.
At least the IPAddressField (ipv4) does it.

Attachments (2)

17751-test.diff (1.6 KB) - added by Claude Paroz 5 years ago.
Test showing issue
17751-2.diff (1.7 KB) - added by Claude Paroz 4 years ago.
Forbid whitespaces in ipv6 addresses

Download all attachments as: .zip

Change History (14)

Changed 5 years ago by Claude Paroz

Attachment: 17751-test.diff added

Test showing issue

comment:1 Changed 5 years ago by Claude Paroz

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

comment:2 Changed 4 years ago by net147

Cc: net147 added

comment:3 Changed 4 years ago by federico.capoano@…

why not just trimming the eventual initial white space? I could try fixing it even though I don't have experience with contributing back to django. Is there any page where it is described how to do so?

comment:4 in reply to:  3 Changed 4 years ago by Igor Petrov

Replying to federico.capoano@…:

why not just trimming the eventual initial white space? I could try fixing it even though I don't have experience with contributing back to django. Is there any page where it is described how to do so?

This page can help you - https://docs.djangoproject.com/en/dev/internals/contributing/

Changed 4 years ago by Claude Paroz

Attachment: 17751-2.diff added

Forbid whitespaces in ipv6 addresses

comment:5 Changed 4 years ago by Claude Paroz

Has patch: set

comment:6 Changed 4 years ago by Erik Romijn

Owner: changed from Federico Capoano to Erik Romijn
Status: newassigned

comment:7 Changed 4 years ago by Erik Romijn

I strongly feel GenericIPAddressField should trim whitespace, and not raise a ValidationError. This is consistent with the behaviour of IntegerField, FloatField and EmailField, which also trim whitespace silently, and seems to be the nicest for usability. I see no risk for misinterpretation of the data when trimming.

The pull request in https://github.com/django/django/pull/757 will trim whitespace on both sides for all GenericIPAddressField instances (IPv4, IPv6 and generic).
For consistency, it also adds this feature to IPAddressField (they use the same validator anyways). This is backwards compatible.

comment:8 Changed 4 years ago by Joeri Bekker

Patch needs improvement: set

I concur with Erik's reasoning.

Patch is good but I would leave in the actual correct test (without a space) and add a separate test for an IP address with a space.

comment:9 Changed 4 years ago by Erik Romijn

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin
Version: 1.4-beta-1master

Agreed, new patch with that change: https://github.com/django/django/pull/759

comment:10 Changed 4 years ago by Erik Romijn <eromijn@…>

Resolution: fixed
Status: assignedclosed

In 21f333bcefccc151d6439246f8203d609ab6ca79:

Fix #17751: Added stripping of whitespace for IPAddressField/GenericIPAddressField

comment:11 Changed 4 years ago by Florian Apolloner <florian@…>

In 67c39a64e018ee32b9c020615babea9b3ac7215f:

Merge pull request #759 from erikr/master

Fix #17751: Added stripping of whitespace for IPAddressField/GenericIPAddressField

comment:12 Changed 4 years ago by Florian Apolloner <florian@…>

Note: See TracTickets for help on using tickets.
Back to Top