Opened 2 years ago

Closed 3 weeks ago

#19910 closed Cleanup/optimization (fixed)

Double redirect with APPEND_SLASH and i18n_patterns

Reported by: ssteinberger@… Owned by: bpeschier
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 2 years ago by claudep

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 2 years ago by aaugustin

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 4 weeks ago by bpeschier

  • Has patch set
  • Owner changed from nobody to bpeschier
  • Status changed from new to assigned

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

comment:4 Changed 4 weeks ago by timgraham

  • Triage Stage changed from Accepted to Ready 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 3 weeks ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

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