Update length regexes on example policies #102
Conversation
|
Hi @spassarop, Could you please share if anything is pending for this PR to merge. Is this issue #101 stands resolved now? |
|
@davewichers are you OK with this PR? |
| // When using exponential directly the "e" or "E" is internally considered as the start of | ||
| // the dimension/unit type. This creates inconsistencies that make the regex validation fail, | ||
| // also in cases like 1e4pt where "e" is considered as dimension instead of "pt". | ||
| assertThat(as.scan("<p style=\"margin: 1.0E+4pt;\">Some text.</p>", policy, AntiSamy.DOM).getCleanHTML(), not(containsString("margin"))); |
There was a problem hiding this comment.
IIRC, the GitHub issue that originally was brought up was when someone was trying to use a fractional point, something like 0.001pt or whatever. This tests for a margin of 10000pt which seems a bit excessive. (Had Fermat had that much room in his margins, "Fermat's Last Theorem" likely would have never been a thing! :)
Anyway, my point is, there should be at least something here with a negative exponent to test the original use case that was causing the problem.
There was a problem hiding this comment.
I get the point, 10000pt was just to test the behavior of a positive exponential (also 0.0001 is excesively low for a margin in my opinion). Test was introduced.
I'm OK with whatever you decide here. |
The tests have comments explaining the bug (#101) and a single case that passes but involves a latent bug.