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.

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

replace unmaintained safemem::copy_over with slice::copy_within #208

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

syphar
Copy link
Contributor

@syphar syphar commented Feb 23, 2024

  • the safemem library is unmaintained
  • both safemem::copy_over and slice::copy_within use ptr::copy internally, so the performance impact should be minimal.
  • slice::copy_within was added in rust 1.37.0, but I don't know if lol-html has an official MSRV

rillian added a commit to brave/brave-core that referenced this pull request Mar 7, 2024
Apply syphar's patch from cloudflare/lol-html#208
to address RUSTSEC-2023-0081 about `safemem` being unmaintained.

This uses `slice::copy_within`, added in Rust 1.37.0, instead, which
performs the same panic-based bounds checks. There should be no
behaviour change. The patch is against lol_html v1.2.0, but the code
hasn't changed since our v0.3.1.
Copy link

@rillian rillian left a comment

Choose a reason for hiding this comment

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

Looks equivalent to me, FWIW.

@rillian
Copy link

rillian commented Mar 8, 2024

CI failures are unrelated clippy warnings.

@inikulin
Copy link
Contributor

inikulin commented Mar 9, 2024

Could you please rebase on top of your clippy fix?

* the `safemem` library is [unmaintained][1]
* both `safemem::copy_over` and `slice::copy_within` use `ptr::copy`
  internally, so the performance impact should me minimal.
* `slice::copy_within` was added in rust 1.37.0, but I don't know if
  lol-html has an official MSRV

[1]: https://rustsec.org/advisories/RUSTSEC-2023-0081.html
@syphar syphar force-pushed the replace-safemem branch from e5f17f9 to ffef8c1 Compare March 9, 2024 13:59
@syphar
Copy link
Contributor Author

syphar commented Mar 9, 2024

Could you please rebase on top of your clippy fix?

done

@inikulin inikulin merged commit de0461a into cloudflare:master Mar 11, 2024
4 checks passed
@inikulin
Copy link
Contributor

Thank you!

@syphar syphar deleted the replace-safemem branch March 11, 2024 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants