#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 )
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:
- Create the above models.
- Create a
Customer
object with a primary key ofcus_C2testing
- Create a Subscription object with a primary key of
sub_C2testing
and a ForeignKey back to the object created in step 1 - 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 - 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 , 6 years ago
Description: | modified (diff) |
---|
follow-up: 3 comment:2 by , 6 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:3 by , 6 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!
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!