Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add relative current_url #259

Closed
treyhunner opened this issue Aug 29, 2022 · 5 comments · Fixed by #287
Closed

Add relative current_url #259

treyhunner opened this issue Aug 29, 2022 · 5 comments · Fixed by #287

Comments

@treyhunner
Copy link

Description

current_url returns the full URL:

>>> request.htmx.current_url
'https://www.pythonmorsel.com/some-page/

I'd like a way to access just the non-domain part of this URL (I'm using it for a ?next= login redirect).

>>> request.htmx.relative_current_url
'/some-page/

I imagine something like this might work.

    @cached_property
    def relative_current_url(self) -> str | None:
        url = self.current_url
        if url:
            urlunparse(urlparse(url)._r(scheme='', netloc=''))
        return url
@adamchainz
Copy link
Owner

I wouldn't feel comfortable just dropping the origin, as it's potentially security-relevant information. Some projects are deployed with multiple origins.

Instead, why not compare with absolute URL's? I don't quite know what you're trying to do, but perhaps you could use something like this:

if request.htmx.current_url == request.build_absolute_uri('/login/'):

@treyhunner
Copy link
Author

treyhunner commented Sep 29, 2022

@adamchainz I wasn't trying to compare, but was instead redirecting to a login page while specifying a next param. That next= param needs to be a relative URL (Django gets upset when it's an absolute URL).

Instead of this:

path = urlunparse(
    urlparse(self.request.htmx.current_url)._replace(netloc="", scheme="")
)
response = redirect_to_login(
    path,
    resolve_url(self.get_login_url()),
    self.get_redirect_field_name(),
)
return HttpResponseClientRedirect(response.url)

I wanted to be able to do this:

response = redirect_to_login(
    self.request.htmx.relative_current_url,
    resolve_url(self.get_login_url()),
    self.get_redirect_field_name(),
)
return HttpResponseClientRedirect(response.url)

In my case I plan to force redirects often with ?next=<CURRENT_RELATIVE_URL>. If you think this is too niche of a use case (or too challenging of a problem to solve without raising security concerns) I understand.

@adamchainz
Copy link
Owner

Yeah I think adding the helper is probably too niche/security sensitive to solve here. Calling Django’s redirect_to_login to generate a response that you then just use the URL from doesn't sit right.

(Django gets upset when it's an absolute URL).

How exactly? redirect_to_login() doesn't seem to mind. The later “allowed redirect” check should be done by the internal function url_has_allowed_host_and_scheme(), which checks for an allowed origin, so it should be fine with absolute URL's on the same origin?

(P.S. you probably want to use urlsplit() instead of urlparse(), as I learned recently. Django could probably do with that as well...)

@treyhunner
Copy link
Author

Yeah I think adding the helper is probably too niche/security sensitive to solve here. Calling Django’s redirect_to_login to generate a response that you then just use the URL from doesn't sit right.

I do agree that the specific use case I showed is a hack that isn't wise. That was the first case that came up and I haven't yet refactored that code.

I just came across another case today though.

Something like this in a non-HTMX Django page:

{% url "users:login" %}?next={{ request.get_full_path }}

Doesn't have an equivalent in django-htmx land without a custom template filter/tag.

{% url "users:login" %}?next={{ request.htmx.relative_current_url }}

(Django gets upset when it's an absolute URL).

How exactly? redirect_to_login() doesn't seem to mind. The later “allowed redirect” check should be done by the internal function url_has_allowed_host_and_scheme(), which checks for an allowed origin, so it should be fine with absolute URL's on the same origin?

From my testing, passing an absolute URL to next= didn't seem to work.
Django seems to just ignore the next value entirely when it wasn't a relative URL (as of Django 3.2 at least).

(P.S. you probably want to use urlsplit() instead of urlparse(), as I learned recently. Django could probably do with that as well...)

Ah I do. Thanks for noting that!

@adamchainz
Copy link
Owner

Added in #287, as current_url_abs_path.

I did and a bit of research to check naming, and was reminded that relative URL’s mean “relative to a given base URL, like ./image.pngis relative to whichever URL. The right name is “absolute-path” as per [section 4.2 of this RFC](https://www.rfc-editor.org/rfc/rfc3986#page-26), which is also what Django (kinda) uses e.g. inModel.get_absolute_url(). So I picked the name with the _abs_path` suffix, which is kinda wordy but at least correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants