Opened 9 years ago

Closed 4 years ago

Last modified 4 years ago

#5622 closed Bug (wontfix)

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

Reported by: anonymous Owned by: Erik Romijn
Component: Forms Version: master
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 9 years ago.
5622.1.patch (420 bytes) - added by Thomas Kerpe 9 years ago.
Alternative Patch at newforms-level
5622.2.diff (1.5 KB) - added by Matias Surdi 8 years ago.
Fix and Tests
empty-ip.diff (2.3 KB) - added by Stephane Raimbault 7 years ago.
Updated to trunk (> Django 1.2.1)
empty-ip-2.diff (2.4 KB) - added by jfunk 6 years ago.
Version that does not break BooleanField

Download all attachments as: .zip

Change History (67)

comment:1 Changed 9 years ago by anonymous

Component: UncategorizedAdmin interface

comment:2 Changed 9 years ago by James Bennett

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.

comment:3 in reply to:  2 Changed 9 years ago by anonymous

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 Changed 9 years ago by Thomas Kerpe

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 Changed 9 years ago by anonymous

Resolution: worksforme
Status: closedreopened

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

comment:6 in reply to:  5 Changed 9 years ago by Thomas Kerpe

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 Changed 9 years ago by Thomas Kerpe

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 Changed 9 years ago by Thomas Kerpe

See also #4684

comment:9 Changed 9 years ago by Thomas Kerpe

Owner: changed from nobody to Thomas Kerpe
Status: reopenednew

Changed 9 years ago by Thomas Kerpe

Attachment: 5622.patch added

comment:10 Changed 9 years ago by Thomas Kerpe

Has patch: set

comment:11 Changed 9 years ago by Thomas Kerpe

Component: Admin interfaceDatabase wrapper

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

Maybe more such Bugs?

comment:12 Changed 9 years ago by anonymous

Owner: changed from Thomas Kerpe to anonymous

comment:13 Changed 9 years ago by Brian Rosner

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.

Changed 9 years ago by Thomas Kerpe

Attachment: 5622.1.patch added

Alternative Patch at newforms-level

comment:14 Changed 9 years ago by Thomas Kerpe

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 Changed 9 years ago by Chris Beaven

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 Changed 8 years ago by Matias Surdi

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 Changed 8 years ago by Chris Beaven

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 Changed 8 years ago by Matias Surdi

I'm using PostgresQL 8.3

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

Changed 8 years ago by Matias Surdi

Attachment: 5622.2.diff added

Fix and Tests

comment:19 Changed 8 years ago by Matias Surdi

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 Changed 7 years ago by Matias Surdi

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 Changed 7 years ago by Alex Gaynor

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 Changed 7 years ago by Tyler Brownell

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 Changed 7 years ago by imbaczek

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

Changed 7 years ago by Stephane Raimbault

Attachment: empty-ip.diff added

Updated to trunk (> Django 1.2.1)

comment:24 Changed 7 years ago by Stephane Raimbault

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 Changed 7 years ago by Łukasz Rekucki

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 Changed 7 years ago by Łukasz Rekucki

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

comment:27 Changed 7 years ago by Łukasz Rekucki

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 Changed 7 years ago by Łukasz Rekucki

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

Changed 6 years ago by jfunk

Attachment: empty-ip-2.diff added

Version that does not break BooleanField

comment:29 Changed 6 years ago by Gonzalo Saavedra

Cc: Gonzalo Saavedra added

comment:30 Changed 6 years ago by anonymous

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

comment:31 Changed 6 years ago by Chris Beaven

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 Changed 6 years ago by Stephane Raimbault

This ticket has a 1.3 milestone...

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

comment:33 Changed 6 years ago by Łukasz Rekucki

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 Changed 6 years ago by Gabriel Hurley

Severity: Normal
Type: Bug

comment:35 Changed 5 years ago by jschrantz

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

comment:36 Changed 5 years ago by Mike Fogel

Cc: mike@… added

comment:37 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

comment:38 Changed 5 years ago by net147

Cc: net147 added

comment:39 Changed 5 years ago by rihad

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

comment:40 Changed 5 years ago by anonymous

models.GenericIPAddressField is not in 1.2

comment:41 Changed 5 years ago by anonymous

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

comment:42 Changed 5 years ago by anonymous

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 Changed 5 years ago by jodym@…

Cc: jodym@… added

comment:44 Changed 5 years ago by anonymous

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 Changed 5 years ago by diepes

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 Changed 5 years ago by Ashutosh Dwivedi

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

comment:47 Changed 5 years ago by Gonzalo Saavedra

Cc: Gonzalo Saavedra removed

comment:48 Changed 5 years ago by Ashutosh Dwivedi

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 Changed 5 years ago by Ashutosh Dwivedi

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

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

comment:50 Changed 5 years ago by Andi Albrecht

Cc: albrecht.andi@… added

comment:51 Changed 5 years ago by Ashutosh Dwivedi

Cc: Ashutosh Dwivedi added

comment:52 Changed 5 years ago by Ashutosh Dwivedi

Owner: changed from nobody to Ashutosh Dwivedi
Status: newassigned

comment:53 Changed 4 years ago by Ashutosh Dwivedi

I think this can be merged with #4136

comment:54 Changed 4 years ago by 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 )

comment:55 Changed 4 years ago by peterkmurphy@…

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 Changed 4 years ago by kace <kace7@…>

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.

comment:57 in reply to:  54 Changed 4 years ago by Anton Agestam

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 Changed 4 years ago by Mike Fogel

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 Changed 4 years ago by Claude Paroz

#6233 reported this issue related to fixtures deserialization.

comment:60 Changed 4 years ago by Erik Romijn

Owner: changed from Ashutosh Dwivedi to Erik Romijn

comment:61 Changed 4 years ago by Erik Romijn

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 Changed 4 years ago by Aymeric Augustin

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

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