Code

Opened 7 years ago

Closed 4 years ago

#5140 closed (worksforme)

url templatetag should raise exception on invalid path

Reported by: oliver.andrich@… Owned by: adrian
Component: Template system Version: master
Severity: Keywords: url templatetag
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I just stumbled across the fact, that the url templatetag silently accepts any path to a view - no matter if it exists or not. In the module django.template.defaulttags I have seen, that after processing the templatetag an URLNode is generatet and afterwards rendered.

    def render(self, context):
        from django.core.urlresolvers import reverse, NoReverseMatch
        args = [arg.resolve(context) for arg in self.args]
        kwargs = dict([(smart_str(k,'ascii'), v.resolve(context)) for k, v in self.kwargs.items()])
        try:
            return reverse(self.view_name, args=args, kwargs=kwargs)
        except NoReverseMatch:
            try:
                project_name = settings.SETTINGS_MODULE.split('.')[0]
                return reverse(project_name + '.' + self.view_name, args=args, kwargs=kwargs)
            except NoReverseMatch:
                return ''

The last try-catch-block which catches the second NoReverseMatch exception is causing the troubles. At least in my eyes.
I would find it much more convenient if I actually get the exception when I cause such an error. This would have saved my some time today, as I created such an error during my Django studies.

Attachments (2)

url_template_tag_raises_exception_on_invalid_path.patch (1.0 KB) - added by oliver.andrich@… 7 years ago.
A newbies patch for this ticket.
propagate_exception_if_debug.patch (729 bytes) - added by kcarnold 6 years ago.
Propagate the NoReverseMatch exception if the reverse lookup fails

Download all attachments as: .zip

Change History (10)

Changed 7 years ago by oliver.andrich@…

A newbies patch for this ticket.

comment:1 Changed 7 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to wontfix
  • Status changed from new to closed

Template tags should never raise exceptions. That must always fail quietly. That is a design feature and there is a lot of code and templates that rely on that (it saves in duplication in templates in a number of places, for example).

comment:3 Changed 6 years ago by orutherfurd <orutherfurd+django@…>

What about letting the exception percolate up when in DEBUG mode, or based on a config knob? It's a pity that this, otherwise very useful, tag silently returns nothing -- in my apps, it's always a mistake when lookup fails.

comment:4 Changed 6 years ago by kcarnold

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Agreed with orutherfurd. I just cleaned up my urls, and a quick-and-dirty patch for this bug is greatly helping me track down my mistakes.

My patch just propagates the exception if settings.TEMPLATE_DEBUG. Better solutions might be:

  1. Use another setting, something like FAIL_ON_TEMPLATE_ERRORS. As mtredinnick says, this might catch too much.
  2. Return an object that compares like a string (so you can use the result in an if tag), but raises an exception when rendering.

But these are only concerns for other template errors; url lookups failing is always a mistake. (I am willing to be proven wrong there.)

Changed 6 years ago by kcarnold

Propagate the NoReverseMatch exception if the reverse lookup fails

comment:5 Changed 6 years ago by SmileyChris

  • Has patch set
  • Triage Stage changed from Unreviewed to Design decision needed

comment:6 Changed 6 years ago by sorl

Correct me if I am wrong but the docs don't say Templae tags can't raise exceptions. I am also +1 to raise an error on NoReverseMatch. Debugging is almost impossible as is.

comment:7 Changed 6 years ago by sorl

Just quoting the docs:

render() should never raise TemplateSyntaxError or any other exception. It should fail silently, just as template filters should.

So this is the problem.

comment:8 Changed 4 years ago by Alex

  • Resolution set to worksforme
  • Status changed from reopened to closed

The URL tag is now noisy.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.