Skip to content

use absolute urls for fragment uri's #8951

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

Closed
wants to merge 1 commit into from
Closed

use absolute urls for fragment uri's #8951

wants to merge 1 commit into from

Conversation

gnat42
Copy link
Contributor

@gnat42 gnat42 commented Sep 6, 2013

When using render_hinclude, a uri is generated that has a hash added to it.
The FragmentListener then checks the hash to make sure it is signed and valid.
The check however uses the http scheme and host so the uri needs to start as
an absolute uri to match the resulting check. Otherwise all render_hinclude calls
result in an AccessDeniedHttpException.

When using render_hinclude, a uri is generated that has a hash added to it.
The FragmentListener then checks the hash to make sure it is signed and valid.
The check however uses the http scheme and host so the uri needs to start as
an absolute uri to match the resulting check. Otherwise all render_hinclude calls
result in an AccessDeniedHttpException.
@gnat42
Copy link
Contributor Author

gnat42 commented Sep 6, 2013

I'm also not sure how to have this applied to 2.3 and 2.2 but that would be great

@@ -91,7 +91,7 @@ public function render($uri, Request $request, array $options = array())
throw new \LogicException('You must use a proper URI when using the Hinclude rendering strategy or set a URL signer.');
}

$uri = $this->signer->sign($this->generateFragmentUri($uri, $request));
$uri = $this->signer->sign($this->generateFragmentUri($uri, $request, true));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As demonstrated by the tests that do not pass anymore, that basically reverts #8879 which is not possible.

@fabpot
Copy link
Member

fabpot commented Sep 8, 2013

This behavior is new as of 2.2.7-DEV, so it should be fixed before releasing 2.2.7.

fabpot added a commit to fabpot/symfony that referenced this pull request Sep 8, 2013
@fabpot
Copy link
Member

fabpot commented Sep 8, 2013

closing in favor of #8960

@fabpot fabpot closed this Sep 8, 2013
fabpot added a commit that referenced this pull request Sep 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/a

fixes a regression introduced in #8879

Commits
-------

49f5027 [HttpKernel] fixer HInclude src (closes #8951)
fabpot added a commit that referenced this pull request Sep 8, 2013
* 2.2:
  [HttpFoundation] fixed regression in the way the request format is handled for duplicated requests (closes #8917)
  [HttpKernel] fixer HInclude src (closes #8951)
  Fixed escaping of service identifiers in configuration
fabpot added a commit that referenced this pull request Sep 10, 2013
* 2.3:
  Fixed docblock in UserInterface::getSalt()
  [Process] Fix #8970 : read output once the process is finished, enable pipe tests on Windows
  [DoctrineBridge] Improved test coverage of EntityChoiceList
  [DoctrineBridge] Improved test coverage of EntityChoiceList
  [Form] Improved test coverage of ChoiceList classes
  [Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted
  [Form] Fixed ChoiceList::get*By*() methods to preserve order and array keys
  [Form] Removed usage of the ChoiceList::getIndicesFor*() methods where they don't offer any performance benefit
  [Form] Improved test coverage of ChoiceList classes
  [Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted
  [Form] Fixed ChoiceList::get*By*() methods to preserve order and array keys
  [Form] Removed usage of the ChoiceList::getIndicesFor*() methods where they don't offer any performance benefit
  Removed duplicate annotation
  [HttpKernel] made code more reliable
  [HttpFoundation] fixed regression in the way the request format is handled for duplicated requests (closes #8917)
  [HttpKernel] fixer HInclude src (closes #8951)
  Fixed escaping of service identifiers in configuration

Conflicts:
	src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/GenericEntityChoiceListTest.php
	src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php
	src/Symfony/Component/HttpKernel/Tests/Fragment/HIncludeFragmentRendererTest.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants