Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#17751 closed Bug (fixed)

GenericIPAddressField allows an ipv6 address to start with a space

Reported by: federico.capoano@… Owned by: Sasha Romijn
Component: Forms Version: dev
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 12 years ago.
Test showing issue
17751-2.diff (1.7 KB ) - added by Claude Paroz 11 years ago.
Forbid whitespaces in ipv6 addresses

Download all attachments as: .zip

Change History (14)

by Claude Paroz, 12 years ago

Attachment: 17751-test.diff added

Test showing issue

comment:1 by Claude Paroz, 12 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

comment:2 by net147, 12 years ago

Cc: net147 added

comment:3 by federico.capoano@…, 11 years ago

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?

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

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/

by Claude Paroz, 11 years ago

Attachment: 17751-2.diff added

Forbid whitespaces in ipv6 addresses

comment:5 by Claude Paroz, 11 years ago

Has patch: set

comment:6 by Sasha Romijn, 11 years ago

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

comment:7 by Sasha Romijn, 11 years ago

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 by Joeri Bekker, 11 years ago

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 by Sasha Romijn, 11 years ago

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 by Erik Romijn <eromijn@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 21f333bcefccc151d6439246f8203d609ab6ca79:

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

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

In 67c39a64e018ee32b9c020615babea9b3ac7215f:

Merge pull request #759 from erikr/master

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

comment:12 by Florian Apolloner <florian@…>, 11 years ago

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