Opened 14 years ago

Closed 13 years ago

Last modified 13 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 14 years ago.
Test showing issue
17751-2.diff​ (1.7 KB ) - added by Claude Paroz 13 years ago.
Forbid whitespaces in ipv6 addresses

Download all attachments as: .zip

Change History (14)

by Claude Paroz, 14 years ago

Attachment: 17751-test.diff​ added

Test showing issue

comment:1 by Claude Paroz, 14 years ago

Triage Stage: Unreviewed → Accepted
Type: Uncategorized → Bug

comment:2 by net147, 13 years ago

Cc: net147 added

comment:3 by federico.capoano@…, 13 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, 13 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, 13 years ago

Attachment: 17751-2.diff​ added

Forbid whitespaces in ipv6 addresses

comment:5 by Claude Paroz, 13 years ago

Has patch: set

comment:6 by Sasha Romijn, 13 years ago

Owner: changed from Federico Capoano to Sasha Romijn
Status: new → assigned

comment:7 by Sasha Romijn, 13 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, 13 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, 13 years ago

Patch needs improvement: unset
Triage Stage: Accepted → Ready for checkin
Version: 1.4-beta-1 → master

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

comment:10 by Erik Romijn <eromijn@…>, 13 years ago

Resolution: → fixed
Status: assigned → closed

In 21f333bcefccc151d6439246f8203d609ab6ca79:

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

comment:11 by Florian Apolloner <florian@…>, 13 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@…>, 13 years ago

In 5ae0c933a8572c173401386f9037901e6b5fff2b:

Fixed errors introduced in 21f333bcefccc151d6439246f8203d609ab6ca79. Refs #17751

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