Opened 17 years ago

Closed 11 years ago

Last modified 11 years ago

#5622 closed Bug (wontfix)

Empty ipaddress raises an error (invalid input syntax for type inet: "")

Reported by: anonymous Owned by: Sasha Romijn
Component: Forms Version: dev
Severity: Normal Keywords: sprintdec01
Cc: matiassurdi@…, stephane.raimbault@…, jschrantz, mike@…, net147, jodym@…, diepes, albrecht.andi@…, Ashutosh Dwivedi Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

That is - the admin interface raises an exception when saving an object with an empty IPAddressField

Attachments (5)

5622.patch (524 bytes ) - added by Thomas Kerpe 16 years ago.
5622.1.patch (420 bytes ) - added by Thomas Kerpe 16 years ago.
Alternative Patch at newforms-level
5622.2.diff (1.5 KB ) - added by Matias Surdi 15 years ago.
Fix and Tests
empty-ip.diff (2.3 KB ) - added by Stephane Raimbault 14 years ago.
Updated to trunk (> Django 1.2.1)
empty-ip-2.diff (2.4 KB ) - added by jfunk 14 years ago.
Version that does not break BooleanField

Download all attachments as: .zip

Change History (67)

comment:1 by anonymous, 17 years ago

Component: UncategorizedAdmin interface

comment:2 by James Bennett, 17 years ago

Resolution: invalid
Status: newclosed

Sounds like you've perhaps set blank=True on the field and forgotten to supply a fallback value elsewhere in your code. Please ask -- with detailed information -- on the django-users mailing list.

in reply to:  2 comment:3 by anonymous, 17 years ago

Resolution: invalid
Status: closedreopened

Replying to ubernostrum:

Sounds like you've perhaps set blank=True on the field and forgotten to supply a fallback value elsewhere in your code. Please ask -- with detailed information -- on the django-users mailing list.

the model definition has this field:

reported_ip = models.IPAddressField(blank=True, null=True, default=None)

The problem is that when i edit an object containing this field, in the admin site, i want to be able to leave this field empty. I'm only working with models definition and I don't have other code.

comment:4 by Thomas Kerpe, 16 years ago

Keywords: sprintdec01 added
Resolution: worksforme
Status: reopenedclosed

I can not reproduce this with newforms-admin [6783].

class Host(models.Model):
    name = models.CharField(maxlength=100)
    ip = models.IPAddressField(blank=True, null=True, default=None)

Create, Update, Delete.
Maybe the Database-Field is "NOT NULL"?

comment:5 by anonymous, 16 years ago

Resolution: worksforme
Status: closedreopened

It's an Postgresql issue.
When using postgresql_psycopg2 the described error will occour.
Not in sqlite3!

in reply to:  5 comment:6 by Thomas Kerpe, 16 years ago

Keywords: postgresql added

Replying to anonymous:

It's an Postgresql issue.
When using postgresql_psycopg2 the described error will occour.
Not in sqlite3!

That was me.

comment:7 by Thomas Kerpe, 16 years ago

Triage Stage: UnreviewedDesign decision needed

Postgresql inet type don't allow empty inet fields. Null is OK.
Should an empty string in the admin be converted to a NULL value in the DB?

http://www.postgresql.org/docs/7.4/interactive/datatype-net-types.html#DATATYPE-INET

comment:8 by Thomas Kerpe, 16 years ago

See also #4684

comment:9 by Thomas Kerpe, 16 years ago

Owner: changed from nobody to Thomas Kerpe
Status: reopenednew

by Thomas Kerpe, 16 years ago

Attachment: 5622.patch added

comment:10 by Thomas Kerpe, 16 years ago

Has patch: set

comment:11 by Thomas Kerpe, 16 years ago

Component: Admin interfaceDatabase wrapper

This Problem applies to whole newforms. In oldforms this is AFAIK handled by html2python.

Maybe more such Bugs?

comment:12 by anonymous, 16 years ago

