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

[HttpKernel] Correctly Render Signed URIs Containing Fragments #29679

Merged
merged 1 commit into from Jan 5, 2019

Conversation

zanbaldwin
Copy link
Member

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no?
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a
  • Rebuild the URL with the computed hash instead of appending it onto the end of the fragment.
  • Update unit tests, and add new unit test to cover URIs that include fragments.

@zanbaldwin zanbaldwin changed the title Correctly Render Signed URIs Containing Fragments [HttpKernel] Correctly Render Signed URIs Containing Fragments Dec 24, 2018
Rebuild the URL with the computed hash instead of appending it onto the end of the URI, preventing incorrect formatting when dealing with URIs containing fragments.
@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Dec 24, 2018
@nicolas-grekas
Copy link
Member

Hi Zan, I'm not sure I get what this fixes? From the description, I feel like this is just a different way to achieve the same. Can you give some more hints please? Thanks :)

@zanbaldwin
Copy link
Member Author

zanbaldwin commented Dec 25, 2018

Sorry, I should have given an example!
If a URI contained a fragment, the resulting signed URL would have resulted in the hash query parameter appended to the URI string (after the fragment), instead of being placed in the query string part of the URL. For example:

Before: http://example.com/path?foo=bar#fragment&_hash=$HASH

After: http://example.com/path?foo=bar&_hash=$HASH#fragment

@fabpot
Copy link
Member

fabpot commented Jan 5, 2019

Thank you @zanbaldwin.

@fabpot fabpot merged commit b9ece6b into symfony:3.4 Jan 5, 2019
fabpot added a commit that referenced this pull request Jan 5, 2019
…ents (zanbaldwin)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] Correctly Render Signed URIs Containing Fragments

| Q             | A
| ------------- | ---
| Branch?       | `3.4`
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no?
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

- Rebuild the URL with the computed hash instead of appending it onto the end of the fragment.
- Update unit tests, and add new unit test to cover URIs that include fragments.

Commits
-------

b9ece6b [HttpKernel] Correctly Render Signed URIs Containing Fragments
This was referenced Jan 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants