Code

Opened 7 years ago

Closed 11 months ago

Last modified 11 months ago

#5622 closed Bug (wontfix)

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

Reported by: anonymous Owned by: erikr
Component: Forms Version: master
Severity: Normal Keywords: sprintdec01
Cc: matiassurdi@…, stephane.raimbault@…, jschrantz, mike@…, net147, jodym@…, diepes, albrecht.andi@…, aashu_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 toke 6 years ago.
5622.1.patch (420 bytes) - added by toke 6 years ago.
Alternative Patch at newforms-level
5622.2.diff (1.5 KB) - added by msurdi 5 years ago.
Fix and Tests
empty-ip.diff (2.3 KB) - added by stephaner 4 years ago.
Updated to trunk (> Django 1.2.1)
empty-ip-2.diff (2.4 KB) - added by jfunk 4 years ago.
Version that does not break BooleanField

Download all attachments as: .zip

Change History (67)

comment:1 Changed 7 years ago by anonymous

  • Component changed from Uncategorized to Admin interface
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 follow-up: Changed 7 years ago by ubernostrum

  • Resolution set to invalid
  • Status changed from new to closed

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

  • Resolution invalid deleted
  • Status changed from closed to reopened

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 6 years ago by toke

  • Keywords sprintdec01 added
  • Resolution set to worksforme
  • Status changed from reopened to closed

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 follow-up: Changed 6 years ago by anonymous

  • Resolution worksforme deleted
  • Status changed from closed to reopened

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

comment:6 in reply to: ↑ 5 Changed 6 years ago by toke

  • Keywords sprintdec01, postgresql added; sprintdec01 removed

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 6 years ago by toke

  • Triage Stage changed from Unreviewed to Design 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 6 years ago by toke

See also #4684

comment:9 Changed 6 years ago by toke

  • Owner changed from nobody to toke
  • Status changed from reopened to new

Changed 6 years ago by toke

comment:10 Changed 6 years ago by toke

  • Has patch set

comment:11 Changed 6 years ago by toke

  • Component changed from Admin interface to Database wrapper

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

Maybe more such Bugs?

comment:12 Changed 6 years ago by anonymous

  • Owner changed from toke to anonymous

comment:13 Changed 6 years ago by brosner

  • Component changed from Database wrapper to django.newforms
  • Version changed from newforms-admin to SVN

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

Changed 6 years ago by toke

Alternative Patch at newforms-level

comment:14 Changed 6 years ago by toke

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 6 years ago by SmileyChris

  • Component changed from django.newforms to Database 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 5 years ago by msurdi

  • 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 5 years ago by SmileyChris

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

I'm using PostgresQL 8.3

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

Changed 5 years ago by msurdi

Fix and Tests

comment:19 Changed 5 years ago by msurdi

  • 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 4 years ago by msurdi

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

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 4 years ago by refringe

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

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

Changed 4 years ago by stephaner

Updated to trunk (> Django 1.2.1)

comment:24 Changed 4 years ago by stephaner

  • Cc stephane.raimbault@… added
  • Component changed from Database layer (models, ORM) to Forms
  • Keywords sprintdec01 added; sprintdec01, postgresql removed
  • Triage Stage changed from Design decision needed to Unreviewed

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

comment:25 Changed 4 years ago by lrekucki

  • Owner changed from anonymous to lrekucki
  • Status changed from new to assigned

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 4 years ago by lrekucki

  • Owner changed from lrekucki to nobody
  • Status changed from assigned to new

comment:27 Changed 4 years ago by lrekucki

  • Summary changed from empty ipaddress raises an error (invalid input syntax for type inet: "") to mpty ipaddress raises an error (invalid input syntax for type inet: "")
  • Triage Stage changed from Unreviewed to Accepted

comment:28 Changed 4 years ago by lrekucki

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

Changed 4 years ago by jfunk

Version that does not break BooleanField

comment:29 Changed 4 years ago by gonz

  • Cc gonz added

comment:30 Changed 4 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 4 years ago by SmileyChris

  • milestone set to 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 3 years ago by stephaner

This ticket has a 1.3 milestone...

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

comment:33 Changed 3 years ago by lrekucki

  • 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 3 years ago by gabrielhurley

  • Severity set to Normal
  • Type set to Bug

comment:35 Changed 3 years ago by jschrantz

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

comment:36 Changed 3 years ago by carbonXT

  • Cc mike@… added

comment:37 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

comment:38 Changed 3 years ago by net147

  • Cc net147 added

comment:39 Changed 2 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 2 years ago by anonymous

models.GenericIPAddressField is not in 1.2

comment:41 Changed 2 years ago by anonymous

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

comment:42 Changed 2 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 2 years ago by jodym@…

  • Cc jodym@… added

comment:44 Changed 2 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 2 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 2 years ago by aashu_dwivedi

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

comment:47 Changed 2 years ago by gonz

  • Cc gonz removed

comment:48 Changed 2 years ago by aashu_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 2 years ago by aashu_dwivedi

can we use the NullableWidget or blank_as_none? field as being discussed here https://code.djangoproject.com/ticket/4136,

Last edited 2 years ago by aashu_dwivedi (previous) (diff)

comment:50 Changed 2 years ago by aalbrecht

  • Cc albrecht.andi@… added

comment:51 Changed 2 years ago by aashu_dwivedi

  • Cc aashu_dwivedi added

comment:52 Changed 2 years ago by aashu_dwivedi

  • Owner changed from nobody to aashu_dwivedi
  • Status changed from new to assigned

comment:53 Changed 20 months ago by aashu_dwivedi

I think this can be merged with #4136

comment:54 follow-up: Changed 18 months 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 18 months 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 17 months 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 15 months ago by antonagestam

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 14 months ago by carbonXT

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 13 months ago by claudep

#6233 reported this issue related to fixtures deserialization.

comment:60 Changed 11 months ago by erikr

  • Owner changed from aashu_dwivedi to erikr

comment:61 Changed 11 months ago by erikr

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

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 11 months ago by aaugustin

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.