Allow loading big xml files with XmlEncoder #17956
Conversation
| @@ -81,7 +81,7 @@ public function decode($data, $format, array $context = array()) | |||
| libxml_clear_errors(); | |||
|
|
|||
| $dom = new \DOMDocument(); | |||
| $dom->loadXML($data, LIBXML_NONET | LIBXML_NOBLANKS); | |||
| $dom->loadXML($data, LIBXML_NONET | LIBXML_NOBLANKS | (defined('LIBXML_PARSEHUGE') ? LIBXML_PARSEHUGE : 0)); | |||
There was a problem hiding this comment.
This constant always exists because Symfony requires PHP >= 5.3.3. You can remove the test.
|
WDYT about keeping limits on by default but making options configurable using the |
|
@dunglas The real question is why someone would want to disable/enable this behavior? |
|
If it's safe and performant, why it is not enabled by default in PHP/libxml? Real question, I've no idea of the history behind this flag. |
|
The reason why it isn't set by default is discussed in this blogpost: http://symfony.com/blog/security-release-symfony-2-0-17-released Making it configurable is okay with me, as long as I can configure it before the The |
|
We should make this configurable and not hardcoded. The merging of #16873 was a mistake. We're going to submit a new PR to remove it and make it configurable as well. The intention to use this flag should be done by the developer. |
|
See #17986 |
…-grekas) This PR was merged into the 2.3 branch. Discussion ---------- [DomCrawler] Dont use LIBXML_PARSEHUGE by default | Q | A | ------------- | --- | Branch | 2.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | no | Fixed tickets | #16873, #17956 | License | MIT | Doc PR | - Because of http://symfony.com/blog/security-release-symfony-2-0-17-released Commits ------- fda32f8 [DomCrawler] Dont use LIBXML_PARSEHUGE by default
|
👍 on making it configurable. How can we make sure this is configurable in userland for the |
|
@rvanlaak you're asking for a new feature, I think we should close this very PR and I invite you to open a new one against master that would allow setting the flags on XmlEncoder and add some glue to make it configurable in FrameworkBundle (I guess, and if worth it). |
|
👍 on closing this PR, see #17987 |
Also see #16873 where this was fixed for the
DomCrawlercomponent.