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

Space encoding in search query changed during login process #4313

Closed
coffee-squirrel opened this issue Aug 22, 2019 · 3 comments · Fixed by #4625
Closed

Space encoding in search query changed during login process #4313

coffee-squirrel opened this issue Aug 22, 2019 · 3 comments · Fixed by #4625
Labels
bug release/documented Documentation and release notes have been updated. web-ui
Milestone

Comments

@coffee-squirrel
Copy link

Bug Report

Space encoding in the search URL param changes from %20 (search-input-field value status: running) to + search-input-field value status:+running) when starting out unauthenticated and then logging in. This results in an invalid search query after getting redirected back.

Steps to Reproduce

  1. Access the Concourse Web interface and ensure you're currently logged out
  2. Use search-input-field to search for something (e.g. status: running)
  3. Click "login" in the upper-right and successfully log in
  4. After getting redirected back, note that space characters have been replaced with +, resulting in something like "No results for status:+running matched your search."

Expected Results

The original search value is preserved after going through the login process.

Actual Results

The original search value is modified after going through the login process.

Version Info

  • Concourse version: 5.4.1
  • Deployment type (BOSH/Docker/binary): BOSH
  • Infrastructure/IaaS: vSphere
  • Browser (if applicable): Firefox 68.0
  • Did this used to work? Unknown
@stale
Copy link

stale bot commented Oct 21, 2019

Beep boop! This issue has been idle for long enough that it's time to check in and see if it's still important.

If it is, what is blocking it? Would anyone be interested in submitting a PR or continuing the discussion to help move things forward?

If no activity is observed within the next week, this issue will be exterminated closed, in accordance with our stale issue process.

@stale stale bot added the wontfix label Oct 21, 2019
@coffee-squirrel
Copy link
Author

This still exists under v5.6.0.

@stale stale bot removed the wontfix label Oct 21, 2019
@jamieklassen
Copy link
Member

jamieklassen commented Oct 22, 2019

I kinda want to describe the root cause here as a bug in the elm URL library: elm/url#32.

More specifically, the way golang handles URLs follows the standard, so that when skymarshal adds a CSRF token to the redirect URL, it actually decodes the %20 in the query string (as Elm assigned it) into a space when parsing the URL, and then re-encodes it as a + following the standards: https://golang.org/src/net/url/url.go?s=8661:8697#L308.

Technically encoding spaces as %20 isn't wrong. Ideally golang would provide some way to mutate the query parameters on a URL without re-encoding (since decoding and encoding are not proper inverses), but as far as I can tell it doesn't. So we kinda have to handle this on the Elm side. When parsing the search parameter, if you see a + you have to workaround elm/url's bug and replace it with a ' '.

jamieklassen pushed a commit that referenced this issue Oct 22, 2019
work around elm/url#32.

#4313

Signed-off-by: Jamie Klassen <cklassen@pivotal.io>
@jamieklassen jamieklassen mentioned this issue Oct 22, 2019
9 tasks
@jamieklassen jamieklassen added this to the v5.7.0 milestone Oct 30, 2019
@jamieklassen jamieklassen added the release/documented Documentation and release notes have been updated. label Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug release/documented Documentation and release notes have been updated. web-ui
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants