Reversing URLs in response middleware or streamed responses won't use per-request urlconf

[Aymeric Augustin]

Theoretically, after ​the URLconf is reset, reversing URLs isn't possible any longer.

This code was introduced in order to fix #5034.

[Anssi Kääriäinen]

Marking as accepted, the reporter seems trustworthy...

[Aymeric Augustin]

I haven't actually tested it, I filed this ticket while I was focusing on another problem in the area.

[Anssi Kääriäinen]

Not directly related, but...

While I was looking at this issue I noticed that we construct a *new* instance of the resolver instead of using get_urlconf() which would give us an existing instance. Seems like we are doing some extra work because of this (likely not much). This might be intentional, and it might be that we aren't actually doing that much extra work. I find the urlresolver code somewhat hard to follow, so I am not sure if there is a problem here or not.

As for this ticket: yes, it looks like if you will do reverse/resolve in the generator of a streaming response it will use the default urlconf instead of the request's urlconf. I wonder if that will be an issue in practice. We could likely defer the reset of urlconf into the .close() of the request.

[Carl Meyer]

#19784 reports the same problem wrt response middleware.

I don't think it's quite accurate to say that "reversing URLs isn't possible any longer"; it just won't use a per-request custom URLconf.

[Florian Apolloner]

So what do we wanna do? Just remove the reset or do it in the close of the Response?

[Carl Meyer]

Removing the reset is not an option, wouldn't the following request in that thread then use the previous request's custom URLconf, which might be incorrect?

[Aymeric Augustin]

The URLconf is set to settings.ROOT_URLCONF at the beginning of every request:
​https://github.com/django/django/blob/1e4a27d08790c96f657d2e960c8142d1ca69aede/django/core/handlers/base.py#L80

And it's set to None at the end of every request:
​https://github.com/django/django/blob/1e4a27d08790c96f657d2e960c8142d1ca69aede/django/core/handlers/base.py#L179

(The try and the finally clauses highlighted in these two links go together.)

---

I think Django should just stop resetting the URLconf at the end of every request:

• it'll be reset at the very beginning of the next request anyway (even before request middleware runs),
• this code was part of the original commit (#5034, [6c61ca3d]) rather than introduced to fix a bug,
• it does cause bugs for streaming responses,
• the rationale given in the comment is rather weak.

So, I'd like to remove this try/finally block — and deindent the 100 lines inside. I'm not attaching a patch because the deindent would cause too much noise.

[Aymeric Augustin]

Related quote from #django-dev:

28-21:32:28      <subleq> I have had problems with that set_urlconf(None)
28-21:32:42      <subleq> I needed to reverse urls in a response middleware

[loic84]

PR ​https://github.com/django/django/pull/885

[Aymeric Augustin]

#16444 was a duplicate.

[Loic Bistuer <loic.bistuer@…>]

In 521765f63d724f0a1becb614530170f5d00fc6a9:

Fixed #19541 -- Fixed BaseHandler to enable reversing URLs in response middlewares
and streamed responses with respect to per-request urlconf.

[Jannis Leidel <jannis@…>]

In 24234555a01f3ebff2e969210024c86842896671:

Merge pull request #885 from loic/ticket19541

Fixed #19541 -- Fixed BaseHandler to enable reversing URLs in response middlewares...