Fix non-base64 data URLs with % characters not followed by hex digits #797

SmaugPool · 2022-10-02T13:37:01Z

When writing accumulated "non-special" characters, slice_start must be updated as some later conditionals/pattern matches don't update it like the case when % is not followed by 2 hex digits.

This fixes #795

SmaugPool · 2022-10-02T13:42:12Z

See #795 (comment) for a more detailed explanation.

All tests pass:

test result: ok. 65 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.72s

I will run more tests but some review is needed as I'm not familiar enough with the source code to be certain that does not cause regressions.

Note: I guess an alternative could be to update slice_start in the empty else when i > slice_start, but there is the risk of adding later new conditionals not updating it again. Maybe some common code with PercentDecode in percent_encoding lib could also be used to avoid duplicate and different implementations.

PS: I can add test cases to avoid later regressions if wanted.

codecov-commenter · 2022-10-02T21:35:03Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.39 🎉

Comparison is base (b228574) 82.05% compared to head (6ad3fc5) 82.44%.

❗ Current head 6ad3fc5 differs from pull request most recent head ca1ab54. Consider uploading reports for the commit ca1ab54 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #797      +/-   ##
==========================================
+ Coverage   82.05%   82.44%   +0.39%     
==========================================
  Files          20       20              
  Lines        3365     3349      -16     
==========================================
  Hits         2761     2761              
+ Misses        604      588      -16

Impacted Files	Coverage Δ
data-url/src/lib.rs	`82.75% <100.00%> (+2.92%)`	⬆️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

lucacasonato · 2023-07-11T16:02:48Z

@SmaugPool Sorry for the late reply. Could you add a test for this?

SmaugPool · 2023-07-11T22:13:43Z

Sure @lucacasonato, I have added some tests.

When writing accumulated "non-special" characters, `slice_start` must be updated as some later conditionals/pattern matches don't update it like the case when `%` is not followed by 2 hex digits. This fixes servo#795

SmaugPool · 2024-02-28T22:07:06Z

@lucacasonato any plan to merge this? It has been a while now (PR from 2022) and it fixes a significant issue with data URLs.

valenting

Thank you for the fix!

SmaugPool force-pushed the master branch from 6e37429 to e6abf03 Compare October 2, 2022 14:01

tmccombs approved these changes Oct 2, 2022

View reviewed changes

SmaugPool force-pushed the master branch from e6abf03 to 5011aa5 Compare July 11, 2023 22:11

SmaugPool force-pushed the master branch 4 times, most recently from 17d8c93 to 6ad3fc5 Compare July 12, 2023 06:25

Fix non-base64 data URLs with % character not followed by hex digits

ca1ab54

When writing accumulated "non-special" characters, `slice_start` must be updated as some later conditionals/pattern matches don't update it like the case when `%` is not followed by 2 hex digits. This fixes servo#795

SmaugPool force-pushed the master branch from 3fca6c0 to ca1ab54 Compare July 12, 2023 19:16

valenting requested a review from lucacasonato July 15, 2023 15:34

valenting enabled auto-merge March 4, 2024 10:14

valenting approved these changes Mar 4, 2024

View reviewed changes

valenting added this pull request to the merge queue Mar 4, 2024

Merged via the queue into servo:master with commit e654efb Mar 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix non-base64 data URLs with % characters not followed by hex digits #797

Fix non-base64 data URLs with % characters not followed by hex digits #797

Fix non-base64 data URLs with % characters not followed by hex digits #797

Fix non-base64 data URLs with % characters not followed by hex digits #797

Conversation

Codecov Report

Choose a reason for hiding this comment