dcramer / django-uuidfield

A UUIDField for Django
BSD 3-Clause "New" or "Revised" License
262 stars 115 forks source link

PostgreSQL throws DataError when trying to retrieve objects with invalid UUID #46

Closed lingxiaoyang closed 9 years ago

lingxiaoyang commented 9 years ago

The following code will trigger the problem with PostgreSQL backend:

SomeModel.objects.get(uuid="invalid_uuid")

An example traceback:

/.../lib/python2.7/site-packages/django/db/backends/utils.pyc in execute(self, sql, params)
     63                 return self.cursor.execute(sql)
     64             else:
---> 65                 return self.cursor.execute(sql, params)
     66 
     67     def executemany(self, sql, param_list):

DataError: invalid input syntax for uuid: "invalid_uuid"
LINE 1: ... "some_model" WHERE "some_model"."uuid" = 'invalid_u...
                                                     ^

PostgreSQL has a UUID type and throws an error with badly formatted UUID. However, it is not the expected behavior of an API. When user provides an invalid UUID, the API in most cases is expected to return 404, instead of 500 caused by this uncaught exception.

The problem lies in method get_db_prep_value. If the provided UUID is not valid, the method should return an arbitrary valid UUID that matches nothing in the database.

ksonbol commented 9 years ago

I think raising a ValueError (as in https://github.com/dcramer/django-uuidfield/pull/48) is the correct behavior, not returning an arbitrary valid UUID that matches nothing in the database. If you are using the UUID in the URL, you could set the regex to allow only UUIDs with correct length. This will raise Http404. For example: url(r'^agencies/(?P<uuid>[^/]{32})/$', AgencyDetail.as_view(), name='details')

rychlis commented 9 years ago

One more thing to check: hon-hex characters in the string. UUIDs with correct lenght containing characters g-z will still cause a 500 error. Solution for the url:

url(r'^agencies/(?P<uuid>[A-Fa-f0-9]{32})/$', AgencyDetail.as_view(), name='details')

lingxiaoyang commented 9 years ago

@ksonbol You are right -- raising ValueError is supported by Django to return 404 response during lookup, which means special URL match is not needed. Django doc