Opened 10 years ago
Closed 10 years ago
#24463 closed Cleanup/optimization (fixed)
Remove old functionality from `HttpRequest`
Reported by: | Rik | Owned by: | nobody |
---|---|---|---|
Component: | HTTP handling | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The scheme()
method on HttpRequest
returns the scheme the request was on, so http or https. If you look at the implementation of the method:
https://github.com/django/django/blob/master/django/http/request.py#L163
You see that it checks a Django setting and if that doesn't result in https, it falls back to the _get_scheme()
method.
Now if you look at the _get_scheme()
method:
https://github.com/django/django/blob/master/django/http/request.py#L159
You see it checks if the environment variable "HTTPS" is set to "on". I think is old functionality that doesn't belong in Django anymore. It was probably from the time mod_python was used. These days this functionality is implemented by WSGI:
https://www.python.org/dev/peps/pep-3333/#environ-variables
(search for HTTPS=on)
Also the WSGI handler in Django overwrites the _get_scheme()
:
https://github.com/django/django/blob/master/django/core/handlers/wsgi.py#L115
And since every HTTP request in Django goes through WSGI, the _get_scheme()
method in HttpRequest
would never be used.
So I think we should remove this functionality from HttpRequest
, to prevent confusion, because when I stumbled upon this piece of code it confused me quite a bit and it took me some time to find out what it might be.
Change History (10)
comment:1 by , 10 years ago
comment:2 by , 10 years ago
@collinanderson, what's the use case for creating HttpRequest
objects manually? And why would it be harder if we merge WSGIRequest
into HttpRequest
?
comment:3 by , 10 years ago
If you have some code running outside of a request that wants a request object, you can create a dummy request just by creating an HttpRequest(). It appears that WSGIRequest.init requires a dictionary to be passed in. Maybe that could be made optional?
It also looks like HttpRequest's GET and POST are mutable by default. (Again, that makes sense for hand-crafted HttpRequests.)
comment:4 by , 10 years ago
If you have code running outside a request that needs a request object, it seems to me like a flaw in the design of the program. However, I can understand there are people who have this anyway, and we should try to not break their code by changing this.
About the mutability of GET and POST, what do you mean by that? In Python, any property of a class is mutable.
comment:5 by , 10 years ago
I'm looking at HttpRequest.__init__
. By default, you'll get an error if you try to add or remove items from the request.GET and request.POST dict-likes.
comment:6 by , 10 years ago
Ok, in that case I guess we should keep WSGIRequest and HttpRequest separate. That might also make sense in the case WSGI might ever be replaced or alternatives arise.
Back to the core issue here: could we remove the checking of the environment variable in _get_scheme()
in HttpRequest
?
comment:7 by , 10 years ago
Hmm... It would be nice if HttpRequest().scheme worked without an error. Maybe we could simply have it return 'http'
and then have a comment saying it's overridden in WSGIRequest?
comment:8 by , 10 years ago
Yeah, that would make sense to me. It would prevent a lot of confusion. Will submit a pull request in a bit.
Personally, I think we should merge all of HttpRequest and WSGIRequest into one class, although currently it looks like HttpRequest is much easier to create requests objects manually.