Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#29839 closed New feature (wontfix)

Make leading character in quote/unquote in contrib.admin.utils configurable

Reported by: Chris Z. Owned by: nobody
Component: contrib.admin Version: 2.1
Severity: Normal Keywords: admin quote unquote
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Chris Z.)

Important Note:

It's worth noting before reading all of this that apparently backporting these two commits will actually eliminate the need for this feature: https://github.com/django/django/commit/e9defb3f6e60b626e9ec40ff5df1322fceb52601 https://github.com/django/django/commit/e4df8e6dc021fa472fa77f9b835db74810184748

The problem:

The current leading character in the contrib.admin.utils quote() and unquote() functions (an underscore) causes some characters to be improperly escaped/unescaped if primary keys contain an underscore. This character could be configurable through a setting in settings.py, so that users who have underscores in text-based primary keys can change it. If there is a primary key with an underscore in it, for example, changing the leading character in the quote/unquote functions to a dot (.) will fix edit links in the django admin.

The current (django 2.1.1) functions, with suggested changes:

def quote(s):
    """
    Ensure that primary key values do not confuse the admin URLs by escaping
    any '/', '_' and ':' and similarly problematic characters.
    Similar to urllib.parse.quote(), except that the quoting is slightly
    different so that it doesn't get automatically unquoted by the Web browser.
    """
    if not isinstance(s, str):
        return s
    res = list(s)
    for i in range(len(res)):
        c = res[i]
        if c in """:/_#?;@&=+$,"[]<>%\n\\""":
            res[i] = '_%02X' % ord(c) # Make this underscore a configurable character
    return ''.join(res)


def unquote(s):
    """Undo the effects of quote(). Based heavily on urllib.parse.unquote()."""
    mychr = chr
    myatoi = int
    list = s.split('_') # Make this underscore a configurable character
    res = [list[0]]
    myappend = res.append
    del list[0]
    for item in list:
        if item[1:2]:
            try:
                myappend(mychr(myatoi(item[:2], 16)) + item[2:])
            except ValueError:
                myappend('_' + item) # Make this underscore a configurable character
        else:
            myappend('_' + item) # Make this underscore a configurable character
    return "".join(res)

A couple example models:

from django.db import models
from django.contrib.auth.models import User

# CUSTOMERS:
class Customer(models.Model):
    user = models.OneToOneField(User, on_delete=models.CASCADE)
    customer_id = models.CharField(max_length=100, primary_key=True)

    def __str__(self):
        return self.customer_id

# SUBSCRIPTIONS:
class Subscription(models.Model):

    subscription_id = models.CharField(max_length=100, primary_key=True)
    customer_id = models.ForeignKey('Customer', db_column='customer_id', on_delete=models.CASCADE)

    def __str__(self):
        return self.subscription_id

Steps to reproduce the issue:

  1. Create the above models.
  2. Create a Customer object with a primary key of cus_C2testing
  3. Create a Subscription object with a primary key of sub_C2testing and a ForeignKey back to the object created in step 1
  4. Navigate to the django admin and click the Subscription object you created. Notice that the underscore in the primary key has been replaced with _5F in the URL
  5. Click the edit customer icon (the small pencil next to the dropdown containing your customer object) and notice that the popup displays Customer with ID "cusÂtesting" doesn't exist. Perhaps it was deleted?. The C2 has been replaced with its unicode codepoint, Â. I believe this is due to the fact that the unquote function splits the ID at the underscore, but regardless of the cause, replacing the underscore in the quote/unquote functions at the locations in the code above with a dot (.) resolves the issue.

The suggestion:

My suggestion is that we make this configurable, so that the quote/unquote functions use an underscore by default, but take a second argument of a leading character if desired. It would ideally be configurable via settings.py in a variable like ADMIN_QUOTE_UNQUOTE_LEADCHAR. This will give users the flexibility to use primary keys with underscores, which are likely to be much more common than keys with dots.

Change History (3)

comment:1 by Chris Z., 5 years ago

Description: modified (diff)

comment:2 by Tim Graham, 5 years ago

Resolution: wontfix
Status: newclosed

I doubt there would be consensus to add the setting you described (and this issue doesn't qualify to be backported per our supported versions policy). Perhaps you could add a test to Django's test suite to demonstrate that the commits you mentioned fixed the issue. Thanks!

in reply to:  2 comment:3 by Chris Z., 5 years ago

Replying to Tim Graham:

I doubt there would be consensus to add the setting you described (and this issue doesn't qualify to be backported per our supported versions policy). Perhaps you could add a test to Django's test suite to demonstrate that the commits you mentioned fixed the issue. Thanks!

Thanks for the response Tim. For now I just backported the commits that are currently in the django master branch and they seem to have resolved the issue. I'm assuming (since I've seen that code in previous versions of django) that it'll be added into a future release and until then I'll just work off my custom fork. If they don't show up in a future release then I'll have to deal with it at that point.

Thanks again!

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