Opened 4 years ago

Closed 18 months ago

#19910 closed Cleanup/optimization (fixed)

Double redirect with APPEND_SLASH and i18n_patterns

Reported by: ssteinberger@… Owned by: Bas Peschier
Component: Core (URLs) Version: 1.4
Severity: Normal Keywords: redirect, i18n, append_slash
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Just a thought:
Using settings.APPEND_SLASH = True in combination with i18n_patterns may lead to unnecessary redirects, like so:

Opening URL: "/some_i18n_url"
Redirect to: "/en/some_i18n_url"
Redirect to: "/en/some_i18n_url/"

It would be more efficient to combine those redirects into.

Change History (5)

comment:1 Changed 4 years ago by Claude Paroz

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

This might require adding a new parameter (force_append_slash) to request.get_full_path as the slash might have to be inserted between the path and a possible query string. Then CommonMiddleware could also benefit from it.

comment:2 Changed 4 years ago by Aymeric Augustin

It might be interesting to include RedirectMiddleware in the scope of this ticket as it also performs redirects that can interact with APPEND_SLASH.

comment:3 Changed 19 months ago by Bas Peschier

Has patch: set
Owner: changed from nobody to Bas Peschier
Status: newassigned

Created a PR for the suggestions above, should we also document the parameter as part of the public API?

comment:4 Changed 18 months ago by Tim Graham

Triage Stage: AcceptedReady for checkin

Looks good, pending some cosmetic edits. I don't feel strongly that the parameter should be documented, but happy to have it if someone sees a use case for it in user code.

comment:5 Changed 18 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 9128762:

Fixed #19910 -- Added slash to i18n redirect if APPEND_SLASH is set.

This introduces a force_append_slash argument for request.get_full_path()
which is used by RedirectFallbackMiddleware and CommonMiddleware when
handling redirects for settings.APPEND_SLASH.

Note: See TracTickets for help on using tickets.
Back to Top