fix(osx): prevent bluetooth connect crashes from missing device metadata.#709
Open
giovanninibarbosa wants to merge 5 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the macOS Bluetooth native interop path against missing device metadata, especially null native strings in connect/disconnect notifications and paired-device enumeration.
Changes:
- Adds safe Objective-C
NSStringto UTF-8 C string copying helpers in native Bluetooth code. - Guards watcher connect/disconnect callbacks against missing addresses and optional names.
- Adds managed-side null pointer handling before converting native UTF-8 strings.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
GalaxyBudsClient.Platform.OSX/Native/src/BluetoothDeviceWatcher.mm |
Adds safe string copying and address validation for native connect/disconnect notifications. |
GalaxyBudsClient.Platform.OSX/Native/src/Bluetooth.mm |
Adds safe enumeration string allocation and cleanup for paired Bluetooth devices. |
GalaxyBudsClient.Platform.OSX/BluetoothService.cs |
Centralizes native UTF-8 pointer conversion and applies it to enumeration and watcher callbacks. |
Comments suppressed due to low confidence (1)
GalaxyBudsClient.Platform.OSX/Native/src/Bluetooth.mm:246
- When
addressStringis missing,FirstNonEmptyStringturns it into an empty string and the enumeration still returns success for that device. That exposes aBluetoothDevicewith an emptyAddress; for example the manual pairing dialog registers the selected device address without revalidating it, and nativebt_connectcannot resolve an empty address. Missing addresses should be filtered out or treated as an enumeration failure rather than returned as valid devices.
NSString *addressString = FirstNonEmptyString([device addressString], nil, nil);
NSString *nameString = FirstNonEmptyString([device name], [device nameOrAddress], addressString);
resultDevice->device_name = CopyNSStringToUtf8CString(nameString);
resultDevice->mac_address = CopyNSStringToUtf8CString(addressString);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Extract IsNullOrEmpty, FirstNonEmptyString and CopyNSStringToUtf8CString into Native/src/NativeStringUtils.h so the paired-device enumeration and watcher callback paths share a single definition instead of keeping two verbatim copies in lockstep.
Skip IOBluetoothDevice entries whose addressString is null or empty instead of returning them with an empty mac_address. Empty addresses flowed through to the manual pairing dialog and bt_connect, which cannot resolve them. result->length now reflects the count of usable devices populated in the loop.
d2028ea to
34cf2ee
Compare
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.
Summary
This fixes a macOS Bluetooth crash where connect or disconnect notifications can arrive without complete device metadata. The native watcher previously copied
NSString.UTF8Stringvalues directly into C strings, which could dereference a null pointer before the managed callback was reached.Tasks
Verification
git diff --check.xcodebuild -project GalaxyBudsClient.Platform.OSX/Native/NativeInterop.xcodeproj -scheme NativeInterop -destination platform=macOS build.Not Run
dotnet restore --configfile GalaxyBudsClient/nuget.config GalaxyBudsClient/GalaxyBudsClient.csproj:dotnetis not available in this shell.dotnet build GalaxyBudsClient.sln -c Debug:dotnetis not available in this shell.dotnet test GalaxyBudsClient.Tests/GalaxyBudsClient.Tests.csproj -c Debug:dotnetis not available in this shell.Notes
There is no dedicated pull request template in
docs/or.github; this draft follows the docs contribution guidance to explain the changes.