[Django] #32698: Deprecate HttpRequest.get_raw_uri
Django
------------------------------------------------+------------------------
Reporter: Adam Johnson | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: HTTP handling | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
`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
[https://github.com/search?l=Python&q=%22request.get_raw_uri%22&type=Code
1000 hits for `get_raw_uri`], but
[https://github.com/search?l=Python&q=%22request.build_absolute_uri%22&type=Code
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32698>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: nobody
Cleanup/optimization |
Component: HTTP handling | Version: dev
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
Django still uses `get_raw_uri()` in technical 500 debug pages. We can
rename it to `get_raw_insecure_uri()`. What do think?
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: nobody
Cleanup/optimization |
Component: HTTP handling | Version: dev
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
We could also moved it to the `ExceptionReporter._get_raw_insecure_uri()`
🤔 and add `request_insecure_uri` to the
`ExceptionReporter.get_traceback_data()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: nobody
Cleanup/optimization |
Component: HTTP handling | Version: dev
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Adam Johnson):
Ah yes, I forgot to grep templates.
Moving it to `ExceptionReporter._get_raw_insecure_uri()` sounds good to
me.
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:3>
Django
ExceptionReporter._get_raw_insecure_uri().
--------------------------------------+------------------------------------
Reporter: Adam Johnson | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: HTTP handling | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by Mariusz Felisiak):
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
I don't think a deprecation is needed, it's a private API.
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:4>
Django
ExceptionReporter._get_raw_insecure_uri().
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: HTTP handling | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Comment (by Adam Johnson):
Yes that's fair.
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:5>
Django
ExceptionReporter._get_raw_insecure_uri().
Reporter: Adam Johnson | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: HTTP handling | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Hasan Ramezani):
* owner: nobody => Hasan Ramezani
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:6>
Django
ExceptionReporter._get_raw_insecure_uri().
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Hasan Ramezani):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:7>
Django
ExceptionReporter._get_raw_insecure_uri().
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
Comment:
[https://github.com/django/django/pull/14331 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:8>
Django
ExceptionReporter._get_raw_insecure_uri().
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Hasan
Type: | Ramezani
Component: HTTP handling | Version: dev
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"8bcb00858e0ddec79cc96669c238d29c30d7effb" 8bcb0085]:
{{{
#!CommitTicketReference repository=""
revision="8bcb00858e0ddec79cc96669c238d29c30d7effb"
Fixed #32698 -- Moved HttpRequest.get_raw_uri() to
ExceptionReporter._get_raw_insecure_uri().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:9>
Django
ExceptionReporter._get_raw_insecure_uri().
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: closed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Jaap Roes):
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 [https://github.com/lepture/authlib/pull/385 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 [https://github.com/elastic/apm-agent-
python/blob/484ba5a1ad07e59bd4adeaf0291cc3da23d76afa/elasticapm/contrib/django/client.py#L155
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)
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:10>
Django
ExceptionReporter._get_raw_insecure_uri().
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: closed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:10 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:11>
Django
ExceptionReporter._get_raw_insecure_uri().
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: closed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Adam Johnson):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:12>
Django
ExceptionReporter._get_raw_insecure_uri().
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: closed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Benjamin Wohlwend):
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
[https://code.djangoproject.com/ticket/27506#comment:3 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32698#comment:13>