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.

Sign up for GitHub

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

Jump to bottom

[BUGFIX] Avoid segmentation faults when processing entities #53

Merged
merged 2 commits into from
Aug 9, 2021
Merged

[BUGFIX] Avoid segmentation faults when processing entities #53

merged 2 commits into from
Aug 9, 2021

Conversation

ohader
Copy link
Contributor

@ohader ohader commented Jul 15, 2021

Related: #52

@ohader ohader marked this pull request as draft July 15, 2021 21:04
@ohader
Copy link
Contributor Author

ohader commented Jul 15, 2021 

vendor/bin/phpunit --no-coverage --filter=/doctypeAndEntityAreRemoved/
PHPUnit 6.5.14 by Sebastian Bergmann and contributors.

[1]    63007 segmentation fault  vendor/bin/phpunit --no-coverage --filter=/doctypeAndEntityAreRemoved/

@ohader ohader marked this pull request as ready for review July 15, 2021 21:35
@ohader ohader mentioned this pull request Jul 15, 2021
Closed
reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this pull request Jul 16, 2021
@ohader
[TASK] Skip SVG sanitizer test causing segmentation fault
0e53b48 
SVG sanitizer test dataset entity.svg is causing segmentation fault
in certain scenarios - which might be related to libxml2 before
version 2.9.12. Unfortunately, investigations did not reveal any
further details other than libxml2.

As a result `entity.svg` test dataset, which is causing this problem is
skipped until darylldoyle/svg-sanitizer#53 is
merged and released in the upstream library.

Resolves: #94565
Releases: master, 10.4, 9.5
Change-Id: I8375954dad64e3955f88122fa51dca7f796d077b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69894
Tested-by: core-ci <typo3@b13.com>
Tested-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this pull request Jul 16, 2021
@ohader
[TASK] Skip SVG sanitizer test causing segmentation fault
eac6c2f 
SVG sanitizer test dataset entity.svg is causing segmentation fault
in certain scenarios - which might be related to libxml2 before
version 2.9.12. Unfortunately, investigations did not reveal any
further details other than libxml2.

As a result `entity.svg` test dataset, which is causing this problem is
skipped until darylldoyle/svg-sanitizer#53 is
merged and released in the upstream library.

Resolves: #94565
Releases: master, 10.4, 9.5
Change-Id: I8375954dad64e3955f88122fa51dca7f796d077b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69895
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this pull request Jul 16, 2021
@ohader
[TASK] Skip SVG sanitizer test causing segmentation fault
c39c206 
SVG sanitizer test dataset entity.svg is causing segmentation fault
in certain scenarios - which might be related to libxml2 before
version 2.9.12. Unfortunately, investigations did not reveal any
further details other than libxml2.

As a result `entity.svg` test dataset, which is causing this problem is
skipped until darylldoyle/svg-sanitizer#53 is
merged and released in the upstream library.

Resolves: #94565
Releases: master, 10.4, 9.5
Change-Id: I8375954dad64e3955f88122fa51dca7f796d077b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69896
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this pull request Jul 16, 2021
@ohader
[TASK] Skip SVG sanitizer test causing segmentation fault
ecb952b 
SVG sanitizer test dataset entity.svg is causing segmentation fault
in certain scenarios - which might be related to libxml2 before
version 2.9.12. Unfortunately, investigations did not reveal any
further details other than libxml2.

As a result `entity.svg` test dataset, which is causing this problem is
skipped until darylldoyle/svg-sanitizer#53 is
merged and released in the upstream library.

Resolves: #94565
Releases: master, 10.4, 9.5
Change-Id: I8375954dad64e3955f88122fa51dca7f796d077b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69894
Tested-by: core-ci <typo3@b13.com>
Tested-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this pull request Jul 16, 2021
@ohader
[TASK] Skip SVG sanitizer test causing segmentation fault
73ef44c 
SVG sanitizer test dataset entity.svg is causing segmentation fault
in certain scenarios - which might be related to libxml2 before
version 2.9.12. Unfortunately, investigations did not reveal any
further details other than libxml2.

As a result `entity.svg` test dataset, which is causing this problem is
skipped until darylldoyle/svg-sanitizer#53 is
merged and released in the upstream library.

Resolves: #94565
Releases: master, 10.4, 9.5
Change-Id: I8375954dad64e3955f88122fa51dca7f796d077b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69895
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this pull request Jul 16, 2021
@ohader
[TASK] Skip SVG sanitizer test causing segmentation fault
6db464e 
SVG sanitizer test dataset entity.svg is causing segmentation fault
in certain scenarios - which might be related to libxml2 before
version 2.9.12. Unfortunately, investigations did not reveal any
further details other than libxml2.

As a result `entity.svg` test dataset, which is causing this problem is
skipped until darylldoyle/svg-sanitizer#53 is
merged and released in the upstream library.

Resolves: #94565
Releases: master, 10.4, 9.5
Change-Id: I8375954dad64e3955f88122fa51dca7f796d077b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69896
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
@lolli42
Copy link
Contributor

lolli42 commented Jul 17, 2021

+1 Confirmed the patch, this fixes the php segfaults we're experiencing.

reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this pull request Jul 20, 2021
@ohader
[TASK] Skip SVG sanitizer test causing segmentation fault
0c78fae 
SVG sanitizer test dataset entity.svg is causing segmentation fault
in certain scenarios - which might be related to libxml2 before
version 2.9.12. Unfortunately, investigations did not reveal any
further details other than libxml2.

As a result `entity.svg` test dataset, which is causing this problem is
skipped until darylldoyle/svg-sanitizer#53 is
merged and released in the upstream library.

Resolves: #94565
Releases: master, 10.4, 9.5
Change-Id: I8375954dad64e3955f88122fa51dca7f796d077b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69975
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this pull request Jul 20, 2021
@ohader
[TASK] Skip SVG sanitizer test causing segmentation fault
b99bf70 
SVG sanitizer test dataset entity.svg is causing segmentation fault
in certain scenarios - which might be related to libxml2 before
version 2.9.12. Unfortunately, investigations did not reveal any
further details other than libxml2.

As a result `entity.svg` test dataset, which is causing this problem is
skipped until darylldoyle/svg-sanitizer#53 is
merged and released in the upstream library.

Resolves: #94565
Releases: master, 10.4, 9.5
Change-Id: I8375954dad64e3955f88122fa51dca7f796d077b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69975
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
@lolli42
Copy link
Contributor

lolli42 commented Aug 8, 2021

Hope it is ok to ping here. :)

This issue is kinda important to us since TYPO3 added svg-sanitizer as dependency to increase security. Those segfaults are nasty and of course an issue for our application stability.
The patch looks good to me and the added tests nail the specific issue.

Is there anything left we may have missed and we could help with to get this fixed and released?

Thanks for all the work on this project. Keep rocking :)

darylldoyle
darylldoyle approved these changes Aug 9, 2021
View reviewed changes
Copy link
Owner

@darylldoyle darylldoyle left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for the work, apologies for not merging this sooner

@darylldoyle darylldoyle merged commit 61acf12 into darylldoyle:master Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darylldoyle darylldoyle darylldoyle approved these changes

@andreaskienast andreaskienast andreaskienast approved these changes

@lolli42 lolli42 lolli42 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ohader @lolli42 @darylldoyle @andreaskienast