Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#17751 closed Bug (fixed)

GenericIPAddressField allows an ipv6 address to start with a space

Reported by: federico.capoano@… Owned by: erikr
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 claudep 3 years ago.
Test showing issue
17751-2.diff (1.7 KB) - added by claudep 3 years ago.
Forbid whitespaces in ipv6 addresses

Download all attachments as: .zip

Change History (14)

Changed 3 years ago by claudep

Test showing issue

comment:1 Changed 3 years ago by claudep

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

comment:2 Changed 3 years ago by net147

  • Cc net147 added

comment:3 follow-up: Changed 3 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 3 years ago by igor_s_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 3 years ago by claudep

Forbid whitespaces in ipv6 addresses

comment:5 Changed 3 years ago by claudep

  • Has patch set

comment:6 Changed 2 years ago by erikr

  • Owner changed from Federico Capoano to erikr
  • Status changed from new to assigned

comment:7 Changed 2 years ago by erikr

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 2 years ago by joeri

  • 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 2 years ago by erikr

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin
  • Version changed from 1.4-beta-1 to master

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

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

  • Resolution set to fixed
  • Status changed from assigned to closed

In 21f333bcefccc151d6439246f8203d609ab6ca79:

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

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

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