Owner: changed from Thomas Kerpe to anonymous

comment:13 by Brian Rosner, 16 years ago

Component: Database wrapperdjango.newforms
Version: newforms-adminSVN

This looks to be a bug in newforms and not only newforms-admin. Moving it out of newforms-admin.

by Thomas Kerpe, 16 years ago

Attachment: 5622.1.patch added

Alternative Patch at newforms-level

comment:14 by Thomas Kerpe, 16 years ago

The alternative Patch solves the same issue as the first one.

IMHO it's a issue at the database layer (the data representation in the database) so I would tend to fix such things at db-layer.

comment:15 by Chris Beaven, 16 years ago

Component: django.newformsDatabase wrapper
Needs tests: set

Needs some tests to show the problem being solved. On the surface, it looks like this should work, since IPAddressField has empty_strings_allowed = False

I concur that it should be fixed at the db level.

comment:16 by Matias Surdi, 15 years ago

Cc: matiassurdi@… added

I've just found this issue too with an IPAddressField. The patch 5622.1.patch seems to solve the problem correctly, at least for me.

Will this be merged into trunk at some point?

comment:17 by Chris Beaven, 15 years ago

Msurdi - which database backend are you using?

Would you like to take a shot at writing some tests - that'll speed up the chance of it getting merged.

comment:18 by Matias Surdi, 15 years ago

I'm using PostgresQL 8.3

And I'm working on the fix and the tests.....

by Matias Surdi, 15 years ago

Attachment: 5622.2.diff added

Fix and Tests

comment:19 by Matias Surdi, 15 years ago

Needs tests: unset

I've just attached the fix and the tests.

Please, let me know if you find something is wrong to fix it. I'd love to get this into trunk before 1.1.

Many thanks.

comment:20 by Matias Surdi, 14 years ago

Hi guys.... I'm hitting this bug/problem again..... and it seems that the original patch is not present on Trunk.

Is it possible that somebody forgot to include the patch?

Thanks...

comment:21 by Alex Gaynor, 14 years ago

Based on the fact that this ticket has never been closed and marked as fixed, yes I would say this patch isn't in trunk.

comment:22 by Tyler Brownell, 14 years ago

I'm still having this issue using the latest trunk. Is there a reason this has not been committed? Is there anything anyone can do to move this forward?

comment:23 by imbaczek, 14 years ago

what's the status on this issue? i've just hit it and it's a bit baffling.

by Stephane Raimbault, 14 years ago

Attachment: empty-ip.diff added

Updated to trunk (> Django 1.2.1)

comment:24 by Stephane Raimbault, 14 years ago

Cc: stephane.raimbault@… added
Component: Database layer (models, ORM)Forms
Keywords: postgresql removed
Triage Stage: Design decision neededUnreviewed

Could you have a look at this patch (opened since 3 years), please?

comment:25 by Łukasz Rekucki, 14 years ago

Owner: changed from anonymous to Łukasz Rekucki
Status: newassigned

There's probably more chance for review is this is marked as accepted (as it's obviously a bug). If someone really needs this in trunk, you can always try django-developers and make a deal with one of core devs.

comment:26 by Łukasz Rekucki, 14 years ago

Owner: changed from Łukasz Rekucki to nobody
Status: assignednew

comment:27 by Łukasz Rekucki, 14 years ago

Summary: empty ipaddress raises an error (invalid input syntax for type inet: "")mpty ipaddress raises an error (invalid input syntax for type inet: "")
Triage Stage: UnreviewedAccepted

comment:28 by Łukasz Rekucki, 14 years ago

Summary: mpty ipaddress raises an error (invalid input syntax for type inet: "")Empty ipaddress raises an error (invalid input syntax for type inet: "")

by jfunk, 14 years ago

Attachment: empty-ip-2.diff added

Version that does not break BooleanField

comment:29 by Gonzalo Saavedra, 14 years ago

Cc: Gonzalo Saavedra added

