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

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

Merged
merged 1 commit into from
Mar 4, 2024
Merged

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

merged 1 commit into from
Mar 4, 2024

Conversation

SmaugPool
Copy link
Contributor

@SmaugPool SmaugPool commented Oct 2, 2022 •
edited
Loading

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
Copy link
Contributor Author

SmaugPool commented Oct 2, 2022 •
edited
Loading

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
Copy link

codecov-commenter commented Oct 2, 2022 •
edited by codecov bot
Loading

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
Copy link
Collaborator

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

@SmaugPool
Copy link
Contributor Author

SmaugPool commented Jul 11, 2023 •
edited
Loading

Sure @lucacasonato, I have added some tests.

@SmaugPool SmaugPool force-pushed the master branch 4 times, most recently from 17d8c93 to 6ad3fc5 Compare July 12, 2023 06:25
@SmaugPool
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
Copy link
Contributor Author

@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 valenting enabled auto-merge March 4, 2024 10:14
valenting
valenting approved these changes Mar 4, 2024
View reviewed changes
Copy link
Collaborator

@valenting valenting left a comment

Choose a reason for hiding this comment

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

Thank you for the fix!

@valenting 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
12 checks passed
Boshen pushed a commit to oxc-project/oxc that referenced this pull request Jun 17, 2024
@renovate
chore(deps): update rust crates (#3709)
8dc985a 
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [criterion2](https://bheisler.github.io/criterion.rs/book/index.html) ([source](https://togithub.com/Boshen/criterion2.rs)) | workspace.dependencies | minor | `0.10.0` -> `0.11.0` |
| [memchr](https://togithub.com/BurntSushi/memchr) | workspace.dependencies | patch | `2.7.2` -> `2.7.4` |
| [oxc-browserslist](https://togithub.com/oxc-project/oxc-browserslist) | workspace.dependencies | patch | `0.17.0` -> `0.17.1` |
| [url](https://togithub.com/servo/rust-url) | workspace.dependencies | patch | `2.5.0` -> `2.5.1` |

---

### Release Notes

<details>
<summary>Boshen/criterion2.rs (criterion2)</summary>

### [`v0.11.0`](https://togithub.com/Boshen/criterion2.rs/blob/HEAD/CHANGELOG.md#0110---2024-06-14)

[Compare Source](https://togithub.com/Boshen/criterion2.rs/compare/v0.10.0...v0.11.0)

##### Added

-   \[**breaking**] remove csv_output ([#&#8203;33](https://togithub.com/Boshen/criterion2.rs/pull/33))
-   rm crate `criterion-macro`

##### Other

-   *(deps)* update dependency rust to v1.79.0 ([#&#8203;35](https://togithub.com/Boshen/criterion2.rs/pull/35))
-   *(deps)* update rust crates ([#&#8203;32](https://togithub.com/Boshen/criterion2.rs/pull/32))
-   *(deps)* lock file maintenance rust crates ([#&#8203;31](https://togithub.com/Boshen/criterion2.rs/pull/31))
-   check unused dependencies

</details>

<details>
<summary>BurntSushi/memchr (memchr)</summary>

### [`v2.7.4`](https://togithub.com/BurntSushi/memchr/compare/2.7.3...2.7.4)

[Compare Source](https://togithub.com/BurntSushi/memchr/compare/2.7.3...2.7.4)

### [`v2.7.3`](https://togithub.com/BurntSushi/memchr/compare/2.7.2...2.7.3)

[Compare Source](https://togithub.com/BurntSushi/memchr/compare/2.7.2...2.7.3)

</details>

<details>
<summary>oxc-project/oxc-browserslist (oxc-browserslist)</summary>

### [`v0.17.1`](https://togithub.com/oxc-project/oxc-browserslist/blob/HEAD/CHANGELOG.md#0171---2024-06-17)

[Compare Source](https://togithub.com/oxc-project/oxc-browserslist/compare/oxc-browserslist-v0.17.0...oxc-browserslist-v0.17.1)

##### Other

-   *(deps)* update npm packages ([#&#8203;45](https://togithub.com/oxc-project/oxc-browserslist/pull/45))

</details>

<details>
<summary>servo/rust-url (url)</summary>

### [`v2.5.1`](https://togithub.com/servo/rust-url/releases/tag/v2.5.1)

[Compare Source](https://togithub.com/servo/rust-url/compare/v2.5.0...v2.5.1)

#### What's Changed

-   Be more detailed in documentation of set_query by [@&#8203;philippeitis](https://togithub.com/philippeitis) in [servo/rust-url#737
-   perf(punycode): avoid double allocation in decode_to_string by [@&#8203;bishopcheckmate](https://togithub.com/bishopcheckmate) in [servo/rust-url#894
-   Use SPECIAL_PATH_SEGMENT when encoding path in from_file_path by [@&#8203;valenting](https://togithub.com/valenting) in [servo/rust-url#902
-   Add dependabot by [@&#8203;oriontvv](https://togithub.com/oriontvv) in [servo/rust-url#903
-   Bump codecov/codecov-action from 3 to 4 by [@&#8203;dependabot](https://togithub.com/dependabot) in [servo/rust-url#904
-   Bump actions/upload-artifact from 2 to 4 by [@&#8203;dependabot](https://togithub.com/dependabot) in [servo/rust-url#905
-   Bump actions/checkout from 3 to 4 by [@&#8203;dependabot](https://togithub.com/dependabot) in [servo/rust-url#906
-   Fix non-base64 data URLs with % characters not followed by hex digits by [@&#8203;SmaugPool](https://togithub.com/SmaugPool) in [servo/rust-url#797
-   Rename `master` branch to `main` by [@&#8203;mrobinson](https://togithub.com/mrobinson) in [servo/rust-url#914
-   Add bench for to_ascii on an already-Punycode name by [@&#8203;hsivonen](https://togithub.com/hsivonen) in [servo/rust-url#915
-   Update URLs by [@&#8203;atouchet](https://togithub.com/atouchet) in [servo/rust-url#916
-   Fix lint by [@&#8203;valenting](https://togithub.com/valenting) in [servo/rust-url#920
-   Fix multiple issues on wasm32, and runs url tests in CI by [@&#8203;micolous](https://togithub.com/micolous) in [servo/rust-url#886
-   Non-special URLs can have their paths erased by [@&#8203;DylanOToole2](https://togithub.com/DylanOToole2) in [servo/rust-url#921
-   docs: document SyntaxViolation variants, remove bare URLs by [@&#8203;aatifsyed](https://togithub.com/aatifsyed) in [servo/rust-url#924
-   docs: Document possible replacements of the base URL by [@&#8203;mo8it](https://togithub.com/mo8it) in [servo/rust-url#926
-   Reimplement idna on top of ICU4X by [@&#8203;hsivonen](https://togithub.com/hsivonen) in [servo/rust-url#923

#### New Contributors

-   [@&#8203;philippeitis](https://togithub.com/philippeitis) made their first contribution in [servo/rust-url#737
-   [@&#8203;bishopcheckmate](https://togithub.com/bishopcheckmate) made their first contribution in [servo/rust-url#894
-   [@&#8203;oriontvv](https://togithub.com/oriontvv) made their first contribution in [servo/rust-url#903
-   [@&#8203;dependabot](https://togithub.com/dependabot) made their first contribution in [servo/rust-url#904
-   [@&#8203;SmaugPool](https://togithub.com/SmaugPool) made their first contribution in [servo/rust-url#797
-   [@&#8203;hsivonen](https://togithub.com/hsivonen) made their first contribution in [servo/rust-url#915
-   [@&#8203;micolous](https://togithub.com/micolous) made their first contribution in [servo/rust-url#886
-   [@&#8203;DylanOToole2](https://togithub.com/DylanOToole2) made their first contribution in [servo/rust-url#921
-   [@&#8203;aatifsyed](https://togithub.com/aatifsyed) made their first contribution in [servo/rust-url#924
-   [@&#8203;mo8it](https://togithub.com/mo8it) made their first contribution in [servo/rust-url#926

**Full Changelog**: servo/rust-url@v2.5.0...v2.5.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 10am on monday" in timezone Asia/Shanghai, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/oxc-project/oxc).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zOTMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjM5My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@valenting valenting valenting approved these changes

@tmccombs tmccombs tmccombs approved these changes

@lucacasonato lucacasonato Awaiting requested review from lucacasonato

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

Successfully merging this pull request may close these issues.

Corrupted data URL output from <rect style='fill:%23000000;' width='100%' height='100%'/>
5 participants
@SmaugPool @codecov-commenter @lucacasonato @valenting @tmccombs