Sanitize href values
#249
Sanitize href values
#249
Conversation
Codecov Report
@@ Coverage Diff @@
## master #249 +/- ##
==========================================
+ Coverage 99.72% 99.72% +<.01%
==========================================
Files 1 1
Lines 363 365 +2
Branches 57 58 +1
==========================================
+ Hits 362 364 +2
Misses 1 1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #249 +/- ##
==========================================
+ Coverage 99.72% 99.72% +<.01%
==========================================
Files 1 1
Lines 365 367 +2
Branches 58 59 +1
==========================================
+ Hits 364 366 +2
Misses 1 1
Continue to review full report at Codecov.
|
|
@coreyward this'll need a rebase - merged another PR that handled the uppercase version of script & style tags |
coreyward
force-pushed
the
sanitize-html-links
branch
from
024d45a to
4ddd526
Compare
|
@probablyup Done. Went ahead and dropped the functionally-identical commit. |
| version "1.2.7" | ||
| resolved "https://registry.yarnpkg.com/fsevents/-/fsevents-1.2.7.tgz#4851b664a3783e52003b3c66eb0eee1074933aa4" | ||
| integrity sha512-Pxm6sI2MeBD7RdD12RYsqaP0nMiwx8eZBXCa6z2L+mRHm2DYrOYwihmhjpkdjUHwQhslWQjRpEgNq4XvBmaAuw== | ||
| fsevents@^1.0.0, fsevents@^1.2.3, fsevents@^1.2.7, fsevents@^1.2.9: |
There was a problem hiding this comment.
can you drop these lockfile changes?
There was a problem hiding this comment.
This was deliberate to get Node v12+ support. I can pull it but it shouldn’t impact downstream dependents, it’ll just mean I can’t contribute.
| @@ -374,6 +374,8 @@ function attributeValueToJSXPropValue(key, value) { | |||
|
|
|||
| return styles; | |||
| }, {}); | |||
| } else if (key === 'href') { | |||
There was a problem hiding this comment.
is this needed for src too on image tags for example?
There was a problem hiding this comment.
Good call, I believe so. I’ll check and add support for this if needed.
There was a problem hiding this comment.
Actually, it looks like only ≤ IE6 are affected by this (modern browsers don't execute javascript-scheme src attrs), and since ReactDOM doesn't work on IE6, I think we're in the clear.
There was a problem hiding this comment.
Ok but we still need it for href?
There was a problem hiding this comment.
Yep; React will gleefully pass these through and browsers interpret them.
quantizor
changed the title
href values, fix script/style sanitizationhref values
|
Released as 6.10.2 😄 |
Previously links like
<a href="javascript:alert('hi')">Foo</a>were passed through unsanitized while[javascript:alert('hi')](Foo)was sanitized. This change makes both sanitized, and also escapes any otherhrefattributes.This fixes #239. This also includes a change to prevent element name case from subverting the HTML element blacklist.