#22266 closed Bug (fixed)
CharField primary keys with underscores are (un)escaped differently in the admin pages
Reported by: | Owned by: | nott | |
---|---|---|---|
Component: | contrib.admin | Version: | 1.6 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Migrated from here: https://code.djangoproject.com/ticket/18381#comment:6
Although that bug was fixed 2 years ago, related issues seem still present in the latest stable Django release 1.6.2.
CharField primary keys with underscores are (un)escaped differently in the admin pages, and when reversing admin URLs with django.core.urlresolvers.reverse() or the {% url %} template tag.
The admin pages add "5F" after the underscore in when rendering URLs, but reverse() does not. As a result, links from custom pages into the admin break when the object's primary key contains underscores.
Steps to reproduce:
- Create a model with "id = models.CharField(max_length=100, primary_key=True)" or other similar text primary key field,
- Register the model to the admin site,
- Create a model instance with test_123 as the primary key,
- Visit the admin, and observe admin URLs: they are of the format "/admin/<app>/<model>/test_5F123/"
- Open the shell, and call reverse('admin:app_model_delete', args=test_123)
- The URLs returned are of the format "/admin/<app>/<model>/test_123/" - no 5F
- As a result, if these links are embedded in pages, they don't work - the admin page tries to decode the primary key, expects 5F, and does not find the object.
[trains] vic@vic ~/projects/trains (master *) $ ./manage.py shell Python 3.3.2+ (default, Oct 9 2013, 14:50:09) >>> from django.core.urlresolvers import reverse >>> reverse('admin:trains_route_change', args=('test_123',)) >>> '/trains/route/test_123/'
Change History (8)
comment:1 by , 11 years ago
comment:2 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 10 years ago
ModelAdmin.get_urls defines the urls like these:
url(r'^$', wrap(self.changelist_view), name='%s_%s_changelist' % info), url(r'^add/$', wrap(self.add_view), name='%s_%s_add' % info), url(r'^(.+)/history/$', wrap(self.history_view), name='%s_%s_history' % info), url(r'^(.+)/delete/$', wrap(self.delete_view), name='%s_%s_delete' % info), url(r'^(.+)/$', wrap(self.change_view), name='%s_%s_change' % info),
As long as we do not know the exact primary key regexp, we have to use (.+)
Consider we have 2 entries with the following PKs: "test_123" and "test_123/history". There is no way to distinguish change view for "test_123/history" and history view for "test_123".
That is why PK quoting was introduced. django.contrib.admin.utils contains 2 functions: quote and unquote. So reversing admin url should usually look like:
reverse('admin:app_model_change', args=(quote(pk),))
Quoting and unquoting doesn't change numerical PKs, that is why this problem haven't been noticed before.
By the way, there is a bug with quoting not being applied in add_view, but it can be easily fixed - that is why the first redirect doesn't work.
I suppose we are really lacking the documentation (https://docs.djangoproject.com/en/1.6/ref/contrib/admin/#overriding-vs-replacing-an-admin-template is evertyhing I could find).
There is no way to get rid of quoting if we leave (.+) url pattern for PK. So we can leave quoting/unquoting as is and fix the redirect in add_view.
Or we can make this pattern configurable, e.g.:
class ModelAdmin(BaseModelAdmin): [...] pk_regexp = ".+" [...] def get_pk_regexp(self): return self.pk_regexp
comment:7 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Might be related to #22223