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

web-sys: unnecessary &mut requirement in WebUSB functions, potentially others #3963

Open
spookyvision opened this issue May 15, 2024 · 3 comments

Comments

@spookyvision
Copy link

spookyvision commented May 15, 2024

several WebUSB functions that take a [u8] slice, e.g., transfer_out_with_u8_array, require it be passed as &mut. As far as I can tell this is not required since USB transfers are unidirectional.

full list of affected functions:

other APIs might also be affected - I searched the docs for with_u8_array and found e.g. ReadableByteStreamController::enqueue_with_u8_array taking a &mut buffer which at first glance is only used for reading.

@Liamolucko
Copy link
Collaborator

This is the case because web-sys is auto-generated from WebIDL, which doesn't include any information about whether or not buffers passed to functions are mutable.

Because of that, it conservatively assumes that all buffers are mutable, since it can't result in UB if a web API tries to mutate an immutable buffer and it's backwards-compatible to change a slice from mutable to immutable.

That change is made by adding the functions whose slices should be immutable to a big list:

pub(crate) static IMMUTABLE_SLICE_WHITELIST: Lazy<BTreeSet<&'static str>> = Lazy::new(|| {
BTreeSet::from_iter(vec![
// ImageData
"ImageData",
// WebGlRenderingContext, WebGl2RenderingContext
"uniform1fv",
"uniform2fv",
"uniform3fv",
"uniform4fv",
"uniform1iv",
"uniform2iv",
"uniform3iv",
"uniform4iv",
"uniformMatrix2fv",
"uniformMatrix3fv",
"uniformMatrix4fv",
"uniformMatrix2x3fv",
"uniformMatrix2x4fv",
"uniformMatrix3x2fv",
"uniformMatrix3x4fv",
"uniformMatrix4x2fv",
"uniformMatrix4x3fv",
"vertexAttrib1fv",
"vertexAttrib2fv",
"vertexAttrib3fv",
"vertexAttrib4fv",
"bufferData",
"bufferSubData",
"texImage2D",
"texSubImage2D",
"compressedTexImage2D",
// WebGl2RenderingContext
"uniform1uiv",
"uniform2uiv",
"uniform3uiv",
"uniform4uiv",
"texImage3D",
"texSubImage3D",
"compressedTexImage3D",
"clearBufferfv",
"clearBufferiv",
"clearBufferuiv",
// WebSocket
"send",
// WebGPU
"setBindGroup",
"writeBuffer",
"writeTexture",
// AudioBuffer
"copyToChannel",
// FontFace
"FontFace", // TODO: Add another type's functions here. Leave a comment header with the type name
// FileSystemSyncAccessHandle and FileSystemWritableFileStream
"write",
// SubtleCrypto
"encrypt",
"decrypt",
"digest",
"sign",
"unwrapKey",
"verify",
])
});
.

@xpe
Copy link

xpe commented May 27, 2024

@spookyvision Do you want to see if WebIDL is aware of the limitation? Solve the problem upstream?

@spookyvision
Copy link
Author

I did a quick scan of the WebIDL spec and it doesn't seem they have a concept of (im)mutability, sadly. I might be wrong, but since the spec refers JavaScript a lot, it's probably closely modeled based on JS semantics.

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

No branches or pull requests

3 participants