The Wayback Machine - https://web.archive.org/web/20240218070452/https://github.com/TryGhost/Ghost/issues/10898
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

Jump to bottom

Update query params forwarding behavior for redirects.json file #10898

Closed
rishabhgrg opened this issue Jul 12, 2019 · 11 comments
Closed

Update query params forwarding behavior for redirects.json file #10898

rishabhgrg opened this issue Jul 12, 2019 · 11 comments
Labels
affects:routing related to the url service & dynamic routing features bug [triage] something behaving unexpectedly help wanted [triage] Ideal issues for contributors to help with server / core Issues relating to the server or core of Ghost stale [triage] Issues that were closed to to lack of traction

Comments

@rishabhgrg
Copy link
Contributor

For custom redirects, Ghost currently overwrites query params in to url as they are in from. This means if to url has any query params, they will get overridden to be empty instead if from url has no query params. This is leading to unexpected behavior, specially in cases where to url has explicit query params. Suggested update to behavior -

  1. Merge from and to query params, with from overwriting to
  2. Add option to not include any params from from

This covers most scenarios in which query params override can be used.
@rishabhgrg rishabhgrg added the server / core Issues relating to the server or core of Ghost label Jul 12, 2019
@naz naz added the affects:routing related to the url service & dynamic routing features label Jul 12, 2019
@allouis
Copy link
Contributor

allouis commented Jul 15, 2019

  • query params in the "from" field should be matched to the "from url"
  • query params in the "to" field should always be kept (ie, take precedence over incoming query params)
  • all other query params are passed through

@aileen
Copy link
Member

aileen commented Jul 30, 2019

This issue also applies to a redirect like this:

[{
"from": "^/media/(.*)$",
"to": "https://storage.googleapis.com/files.domain.com/media/$1",
"permanent": true
}]

In this case the URL works, but not for the $1 part, which is what I would expect and also our docs are describing.

@ErisDS ErisDS added the bug [triage] something behaving unexpectedly label Aug 21, 2019
@stale
Copy link

stale bot commented Nov 19, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale [triage] Issues that were closed to to lack of traction label Nov 19, 2019
@naz naz added help wanted [triage] Ideal issues for contributors to help with and removed stale [triage] Issues that were closed to to lack of traction labels Nov 19, 2019
@stale
Copy link

stale bot commented Feb 17, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale [triage] Issues that were closed to to lack of traction label Feb 17, 2020
@stale stale bot closed this as completed Feb 24, 2020
@allouis allouis reopened this Feb 25, 2020
@stale stale bot removed the stale [triage] Issues that were closed to to lack of traction label Feb 25, 2020
@allouis allouis added the pinned [triage] Ignored by stalebot label Feb 25, 2020
@Louvki
Copy link

Louvki commented Mar 11, 2020

Is there any workaround for this?

@allouis
Copy link
Contributor

allouis commented Mar 12, 2020

Hey @Louvki o/

No workaround that I'm aware of I'm afraid!

We'd welcome a PR if you find the time 👻

@daviddarnes
Copy link
Contributor

@Louvki 👋, if you create a topic on our forum I might be able to help you with a workaround https://forum.ghost.org

@fedetibaldo fedetibaldo mentioned this issue Apr 29, 2020
Merged
3 tasks
ErisDS pushed a commit that referenced this issue Apr 30, 2020
@fedetibaldo
🐛 Fixed regex match replacement when dealing with external URLs (#11781)
4fcc310 
refs #10898

- Execute string replacement on external paths
- Take non-top-level base URLs into consideration (to avoid #10776 dups)
- Added tests for all of the above cases
@ErisDS ErisDS removed the pinned [triage] Ignored by stalebot label Aug 3, 2020
@stale
Copy link

stale bot commented Nov 2, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale [triage] Issues that were closed to to lack of traction label Nov 2, 2020
@sainthkh
Copy link
Contributor

sainthkh commented Nov 4, 2020

Hello, @Stale. I'm working on this.

@stale stale bot removed the stale [triage] Issues that were closed to to lack of traction label Nov 4, 2020
@sainthkh sainthkh mentioned this issue Nov 5, 2020
Merged
3 tasks
sainthkh added a commit to sainthkh/Ghost that referenced this issue Nov 5, 2020
@sainthkh
Forward query params when redirect.
9a748f4 
ref TryGhost#10898

- It parses the query params and merge them and stringify the result.
@sainthkh
Copy link
Contributor

sainthkh commented Nov 5, 2020

Todo

naz pushed a commit that referenced this issue Jan 5, 2021
@sainthkh
🐛 Fixed redirects "to" query params forwarding (#12333)
7528ec8 
ref #10898

- The redirects configuration's `to` & `from` URL parameters used to ignore it's query string parameters, which resulted in unexpected behavior
- Current changeset only partially fixes the issue. Now `to` URL's query parameters always take precedence over incoming query parameters and the rest of query parameters are passed through.
@stale
Copy link

stale bot commented Jun 9, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale [triage] Issues that were closed to to lack of traction label Jun 9, 2021
@stale stale bot closed this as completed Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
affects:routing related to the url service & dynamic routing features bug [triage] something behaving unexpectedly help wanted [triage] Ideal issues for contributors to help with server / core Issues relating to the server or core of Ghost stale [triage] Issues that were closed to to lack of traction
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@ErisDS @naz @daviddarnes @allouis @rishabhgrg @aileen @sainthkh @Louvki