web_sys::ImageData Constructor is Not Sound

[anlumo]

Describe the Bug

The two constructors for web_sys::ImageData take a Clamped<&[u8]>, which is a reference. However, ImageData does not copy this data to its own buffer, so the resulting JS object depends on local data in a way that the Rust compiler is not aware of it.

Steps to Reproduce

Example code:

use wasm_bindgen::Clamped;
use web_sys::ImageData;

#[wasm_bindgen]
pub fn foo() -> ImageData {
    let bitmap = vec![0u8; 100 *100 * 4];
    ImageData::new_with_u8_clamped_array_and_sh(Clamped(&bitmap), 100, 100)
}

Then use that code in a wasm module and call it from JavaScript. In JavaScript, store that ImageData and use it for a while.

Expected Behavior

The ImageData stays the same.

Actual Behavior

After a few operations on the wasm side, data corruption in the ImageData occurs.

In our project, it looks like this:

The noise above used to be proper text.

Additional Context

A workaround would be to create a new Uint8ClampedArray and store it there, since then the JS runtime is responsible for memory management. However, unlike in JavaScript directly, in web_sys there is no constructor exposed that can take a Uint8ClampedArray.

What works for me is to return a Uint8ClampedArray, width and height from the function instead and construct the ImageData on the JavaScript side. However, this is not a general solution since sometimes you might want to use the ImageData directly in Rust.

Also, the two constructors of ImageData exposed right now should be marked as unsafe, since they're not making sure that the data referenced is staying alive during the runtime of the ImageData.

[anlumo]

Related to #423

[alexcrichton]

Unfortunately I think this is an example where we can't guarantee that everything in web-sys is 100% safe. We do our best but sometimes you'll also need to be aware of what API is being called. We could try to whitelist these perhaps to do some copies as well, but that's not necessarily always desired.

In general I'm not really sure what a fix for this would look like, if any.

[daxpedda]

Closing as duplicate of #423.