fix(webbluetooth): read characteristic values via the DataView's buffer - #942
Open
cfanboy wants to merge 1 commit into
Open
fix(webbluetooth): read characteristic values via the DataView's buffer#942cfanboy wants to merge 1 commit into
cfanboy wants to merge 1 commit into
Conversation
`readValue()` resolves to a DataView. `Uint8Array::new(&data_view)` treats it as a plain array-like (length undefined) and yields an empty array, so the subsequent `copy_to` length assertion panics and kills the wasm instance. Any protocol that performs a hardware read during init (e.g. sensee-v2) dies right after chooser pairing. Build the Uint8Array over the DataView's underlying buffer instead, honoring its byte offset/length — the Subscribe notification handler already uses the buffer-based pattern. Verified against a physical Sensee Capsule (CCPA10S2) via Chrome/macOS: the device identifies and runs through the browser embedded server. Fixes buttplugio#941 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QitzJcyNqb1CHnMJ8rcZP1
|
UPPower seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #941 (maintainer green-lit the PR in the issue thread).
Problem
In the
WebBluetoothDeviceCommand::Readhandler,readValue()resolves to a DataView, but the bytes were extracted withUint8Array::new(&data_view). In JS,new Uint8Array(dataView)treats the DataView as a plain array-like (length === undefined), producing an empty array. The subsequentcopy_tothen trips js-sys's length assertion and panics, killing the whole wasm instance:Net effect: any protocol that performs a hardware read during init dies silently right after chooser pairing, and the embedded server is dead from that point on. Native btleplug paths are unaffected, which makes this confusing to diagnose from the outside.
Fix
Build the
Uint8Arrayover the DataView's underlyingbuffer(), honoring its byte offset/length — the Subscribe notification handler already uses the buffer-based pattern. One expression changed, plus a comment explaining whyUint8Array::newmust not be used here.Verification
cargo check -p buttplug_server_hwmgr_webbluetooth --target wasm32-unknown-unknownpasses.CCPA10S2, protocolsensee-v2, which reads model data from Tx during init) on Chrome/macOS: with this patch in our wasm build, the device identifies and runs (vibrate + constrict) through the browser embedded server; without it, the panic above reproduces 100% after pairing.