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

[DependencyInjection] force enabling the external XML entity loaders #18908

Merged
merged 1 commit into from May 30, 2016

Conversation

Projects
None yet
4 participants
@xabbuh
Member

xabbuh commented May 29, 2016

Q A
Branch? 2.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #18876
License MIT
Doc PR
@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented May 30, 2016

Do we have the same issue in XliffFileLoader.php and XmlUtils.php?

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented May 30, 2016

Thank you @xabbuh.

@nicolas-grekas nicolas-grekas merged commit 142b1a4 into symfony:2.3 May 30, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request May 30, 2016

bug #18908 [DependencyInjection] force enabling the external XML enti…
…ty loaders (xabbuh)

This PR was merged into the 2.3 branch.

Discussion
----------

[DependencyInjection] force enabling the external XML entity loaders

| Q             | A
| ------------- | ---
| Branch?       | 2.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #18876
| License       | MIT
| Doc PR        |

Commits
-------

142b1a4 force enabling the external XML entity loaders
@sstok

This comment has been minimized.

Contributor

sstok commented May 30, 2016

@nicolas-grekas It seems so, https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Translation/Loader/schema/dic/xliff-core/xliff-core-1.2-strict.xsd#L33

I wonder if this will not load the external resources of the to be validated XML document, which would break the security system that was introduced 😨 can this be checked somehow?

@xabbuh xabbuh deleted the xabbuh:issue-18876 branch May 30, 2016

nicolas-grekas added a commit that referenced this pull request May 30, 2016

Revert "bug #18908 [DependencyInjection] force enabling the external …
…XML entity loaders (xabbuh)"

This reverts commit 44f6f89, reversing
changes made to 57d6053.
@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented May 30, 2016

Reverted because this look suspicious and need more thought.
@xabbuh now that 2.3 is EOLed, could you please reopen this on 2.7 so that the discussion can continue?

@xabbuh

This comment has been minimized.

Member

xabbuh commented May 30, 2016

@fabpot fabpot referenced this pull request May 30, 2016

Merged

Release v2.3.42 #18916

fabpot added a commit that referenced this pull request Jun 6, 2016

Merge branch '2.3' into 2.7
* 2.3:
  updated VERSION for 2.3.42
  update CONTRIBUTORS for 2.3.42
  updated CHANGELOG for 2.3.42
  Revert "bug #18908 [DependencyInjection] force enabling the external XML entity loaders (xabbuh)"
  Partial revert of previous PR
  [DependencyInjection] Skip deep reference check for 'service_container'
  Catch \Throwable
  [Serializer] Add missing @throws annotations
  Fix for #18843
  force enabling the external XML entity loaders
  Removed UTC specification with timestamp

nicolas-grekas added a commit that referenced this pull request Jun 6, 2016

Merge branch '2.7' into 2.8
* 2.7:
  `@throws` annotations should go after `@return`
  Fix merge
  updated VERSION for 2.3.42
  update CONTRIBUTORS for 2.3.42
  updated CHANGELOG for 2.3.42
  Revert "bug #18908 [DependencyInjection] force enabling the external XML entity loaders (xabbuh)"
  Partial revert of previous PR
  [DependencyInjection] Skip deep reference check for 'service_container'
  Catch \Throwable
  [Serializer] Add missing @throws annotations
  Fix for #18843
  force enabling the external XML entity loaders
  Removed UTC specification with timestamp

Conflicts:
	src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
	src/Symfony/Component/Finder/Finder.php
	src/Symfony/Component/Security/Acl/Dbal/MutableAclProvider.php
	src/Symfony/Component/Security/Acl/Domain/ObjectIdentity.php
	src/Symfony/Component/Security/Acl/Model/AclInterface.php
	src/Symfony/Component/Security/Acl/Model/MutableAclProviderInterface.php
	src/Symfony/Component/Security/Acl/Permission/MaskBuilder.php
	src/Symfony/Component/Translation/Loader/XliffFileLoader.php
	src/Symfony/Component/Yaml/Tests/InlineTest.php

nicolas-grekas added a commit that referenced this pull request Jun 6, 2016

Merge branch '2.8' into 3.0
* 2.8:
  `@throws` annotations should go after `@return`
  Fix merge
  updated VERSION for 2.3.42
  update CONTRIBUTORS for 2.3.42
  updated CHANGELOG for 2.3.42
  Revert "bug #18908 [DependencyInjection] force enabling the external XML entity loaders (xabbuh)"
  Partial revert of previous PR
  [DependencyInjection] Skip deep reference check for 'service_container'
  Catch \Throwable
  [Serializer] Add missing @throws annotations
  Fix for #18843
  force enabling the external XML entity loaders
  Removed UTC specification with timestamp

Conflicts:
	CHANGELOG-2.3.md
	src/Symfony/Bundle/FrameworkBundle/Routing/DelegatingLoader.php
	src/Symfony/Bundle/TwigBundle/Extension/AssetsExtension.php
	src/Symfony/Component/Config/Loader/FileLoader.php
	src/Symfony/Component/DependencyInjection/Container.php
	src/Symfony/Component/DependencyInjection/ContainerBuilder.php
	src/Symfony/Component/Finder/Expression/Expression.php
	src/Symfony/Component/Finder/Finder.php
	src/Symfony/Component/HttpKernel/DependencyInjection/ContainerAwareHttpKernel.php
	src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php

nicolas-grekas added a commit that referenced this pull request Jun 6, 2016

Merge branch '3.0' into 3.1
* 3.0:
  `@throws` annotations should go after `@return`
  Fix merge
  updated VERSION for 2.3.42
  update CONTRIBUTORS for 2.3.42
  updated CHANGELOG for 2.3.42
  Revert "bug #18908 [DependencyInjection] force enabling the external XML entity loaders (xabbuh)"
  Partial revert of previous PR
  [DependencyInjection] Skip deep reference check for 'service_container'
  Catch \Throwable
  [Serializer] Add missing @throws annotations
  Fix for #18843
  force enabling the external XML entity loaders
  Removed UTC specification with timestamp

Conflicts:
	src/Symfony/Component/Yaml/Tests/InlineTest.php

nicolas-grekas added a commit that referenced this pull request Jun 6, 2016

Merge branch '3.1'
* 3.1:
  `@throws` annotations should go after `@return`
  Fix merge
  updated VERSION for 2.3.42
  update CONTRIBUTORS for 2.3.42
  updated CHANGELOG for 2.3.42
  Revert "bug #18908 [DependencyInjection] force enabling the external XML entity loaders (xabbuh)"
  Partial revert of previous PR
  [DependencyInjection] Skip deep reference check for 'service_container'
  Catch \Throwable
  [Serializer] Add missing @throws annotations
  Fix for #18843
  force enabling the external XML entity loaders
  Removed UTC specification with timestamp

This was referenced Jun 6, 2016

fabpot added a commit that referenced this pull request Jun 13, 2016

bug #18915 [DependencyInjection] force enabling the external XML enti…
…ty loaders (xabbuh)

This PR was merged into the 2.7 branch.

Discussion
----------

[DependencyInjection] force enabling the external XML entity loaders

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #18876, #18908
| License       | MIT
| Doc PR        |

Commits
-------

12b5509 force enabling the external XML entity loaders

@fabpot fabpot referenced this pull request Jun 15, 2016

Merged

Release v3.1.1 #19055

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment