nshafer / django-hashid-field

Django Model Field that uses Hashids to obscure the value
MIT License
370 stars 40 forks source link

hashID issue on django-admin -> quote/unquote #84

Closed areski closed 7 months ago

areski commented 1 year ago

I had some views that stopped working on Django-admin, and realize Django-admin use an unquote function on URL.

After some investigatigation, I found https://github.com/django/django/blob/f7389c4b07ceeb036436e065898e411b247bca78/django/contrib/admin/utils.py#L22-L23

Here the full quote_map:

QUOTE_MAP = {i: "_%02X" % i for i in b'":/_#?;@&=+$,"[]<>%\n\\'}
Out[17]: 
{34: '_22',
 58: '_3A',
 47: '_2F',
 95: '_5F',
 35: '_23',
 63: '_3F',
 59: '_3B',
 64: '_40',
 38: '_26',
 61: '_3D',
 43: '_2B',
 36: '_24',
 44: '_2C',
 91: '_5B',
 93: '_5D',
 60: '_3C',
 62: '_3E',
 37: '_25',
 10: '_0A',
 92: '_5C'}

# Example of unquote on a hashID
In [8]: unquote('myprefix_24D8KLZLM6LKR')
Out[8]: 'myprefix$D8KLZLM6LKR'

Therefore if you have a hashID with a prefix eg. myprefix_ then followed by one of those 2 characters, Django-admin will convert the hashID to that character, and will be unable to retrieve the corresponding object.

We might want to prevent a hashID to start with those 2 characters to avoid this issue.

jackmpcollins commented 11 months ago

I encountered this issue today with id bv_att_3EZldmK. Django admin had error

... with ID “bv_att>ZldmK” doesn’t exist. Perhaps it was deleted?
nshafer commented 10 months ago

I've been able to replicate this, but I can't think of any way to fix it. It's really due to those strings, such as "_24" being specially handled in any primary key. This would be a problem with any library that creates primary keys with those strings in them. It's really unfortunate that Django uses something so simple. I found this bug in the tracker requesting a way to customize the character, but it was rejected. The only thing I can think of us to not end your prefix with "_" at all, which of course doesn't help if you've already published links with it.

This would affect other encodings, like the url-safe Base64 according to rfc4648 since it uses underscores, and there's a chance that it would include one of these reserved "_XX" patterns.

Ultimately I think this is a bug with Django's admin that should get fixed.