Opened 21 months ago

Last modified 21 months ago

#26431 new Bug

translate_url() creates an incorrect URL when optional named groups are missing in the URL pattern

Reported by: EugeneM Owned by: nobody
Component: Core (URLs) Version: 1.9
Severity: Normal Keywords: translate_url
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

There is a problem when translating urls with absent 'optional' arguments
(it's seen in test case of the patch)

Attachments (2)

patch-stable-1.9.txt (2.2 KB) - added by EugeneM 21 months ago.
patch-master.txt (2.2 KB) - added by EugeneM 21 months ago.

Download all attachments as: .zip

Change History (4)

Changed 21 months ago by EugeneM

Attachment: patch-stable-1.9.txt added

Changed 21 months ago by EugeneM

Attachment: patch-master.txt added

comment:1 Changed 21 months ago by Tim Graham

Patch needs improvement: set
Summary: django.core.urlresolvers.translate_url fixtranslate_url() creates an incorrect URL when optional named groups are missing in the URL pattern
Triage Stage: UnreviewedAccepted

Could you please provide the patch as a pull request against master? Unless this is a regression, it probably doesn't qualify for a backport to 1.9 (see our supported versions policy). Please check the patch on Python 3 as well (dict.iteritems() doesn't exist there).

comment:2 Changed 21 months ago by Marten Kenbeek

The issue here is that resolve() will return None for any optional capture groups that aren't captured, but reverse() will use force_text() and convert it to the literal string 'None'. This causes an asymmetry between resolve() and reverse().

I think it's a better idea to treat this asymmetry as a bug, and solve it within reverse(). We would discard any arguments that are None. Incidentally this will allow for cleaner code when you don't know if an optional parameter is set when you call reverse() or the {% url %} template tag:

{% if obj.might_be_none != None %}
    {% url 'my_view' obj.might_be_none %}
{% else %}
    {% url 'my_view' %}
{% endif %}

vs.

{% url 'my_view' obj.might_be_none %}

I think that's a reasonably common usecase, so it would be nice to support this cleaner syntax. Non-existent template variables will still pass an empty string rather than None, so this won't hide any typos in your template variable names.

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