Missing call to super in `WSGIRequest.__init__()`

[loic84]

We can't just call the constructor, because setting the GET, POST, etc. in HttpRequest.__init__() will clash with the @property definitions inWSGIRequest.

That said, we need to ensure that future additions to HttpRequest.__init__() will also find their way toWSGIRequest.

[loic84]

I've got two solutions to offer:

1. The heavy machinery which enables WSGIRequest to call super in __init__():

​https://github.com/loic/django/compare/ticket20619

2. A manual fix for the missing var initialization and a comment to hopefully prevent this from happening again.

​https://github.com/loic/django/compare/ticket20619_take2

[Marc Tamlyn]

+1 to approach 1, but not confident enough in this area of the code base to merge it

[Florian Apolloner]

I am not that happy with approach one since it:

• Adds stuff to HttpRequest which it doesn't need at all since it only uses dicts
• Introduces an extra function call to every GET/POST/etc access

That said, I don't have any better ideas.

[loic84]

To be honest, I'm not totally happy showing up on git blame for such a hack, that said it's the most compatible fix I've found.

Do we really need WSGIRequest to use properties? The comment in HttpRequest.encoding seems to hint that we do, but I can't figure out when it comes into play. Anyone familiar with this area of Django?

[loic84]

I came up with a 3rd option:

A refactor of HttpRequest and WSGIRequest that removes all hacks specific to WSGIRequest from HttpRequest:

​https://github.com/loic/django/compare/ticket20619_take3

That's my favorite option by a big margin.

[Tom Christie]

@loic84 - It looks to me like that would change behavior. The lazy evaluation of request.POST/request.FILES is important for cases such as setting request.upload_handlers, as well as the effecting the behavior of accessing request.read(). That change looks like it'd cause request.POST/request.FILES to be loaded if request.GET is accessed, which is problematic. In other words, accessing request.GET shouldn't cause the request stream to be consumed and parsed.

[loic84]

@tomchristie: You are absolutely right. I added another commit to the same branch that should address this issue.

[loic84]

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

This should be enough to drop the priority on this ticket in view of 1.6b1. Note that the commit message won't close the ticket, we'll still need a long term solution for the missing call to super.

While I'm at it, I'll mention that ​https://github.com/loic/django/compare/ticket20619_take3 addresses two outstanding issues:

• We initialize a mutable at the class level in HttpRequest._upload_handlers.
• WSGIRequest.REQUEST would go stale if you access it and then change the encoding.

[Loic Bistuer <loic.bistuer@…>]

In 48ce167d895b7e2a9d00884a4a8679851fa890af:

Fixed missing initializations in WSGIRequest. Refs #20619

[Andrew Godwin <andrew@…>]

In b21e96d00d12f8467d61b15a943994b303504e0e:

Merge pull request #1311 from loic/ticket20619_take2

Fixed missing initializations in WSGIRequest. Refs #20619

[loic84]

Rebased ​https://github.com/loic/django/compare/ticket20619_take3 to account for [48ce167d895b7e2a9d00884a4a8679851fa890af].

[loic84]

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

[Jannis Leidel]

I've talked to loic84 on IRC and think there is no evidence that refactoring this piece of code will lead to improving the code. The call to super may be missing, but there is no feature on the horizon to my knowledge (and to loic84's knowledge) that would require it.

Given the central role the HttpRequest class plays in *any application* I'm closing this ticket as invalid and ask to reopen it if an actual use case presents itself to make a sensible decision of the usefulness of this refactor.

TLDR; uneeded code churn with high risk

[Tom Christie]

@jezdez - Yup, seems like a good call to me. Thanks.