Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Made fragment URLs relative instead of absolute by default#8879
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
fabpot added a commit that referenced this pull requestAug 30, 2013
This PR was merged into the 2.2 branch.Discussion----------Made fragment URLs relative instead of absolute by default| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#8458| License | MIT| Doc PR | n/aThis fixes a regression introduced with the new fragment system where fragment URLs were generated as absolute URLs. As per the ESI spec, there is no need to generate an absolute URL and this can even be problematic when dealing with internal sub-requests in a more "complex" hosting environment.Commits-------91234cd [HttpKernel] changed fragment URLs to be relative by default (closes#8458)
This was referencedSep 7, 2013
fabpot added a commit that referenced this pull requestSep 8, 2013
This PR was merged into the 2.2 branch.Discussion----------[HttpKernel] fix HInclude src (closes#8951)| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#8951| License | MIT| Doc PR | n/afixes a regression introduced in#8879Commits-------49f5027 [HttpKernel] fixer HInclude src (closes#8951)
fabpot added a commit that referenced this pull requestJul 29, 2022
…ith absolute URIs (Kern046)This PR was merged into the 6.2 branch.Discussion----------[HttpKernel] Add option to render Surrogate fragment with absolute URIs| Q | A| ------------- | ---| Branch? | 6.2| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets || License | MIT| Doc PR |Context-------Using ESI blocks with Cloudflare Workers we faced an issue with the `/_fragment` relative URIs, as Cloudflare didn't succeed to handle it. We needed the absolute URL to make it work.Proposed solution--------In Symfony's codebase I noticed that `Symfony\Component\HttpKernel\Fragment\FragmentUriGenerator::generate` already supports a `$absolute` parameter to fit the needs of `fragment_uri` Twig function (I even wonder if this wasn't a previously supported feature when I see#8879), but this parameter couldn't be set through `render_esi`.This PR adds the support of a new `absolute_uri` option in order to set this parameter and get an absolute URI for ESI and SSI fragments. It only applies when using a `ControllerReference` as the URI or `alt` option. It defaults to `false` to avoid causing any BC break.### NamingI am not that confident about the naming:- I used `absolute_uri` for the option to keep it more self-descriptive than just `absolute`.- I used `$absolute` to keep it more consistent with the existing parameters and usage in `HttpKernelRuntime::generateFragmentUri`.I have doubts about the pertinence of such a diff between the option and the variable name. Let me know what you think about it !Commits-------f9f3ed0 [HttpKernel] Add option to render Surrogate fragment with absolute URIs
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes a regression introduced with the new fragment system where fragment URLs were generated as absolute URLs. As per the ESI spec, there is no need to generate an absolute URL and this can even be problematic when dealing with internal sub-requests in a more "complex" hosting environment.