[WIP] Test SVG sanitize #8024
[WIP] Test SVG sanitize #8024
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8024 +/- ##
==========================================
+ Coverage 41.6% 41.61% +0.01%
==========================================
Files 479 480 +1
Lines 64106 64120 +14
==========================================
+ Hits 26669 26683 +14
Misses 33979 33979
Partials 3458 3458
Continue to review full report at Codecov.
|
GiteaBot
added
the
lgtm/need 2
modules/util/svg/svg.go
Outdated
| @@ -0,0 +1,35 @@ | |||
| // Copyright 2019 The Gitea Authors. All rights reserved. | |||
There was a problem hiding this comment.
How about rename modules/util to modules/image so that we can put all image related packages there.
modules/util/svg/svg.go
Outdated
| ) | ||
|
|
||
| // MinifySVG compact svg strings | ||
| func MinifySVG(svgData io.Reader) (*bytes.Buffer, error) { |
There was a problem hiding this comment.
Since package name has svg. The function name could be simple as Minify.
There was a problem hiding this comment.
@lafriks there is a sanitize function. The main goal of minify is to be able to use the same test fixture from know lib because the indentation of bluemonday is not the same. Maybe we would kept it to serve rendered/filtered file but it is not necessary.
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions. |
|
This pull request has been automatically closed because of inactivity. You can re-open it if needed. |
|
@sapk Did you reach any conclusion on this? Difficult to implement, or to get / ensure sanitized enough? Or would this work if just integrate the Sanitize and Minify functions into the code serving the content? |
|
@davidsvantesson From what, I recall, some part are good like the test that re-use samples of good lib in another language. The cleaning would not be perfect in the sense that I don't have exactly the same result as expected but it would may be near enough but still need work to validate. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions. |
|
Thx stale bot for the reminder. 😄 |
|
@sapk go v1.14 is needed to have valid output with |
|
@6543 Yes currently setting it up. (I am on a temporary WSL debian which is not yet on go1.14 😢) |
|
I think the bluemonday poilcy need to be rethink and document what it allow. It will work to provide a safe svg but it will be at the cost of limited svg functionnality currently. At first we could allow a minimal subset of svg func for safety and increase the allowes element list with the demand. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions. |
|
status? |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions. |
|
This pull request has been automatically closed because of inactivity. You can re-open it if needed. |
Test using https://github.com/microcosm-cc/bluemonday and (will) use test fixtures from https://github.com/cure53/DOMPurify and https://github.com/darylldoyle/svg-sanitizer
Related: #1095