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

Fix infinite redirection when path root contains query params #203

Conversation

marwanehcine
Copy link
Contributor

to fix infinite redirection when path root contains query params, traefik redirection rules in docker-compose.override.yml has to be updated. A new regex formula is used to check is query param exist or not

@marwanehcine marwanehcine changed the base branch from master to add_gateway May 4, 2023 20:54
@@ -108,15 +108,10 @@ services:
)
- "traefik.http.routers.traefik-redirect.priority=10"
- "traefik.http.routers.traefik-redirect.middlewares=add-trailing-slash@docker"
- "traefik.http.middlewares.add-trailing-slash.redirectregex.regex=^https?://(.*)/(.+)"
- "traefik.http.middlewares.add-trailing-slash.redirectregex.regex=^https?://^[^?]+$/(.+)"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jahow @emmdurin , check changes please

@jeanmi151 jeanmi151 merged commit 8d4acde into georchestra:add_gateway May 5, 2023
@jahow
Copy link
Contributor

jahow commented May 5, 2023

I have tested this and it works, but I think the regex is misleading. Something like that would be clearer IMO:

- "traefik.http.middlewares.add-trailing-slash.redirectregex.regex=^https?://(.+)/([^?]+)(\\?.*)?$"
- "traefik.http.middlewares.add-trailing-slash.redirectregex.replacement=https://$${1}/$${2}/$${3}"

Here we add a third capturing group containing query params, which we reappend after the trailing slash. What do you think @marwanehcine?

@jeanmi151
Copy link
Contributor

jeanmi151 commented May 5, 2023

I have tested this and it works, but I think the regex is misleading. Something like that would be clearer IMO:

- "traefik.http.middlewares.add-trailing-slash.redirectregex.regex=^https?://(.+)/([^?]+)(\\?.*)?$"
- "traefik.http.middlewares.add-trailing-slash.redirectregex.replacement=https://$${1}/$${2}/$${3}"

Here we add a third capturing group containing query params, which we reappend after the trailing slash. What do you think @marwanehcine?

can you please continue this PR here : #197
to not have duplicates

emmdurin pushed a commit that referenced this pull request Jun 9, 2023
* rebase from #168

* update traefik redirection rules
emmdurin pushed a commit that referenced this pull request Jul 11, 2023
* rebase from #168

* update traefik redirection rules
f-necas pushed a commit that referenced this pull request Feb 19, 2024
* rebase from #168

* update traefik redirection rules
edevosc2c pushed a commit that referenced this pull request Jun 7, 2024
* rebase from #168

* update traefik redirection rules
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 this pull request may close these issues.

3 participants