Django-rest-framework: Backslashes added to paths at docs from `include_docs_urls`

[scardine]

Checklist

• [x] I have verified that that issue exists against the master branch of Django REST framework.
• [x] I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
• [x] This is not a usage question. (Those should be directed to the discussion group instead.)
• [x] This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
• [ ] I have reduced the issue to the simplest possible case.
• [ ] I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

Using DRF installed from github, Django 2.0.

Included the following at urls.py:

urlpatterns = [
    ...
    path('^api/token-auth/', authtoken_views.obtain_auth_token)
    path('^docs/', include_docs_urls(title='API Docs', authentication_classes=[], permission_classes=[], public=True)),
    ...
]

Expected behavior

Urls in docs should api/endpoint-name/

Actual behavior

Urls are api\/endpoint\-name/, when using the "interact" button you get a 404 (request URL is /api%5C/profiles/)

[scardine]

Downgrading to Django 1.11 fixed the problem. Would tests/test_renderers.py be the appropriate file to add a test for this issue?

[carltongibson]

Hi @scardine.

I need to look into this but I'm not sure it is a renderer issue. See @axnsan12's commit referencing this issue and #5672.

5609 (although distinct) is in a similar ballpark here.

[axnsan12]

More specifically, I think this should be addressed by Django in simplify_regex, which should, quote,

    r"""
    Clean up urlpattern regexes into something more readable by humans. For
    example, turn "^(?P<sport_slug>\w+)/athletes/(?P<athlete_slug>\w+)/$"
    into "/<sport_slug>/athletes/<athlete_slug>/".
    """

get_path_from_regex in drf makes use of that function.

I filed a bug in their tracker - https://code.djangoproject.com/ticket/28936

[carltongibson]

Good follow-up @axnsan12. Thanks.

[scardine]

Thanks, if it is not fixed by next week I will try to submit a patch for simplify_regex on my holiday's recess.

[carltongibson]

Note: Discussion on #5686 has details worth viewing.

[tiltec]

I think one should do str(pattern) before passing it to simplify_regex. In fact, you don't need to pass a RoutePattern into simplify_regex at all, as it's not a regex (but you may for code simplicity reasons).

The _bigger_ problem here seems not the escaping (can be dealt with calling str(pattern)), but that the result may contain type coercion tags.

Before in Django 1.x, the output of simplify_regex would always look like: /example/<id>/.
Whereas now, it can also be: /example/<int:id>/. Which may be unexpected for some follow-up code.

[scardine]

@tiltec I +1 your assessment.

Calling str(pattern) in Python 2.7 may trigger UnicodeError. BTW, are there any plans to do the same in DRF now that Django 2.0 dropped support for 2.7? Life is easier when you don't have to think about 2.7 compatibility.

Django 1.11 is the last version supporting 2.7 but it is an LTS version and support ends only in April 2020.

[tiltec]

Seems only tangentially related, but I wonder if there's a bug in Django 2.0 when using str(pattern) together with translatable URL patterns.
It clearly returns the gettext_lazy function instead of the translated string. Will set up a more comprehensive testcase for it...

(EDIT) Upstream bug filed: https://code.djangoproject.com/ticket/28947

[tiltec]

In Django 1.x, the output of simplify_regex would always look like: /example/<id>/.
Whereas now, it can also be: /example/<int:id>/. Which may be unexpected for some follow-up code.

I encountered a follow-up bug.

DRF uses uritemplate to parse the URL variables, but unfortunately the colon means something else there. RFC 6570 says that id:5 specifies a parameter with max length of 5.

Doesn't really fit int:id, where int is the _converter_ and id the parameter name. Therefore I get ValueError: invalid literal for int() with base 10: 'id' from uritemplate.

Maybe it's easiest to strip the converter part out for SchemaGenerator purposes?

[axnsan12]

I think the converter part could be useful for the typing information it could provide about the path parameter. Perhaps the most complete solution would be to try and parse it just like django does?

EDIT: or strip it and use the already parsed converters on the RoutePattern

[carltongibson]

3.7.4 will go tomorrow. I don’t know if a PR could be incoming by then, but it would make a great addition for the holidays if it could. 🙂

[tiltec]

@carltongibson PR is ready, I could work on it a bit more today if needed 😄

[carltongibson]

Ah, good work @tiltec! I will review tomorrow AM.

If others could comment that would help too!