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

Avoid redundant URL encoding #4555

Merged
merged 2 commits into from Apr 4, 2019

Conversation

Projects
None yet
4 participants
@zauguin
Copy link
Contributor

commented Feb 4, 2019

The redirect URL for SSO logins is transmitted using a HTML form field, therefore the browser takes care of proper URL encoding. We should therefore not encode the value because otherwise the redirect URL is "double-encoded" and no longer usable.

This fixes a regression introduced by #4220.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file
  • Pull request includes a sign off
Do not double encode fallback redirect URL
Signed-off-by: Marcel Fabian Krüger <zauguin@gmail.com>
@codecov-io

This comment has been minimized.

Copy link

commented Feb 4, 2019

Codecov Report

Merging #4555 into develop will increase coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #4555      +/-   ##
===========================================
+ Coverage     74.9%   74.91%   +<.01%     
===========================================
  Files          338      338              
  Lines        34509    34509              
  Branches      5620     5620              
===========================================
+ Hits         25850    25852       +2     
  Misses        7075     7075              
+ Partials      1584     1582       -2
@erikjohnston

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

@richvdh does this look plausible, given you touched it last?

@richvdh

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

yes, yes it does.

@richvdh

richvdh approved these changes Apr 4, 2019

Copy link
Member

left a comment

thanks!

@richvdh richvdh merged commit 9f5d206 into matrix-org:develop Apr 4, 2019

0 of 6 checks passed

buildkite/synapse Build #837 failed (3 seconds)
Details
buildkite/synapse/pipeline Failed (exit status 1)
Details
ci/circleci: sytestpy2merged CircleCI is running your tests
Details
ci/circleci: sytestpy2postgresmerged CircleCI is running your tests
Details
ci/circleci: sytestpy3merged CircleCI is running your tests
Details
ci/circleci: sytestpy3postgresmerged CircleCI is running your tests
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.