comment:30 by anonymous, 14 years ago

Same problem as I've duplicated here by mistake. No milestone set in 3 years? This looks promising :)

comment:31 by Chris Beaven, 14 years ago

milestone: 1.3

How about we slate it for 1.3 then. Get some discussion going in django-dev group to get things rolling.

comment:32 by Stephane Raimbault, 13 years ago

This ticket has a 1.3 milestone...

Smiley, have you received feedback on this bug from django-dev?

comment:33 by Łukasz Rekucki, 13 years ago

Patch needs improvement: set

IMHO, Current patch fixes the problem on the wrong level (It shows however, that the validation part should be moved to the model field, now that we have model validation). forms.IPAddressField should return a string that's a valid IPv4 address, fail or return None (not "", like it's documented right now).

comment:34 by Gabriel Hurley, 13 years ago

Severity: Normal
Type: Bug

comment:35 by jschrantz, 13 years ago

Cc: jschrantz added
Easy pickings: unset
UI/UX: unset

comment:36 by Mike Fogel, 13 years ago

Cc: mike@… added

comment:37 by Jacob, 13 years ago

milestone: 1.3

Milestone 1.3 deleted

comment:38 by net147, 12 years ago

Cc: net147 added

comment:39 by rihad, 12 years ago

In the mean time, we can use models.GenericIPAddressField. It's still Postgres type inet, and it seems to just work.

comment:40 by anonymous, 12 years ago

models.GenericIPAddressField is not in 1.2

comment:41 by anonymous, 12 years ago

is there a way to overwrite django.db.backends.postgresql.creation?

comment:42 by anonymous, 12 years ago

this is very ugly but it worked for me:

class IPAddressField(models.IPAddressField):

max_length = 15
def get_internal_type(self):

return "CharField"

comment:43 by jodym@…, 12 years ago

Cc: jodym@… added

comment:44 by anonymous, 12 years ago

Will a decision be made on how to fix this bug? I vote for a null=True, convert empty strings to null solution, for what it's worth.

comment:45 by diepes, 12 years ago

Cc: diepes added

Bumped into same problem using sqlight3 in django 1.4

For me the solution is default="0.0.0.0"

comment:46 by Ashutosh Dwivedi, 12 years ago

@diepes Just check with django (1, 5, 0, 'alpha', 0) for sqlite3 not able to reproduce it there.

comment:47 by Gonzalo Saavedra, 12 years ago

Cc: Gonzalo Saavedra removed

comment:48 by Ashutosh Dwivedi, 12 years ago

can we really use empty_strings_allowed? from what i can see the only use of empty_strings_allowed is to check

      if (self.empty_strings_allowed and
              connection.features.interprets_empty_strings_as_nulls):
              self.null = True

which is the case for oracle.We can have blank values for integer field in the db even if empty_strings_allowed is set to false for it.

comment:49 by Ashutosh Dwivedi, 12 years ago

can we use the NullableWidget as being discussed here https://code.djangoproject.com/ticket/4136

Version 0, edited 12 years ago by Ashutosh Dwivedi (next)

comment:50 by Andi Albrecht, 12 years ago

Cc: albrecht.andi@… added

comment:51 by Ashutosh Dwivedi, 12 years ago

Cc: Ashutosh Dwivedi added

comment:52 by Ashutosh Dwivedi, 12 years ago

Owner: changed from nobody to Ashutosh Dwivedi
Status: newassigned

comment:53 by Ashutosh Dwivedi, 12 years ago

I think this can be merged with #4136

comment:54 by kace, 11 years ago

For users facing this irritating issue, there is a quick, effective work-around (based on one of toke's patches (thank you)). Put this into "models.py":

class IPAddressFieldNullable(models.IPAddressField) :
    def get_db_prep_save(self, value):
        return value or None

Then change your IP attribute to use this modified field. E.g.:

-   someip = models.IPAddressField(blank=True, null=True )
+   someip = IPAddressFieldNullable(blank=True, null=True )

comment:55 by peterkmurphy@…, 11 years ago

Thanks, kace - but this has added a new error with Mezzanine. (Mind you, it's with a PostGIS enabled database as well.)

TypeError at /admin/eventjunkie/ejeventcomment/add/

get_db_prep_save() got an unexpected keyword argument 'connection'

Request Method: 	POST
Request URL: 	http://eventjunkie.pkmurphy.com.au/admin/eventjunkie/ejeventcomment/add/
Django Version: 	1.4.1
Exception Type: 	TypeError
Exception Value: 	

get_db_prep_save() got an unexpected keyword argument 'connection'

Exception Location: 	/home/daoosaigon/envs/mezz64nosite/lib/python2.7/site-packages/django/db/models/sql/compiler.py in as_sql, line 872
Python Executable: 	/home/daoosaigon/envs/mezz64nosite/bin/python

comment:56 by kace <kace7@…>, 11 years ago

peterk, all I can tell you is it works very well for me. Probably, you have to point the finger at Mezzanine. (... In any case, this really isn't right place to go into troubleshooting details!) Best of luck.

in reply to:  54 comment:57 by Anton Agestam, 11 years ago

Replying to kace:

For users facing this irritating issue, there is a quick, effective work-around (based on one of toke's patches (thank you)). Put this into "models.py":

class IPAddressFieldNullable(models.IPAddressField) :
    def get_db_prep_save(self, value):
        return value or None

Then change your IP attribute to use this modified field. E.g.:

-   someip = models.IPAddressField(blank=True, null=True )
+   someip = IPAddressFieldNullable(blank=True, null=True )

Thanks for quick fix, confirmed working. Will this ever be fixed?

comment:58 by Mike Fogel, 11 years ago

Extending the 'blank' kwarg as described here https://code.djangoproject.com/ticket/4136#comment:24 would cover this case as well by doing:

some_ip = models.IPAddressField(blank=models.SET_NULL, null=True)

comment:59 by Claude Paroz, 11 years ago

#6233 reported this issue related to fixtures deserialization.

comment:60 by Sasha Romijn, 11 years ago

Owner: changed from Ashutosh Dwivedi to Sasha Romijn

comment:61 by Sasha Romijn, 11 years ago

Resolution: wontfix
Status: assignedclosed

I can confirm this issue exists with IPAddressField. The problem is that the both IPAddressField and GenericIPAddressField are backed by string types on most databases, but by INET on PostgreSQL. That explains why storing a blank value works with most databases, but breaks with PostgreSQL.

This issue does not affect GenericIPAddressField, because in get_db_prep_value, it does: return value or None - as kace already suggested. An empty string evaluates to false, the value returned is None, therefore it will be stored as NULL in the database - a valid value for PostgreSQL's INET.

The obvious fix here is to use the same get_db_prep_value in IPAddressField. However, this breaks backwards compatibility. Anyone who is running a database other than PostgreSQL, will currently have these blank values stored as an empty string. After this fix, they will suddenly be stored as NULL in all databases. This means existing queries may also break - querying for an empty string is different from querying for NULL.

The simple workaround is to use GenericIPAddressField in all cases, which can be made to behave similar to IPAddressField, as you can control the protocols it will accept. Considering that fixing this will break backwards compatibility for some people, and a drop-in workaround is available, I think we should not resolve the issue. Therefore, I am closing the ticket as wontfix. Overall, maybe we should even deprecate IPAddressField, as it can be completely replaced by GenericIPAddressField.

Regarding the failure peterkmurphy saw with mezzanine, the issue is not caused by mezzanine - it's the incorrect signature for get_db_prep_value.
The correct implementation would be:

    def get_db_prep_value(self, value, connection, prepared=False):
        if not prepared:
            value = self.get_prep_value(value)
        return value or None

comment:62 by Aymeric Augustin, 11 years ago

erik's analysis is sound, I confirm the wontfix.

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