#32698 closed Cleanup/optimization (fixed)
Move HttpRequest.get_raw_uri() to ExceptionReporter._get_raw_insecure_uri().
Reported by: | Adam Johnson | Owned by: | Hasan Ramezani |
---|---|---|---|
Component: | HTTP handling | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
get_raw_uri()
is undocumented, insecure alternative to build_absolute_uri(None)
. Django used it internally until ea542a9c7239b5b665797b7c809f1aceb0b412cf / #28007 (4 years ago). Since then it only exists in tests.
The only ticket mentioning get_raw_uri()
is #27506, recommending its use in the opbeat python integration.
GitHub code search reveals 1000 hits for `get_raw_uri`, but 40,000 hits for `build_absolute_uri`.
Users seem to be stumbling upon get_raw_uri()
, e.g. from IDE autocompletion, when they want build_absolute_uri()
.
I think we should deprecate it. Niche use cases, such as APM packages like opbeat, can read the internal attributes of HttpRequest
instead.
Change History (13)
comment:1 by , 4 years ago
comment:2 by , 4 years ago
We could also moved it to the ExceptionReporter._get_raw_insecure_uri()
🤔 and add request_insecure_uri
to the ExceptionReporter.get_traceback_data()
.
comment:3 by , 4 years ago
Ah yes, I forgot to grep templates.
Moving it to ExceptionReporter._get_raw_insecure_uri()
sounds good to me.
comment:4 by , 4 years ago
Easy pickings: | set |
---|---|
Summary: | Deprecate HttpRequest.get_raw_uri → Move HttpRequest.get_raw_uri() to ExceptionReporter._get_raw_insecure_uri(). |
Triage Stage: | Unreviewed → Accepted |
I don't think a deprecation is needed, it's a private API.
comment:6 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:7 by , 4 years ago
Has patch: | set |
---|
follow-up: 11 comment:10 by , 3 years ago
Is it possible to reconsider the decision to flat out removal of get_raw_uri
instead of going through a deprecation period?
While testing Django 4.0a1 on one of our projects we noticed (by way of failing tests) that one of our dependencies (authlib) currently relies on the existence of get_raw_uri
.
I created a PR in that project, so hopefully it'll be fixed before the release of Django 4.0.
Another tool we use (elastic-apm) guards the usage of get_raw_uri
and will fall back to pre-Django 1.9 behaviour if that method doesn't exist. Making it easy for the removal to go unnoticed. (I haven't made a PR there as I'm unsure on what the correct fix would be, and I'm not willing to go through the effort of signing their CLA)
comment:11 by , 3 years ago
Replying to Jaap Roes:
Is it possible to reconsider the decision to flat out removal of
get_raw_uri
instead of going through a deprecation period?
3rd-party dependencies are always an issue when upgrading, however get_raw_uri()
is undocumented and insecure API. I don't think there is any reason to keep it longer, even deprecated.
comment:12 by , 3 years ago
I agree with Mariusz. Those projects using it did so at their own risk, and are responsible for keeping up with Django. Report the problem to elastic-apm.
comment:13 by , 3 years ago
While it may not change the fact that this method has been undocumented and is not subject to a deprecation period, I think it is valuable context that we didn't simply stumble over get_raw_uri()
, but were pointed towards it by a core developer as a suitable alternative to build_absolute_uri()
(the mentioned "Opbeat" in that ticket is the predecessor of elastic APM).
That being said, we plan to implement a workaround on our side in time for Django 4.0, probably sooner.
Django still uses
get_raw_uri()
in technical 500 debug pages. We can rename it toget_raw_insecure_uri()
. What do think?