New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Object IDs with special characters #66

Closed
guettli opened this Issue Jun 21, 2011 · 4 comments

Comments

Projects
None yet
2 participants
@guettli
Contributor

guettli commented Jun 21, 2011

Hi,

object_id gets quoted by django admin. You need to unquote it, to get the result.

Here is a patch.

diff --git a/src/reversion/models.py b/src/reversion/models.py
index f412887..dc0be32 100644
--- a/src/reversion/models.py
+++ b/src/reversion/models.py
@@ -6,6 +6,7 @@ from django.contrib.contenttypes.models import ContentType
 from django.core import serializers
 from django.db import models, IntegrityError
 from django.db.models import Count
+from django.contrib.admin.util import unquote


 import reversion
@@ -83,7 +84,7 @@ class VersionManager(models.Manager):
     def get_for_object_reference(self, model, object_id):
         """Returns all versions for the given object reference."""
         content_type = ContentType.objects.get_for_model(model)
-        object_id = unicode(object_id)
+        object_id = unquote(unicode(object_id))
         versions = self.filter(content_type=content_type, object_id=object_id)
         versions = versions.order_by("pk")
         return versions
@@ -131,7 +132,7 @@ class VersionManager(models.Manager):
             select_related = tuple(select_related) + ("revision",)
         # Fetch the version.
         content_type = ContentType.objects.get_for_model(model_class)
-        object_id = unicode(object_id)
+        object_id = unquote(unicode(object_id))
         versions = self.filter(content_type=content_type, object_id=object_id)
         versions = versions.order_by("-pk")
         if select_related:
@guettli

This comment has been minimized.

Show comment
Hide comment
@guettli

guettli Jun 21, 2011

Contributor

I had this problem with a object which had a primary key with two underscores: foo__bar

Contributor

guettli commented Jun 21, 2011

I had this problem with a object which had a primary key with two underscores: foo__bar

@etianen

This comment has been minimized.

Show comment
Hide comment
@etianen

etianen Jul 7, 2011

Owner

Thanks for the report.

I don't think this is the right place for the fix. Instead, the object_id should be unquoted in the VersionAdmin view methods.

Could you update your patch to move the quoting to the VersionAdmin class?

Owner

etianen commented Jul 7, 2011

Thanks for the report.

I don't think this is the right place for the fix. Instead, the object_id should be unquoted in the VersionAdmin view methods.

Could you update your patch to move the quoting to the VersionAdmin class?

@guettli

This comment has been minimized.

Show comment
Hide comment
@guettli

guettli Aug 16, 2011

Contributor

Dear Dave,

I have updated the patch. The strange thing is this: django itself does not use its own quote/unquote every time. If you create a new object with a primary key with an underscore, the underscore is not quoted to _5F. But if you go back
to the list view and then use the link to edit the object, the underscore gets quoted. At least on django 1.3.

diff --git a/src/reversion/admin.py b/src/reversion/admin.py
index 9816992..1a0179b 100644
--- a/src/reversion/admin.py
+++ b/src/reversion/admin.py
@@ -6,7 +6,8 @@ from django.db import models, transaction
 from django.conf.urls.defaults import patterns, url
 from django.conf import settings
 from django.contrib import admin
-from django.contrib.admin import helpers
+from django.contrib.admin import helpers 
+from django.contrib.admin.util import unquote
 from django.contrib.contenttypes.generic import GenericInlineModelAdmin, GenericRelation
 from django.contrib.contenttypes.models import ContentType
 from django.core.urlresolvers import reverse
@@ -337,6 +338,7 @@ class VersionAdmin(admin.ModelAdmin):
     @reversion.revision.create_on_success
     def revision_view(self, request, object_id, version_id, extra_context=None):
         """Displays the contents of the given revision."""
+        object_id = unquote(object_id) # Underscores in primary key get quoted to "_5F"
         obj = get_object_or_404(self.model, pk=object_id)
         version = get_object_or_404(Version, pk=version_id, object_id=unicode(obj.pk))
         # Generate the context.
@@ -374,6 +376,7 @@ class VersionAdmin(admin.ModelAdmin):

     def history_view(self, request, object_id, extra_context=None):
         """Renders the history view."""
+        object_id = unquote(object_id) # Underscores in primary key get quoted to "_5F"
         opts = self.model._meta
         action_list = [{"revision": version.revision,
                         "url": reverse("%s:%s_%s_revision" % (self.admin_site.name, opts.app_label, opts.module_name), args=(version.object_id, version.id))}
Contributor

guettli commented Aug 16, 2011

Dear Dave,

I have updated the patch. The strange thing is this: django itself does not use its own quote/unquote every time. If you create a new object with a primary key with an underscore, the underscore is not quoted to _5F. But if you go back
to the list view and then use the link to edit the object, the underscore gets quoted. At least on django 1.3.

diff --git a/src/reversion/admin.py b/src/reversion/admin.py
index 9816992..1a0179b 100644
--- a/src/reversion/admin.py
+++ b/src/reversion/admin.py
@@ -6,7 +6,8 @@ from django.db import models, transaction
 from django.conf.urls.defaults import patterns, url
 from django.conf import settings
 from django.contrib import admin
-from django.contrib.admin import helpers
+from django.contrib.admin import helpers 
+from django.contrib.admin.util import unquote
 from django.contrib.contenttypes.generic import GenericInlineModelAdmin, GenericRelation
 from django.contrib.contenttypes.models import ContentType
 from django.core.urlresolvers import reverse
@@ -337,6 +338,7 @@ class VersionAdmin(admin.ModelAdmin):
     @reversion.revision.create_on_success
     def revision_view(self, request, object_id, version_id, extra_context=None):
         """Displays the contents of the given revision."""
+        object_id = unquote(object_id) # Underscores in primary key get quoted to "_5F"
         obj = get_object_or_404(self.model, pk=object_id)
         version = get_object_or_404(Version, pk=version_id, object_id=unicode(obj.pk))
         # Generate the context.
@@ -374,6 +376,7 @@ class VersionAdmin(admin.ModelAdmin):

     def history_view(self, request, object_id, extra_context=None):
         """Renders the history view."""
+        object_id = unquote(object_id) # Underscores in primary key get quoted to "_5F"
         opts = self.model._meta
         action_list = [{"revision": version.revision,
                         "url": reverse("%s:%s_%s_revision" % (self.admin_site.name, opts.app_label, opts.module_name), args=(version.object_id, version.id))}
@etianen

This comment has been minimized.

Show comment
Hide comment
@etianen

etianen Aug 20, 2011

Owner

Thanks! I've applied your patch, and everything still works! :)

Owner

etianen commented Aug 20, 2011

Thanks! I've applied your patch, and everything still works! :)

@etianen etianen closed this Aug 20, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment