erwanvivien / fast_qr

Ultra fast QRCode generation
https://fast-qr.com/
MIT License
197 stars 29 forks source link

Remove wee_alloc. #22

Closed AntoniosBarotsis closed 1 year ago

AntoniosBarotsis commented 1 year ago

wee_alloc is no longer maintained and has a few critical open issues (memory leaks), see here.

$ cargo audit
    Fetching advisory database from `https://github.com/RustSec/advisory-db.git`
      Loaded 511 security advisories (from C:\Users\anton\.cargo\advisory-db)
    Updating crates.io index
    Scanning Cargo.lock for vulnerabilities (121 crate dependencies)
Crate:     wee_alloc
Version:   0.4.5
Warning:   unmaintained
Title:     wee_alloc is Unmaintained
Date:      2022-05-11
ID:        RUSTSEC-2022-0054
URL:       https://rustsec.org/advisories/RUSTSEC-2022-0054
Dependency tree:
wee_alloc 0.4.5
└── fast_qr 0.8.4

This reverts back to the recommended dlmalloc-rs.

I ran the wasm-pack.sh file and everything seems fine but I have not tested this from JS directly. A few other PRs seem to just remove the extra wee_alloc stuff with no other change so this should be fine. I have also not committed any of the changes that happen to the pkg/* files when I run wasm-pack.sh, should I do that?

erwanvivien commented 1 year ago

It is fine not to commit the pkg, I do that when upgrading version usually.

I will look at that, since it really helps reducing the size of the wasm package, thanks for the hindsight

AntoniosBarotsis commented 1 year ago

There's also lol_alloc but the author mentioned how it's not really tested in production.

erwanvivien commented 1 year ago

I think we'll just go with no alloc, thus using dlmalloc-rs. Thanks Antonios

After trying, it "only" adds 2kB to the gzip archive, while it matters, it's better than leaking memory

maciejhirsz commented 1 year ago

Hey @erwanvivien, can we get this patch published on crates.io? I've a project that uses fast_qr in Wasm, and GitHub's security bot keeps giving me grief :upside_down_face:.

erwanvivien commented 1 year ago

Dang, super sorry, I really thought I had done it. Will do right now

erwanvivien commented 1 year ago

@maciejhirsz Update to 8.5 :) Again, sorry for the delay