Skip to content
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

HyperlinkedRelatedField exception on string with %20 instead of space #4748

Closed
tomkoy opened this issue Dec 11, 2016 · 6 comments · Fixed by #7059
Closed

HyperlinkedRelatedField exception on string with %20 instead of space #4748

tomkoy opened this issue Dec 11, 2016 · 6 comments · Fixed by #7059
Labels
Milestone

Comments

@tomkoy
Copy link

tomkoy commented Dec 11, 2016

Use serializers.HyperlinkedRelatedField where defined lookup_field link to the field string causes an exception 'Invalid hyperlink - Object does not exist.' when the value of that field contains characters that are encoded in the url. eg space -> %20

Missing unquote when try get the value from the url

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 12, 2016

Looks legit as https://github.com/tomchristie/django-rest-framework/blob/master/rest_framework/generics.py#L96 doesn't unquote the data from url.

@tomchristie
Copy link
Member

tomchristie commented Dec 12, 2016

I'm rather surprised - I'd have fully expected that percent encoding is handled for you by Django's URL routing. A failing test case here might be a good first pass.

@rooterkyberian
Copy link
Contributor

rooterkyberian commented Apr 14, 2017

@xordoquy what you linked seems unrelated, as the error mentioned here is most likely raised from:

self.fail('does_not_exist')

The view with custom lookup field doesn't seem to cause any trouble by itself.

So why does resolve it work in view, but not in related field?
'%20' in path gets unquoted quite early on, already in WSGI server (or django test client). This is seems not exactly well defined part of WSGI, so some servers may not unquote this, but django builtin server does, and so is uwsgi and few others. Anyway, this is just explanation why it does seem to work when working with path from taken request object.

In related field we deal with user provided data, which is not parsed by any magical WSGI implementation. So I think we should call uri_to_iri (just like django test server) on urlparse.urlparse(data).path.

@hexvolt
Copy link

hexvolt commented Nov 19, 2018

@tomchristie @rooterkyberian @xordoquy it seems to be broken again since the Django's uri_to_iri() function has changed in Django2.0+

See what they've changed: django/django@03281d8#diff-d3fe562051dcbab80f60f067fa6a5838R180

I.e. now, %20 stays in the URL as opposed to how it worked before. Therefore, HyperlinkedRelatedField doesn't work with hyperlinks containing spaces%20 and returns "Object does not exist".

And the unit test in DRF passes because here #5179 it doesn't cover the %20 case. Thus, HyperlinkedRelatedField appear to be not fully compatible with Django 2.0+

@rpkilby
Copy link
Member

rpkilby commented Nov 19, 2018

Thanks @hexvolt.

@rpkilby rpkilby reopened this Nov 19, 2018
@auvipy
Copy link
Contributor

auvipy commented Nov 6, 2019

@rpkilby if you don't mind, can you include this on 3.11.x milestone? the milestone tag seems wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants