WebBluetoothCG / web-bluetooth

Bluetooth support for the Web.
http://www.w3.org/community/web-bluetooth/
Other
1.38k stars 187 forks source link

Handling readValue from protected/secure characteristics of not-yet-PIN-paired devices is underspecified #563

Closed ariccio closed 2 years ago

ariccio commented 2 years ago

After a few days of looking at this, I believe that this is indeed an issue with the spec. I understand PIN codes are a work in progress for the group, and I hope you'll appreciate this.

Essentially, there's no proper way to recover when a call to a readValue on a characteristic fails because the PIN code has not yet been entered. In current Chrome Canary, this usually causes the promise to immediately reject with a "NetworkError: Authentication canceled", even while the PIN dialog is still open, leaving the app without a promise to wait on for when the PIN is actually entered and full pairing completes. Manually reloading the page after entering the PIN works (of course), since the device is now correctly paired.

The most reasonable way I can think of to deal with this right now is to do the classic wrong thing: wait a few seconds and try again with a recursive retry countdown. I could simply as end users to reload the page after entering the PIN, but that friction is going to increase the bounce rate a lot.

The Aranet4 is a BLE device (and quite a well built BLE device at that), which I suspect has something to do with it because of on the fly pairing. This, I strongly believe, is not a lower-level issue in the stack. If WinDbg is connected to chrome during the attempted readValue, the operating system emits a log message: "The attribute requires authentication before it can be read or written.", which indicates to me that everything below the browser is working correctly.

After reading through a couple thousand lines of chromium source and enough of the spec document, I think chrome's implementation is conforming with the draft standard, it's just not yet complete.

What would be the best behavior for a web bluetooth standard? Waiting for the PIN dialog to complete? A different type of exception raised to the user? A mechanism to check before calling readValue?

Background, for the curious

reillyeon commented 2 years ago

The Promise returned by the readValue() call should not settle until pairing is complete. However, the current implementation only allows a single pending pairing operation and so if additional calls to readValue() are made they will fail until pairing is complete.

reillyeon commented 2 years ago

I think the specification does provide sufficient language here as the steps for handling errors include attempting to pair before retrying the operation, as Chromium now does. I think it can be considered an implementation bug that on a platform (Windows) which otherwise supports multiple parallel calls to methods like readValue() it does not support parallel requests for pairing. I'll add a note referencing this issue to Chromium issue 960258. If you agree we can close this issue.

reillyeon commented 2 years ago

Looking at the sample code I see that there are indeed multiple calls to readValue() being made in parallel, one for each characteristic:

return this.getGATTServer()
  .then(server => server.getPrimaryService(serviceUuid))
  .then(service => service.getCharacteristics())
  .then(characteristics => {
    return Promise.all(
      characteristics
        .filter(characteristic => characteristicUuids.includes(characteristic.uuid))
        .map(async characteristic => ({
          uuid: characteristic.uuid,
          value: await characteristic.readValue(),
       })),
    )
  })
  .then(values => values.map(value => ({
    uuid: value.uuid,
    value: characteristicResolvers[value.uuid](value.value),
  })))

(From https://github.com/kasparsd/sensor-pilot/blob/master/src/ble-device.js.)

ariccio commented 2 years ago

...oy, I think you may be right. I think this makes perfect (annoying) sense.

His code is a bit more deeply chained than I'd ever write, so it's hard to trace where the exception is thrown (the second request?)... It's also a bit more confusing than I expect, because the "NetworkError: Authentication canceled" error message does not suggest the cause was something other than multiple requests in flight. I also sometimes (rarely) see a "DOMException: Connection already in progress.", which is more informative. The exception is also not immediately raised on the second call to readValue, which I do not understand. I guess chrome must not check whether one is in flight before queueing the task internally?

It sounds to me like the correct fix is going to take a promise queue, which is a headache, but entirely possible. I'll close the issue as soon as I understand what you mean by an implementation bug in windows. Is it documented that windows doesn't support multiple parallel pairing requests? Hmmm. If so, would it make more sense for it to be up to the browser or the client code to make sure there's only one pairing request at a time?

reillyeon commented 2 years ago

The implementation bug is that Windows supports multiple concurrent calls to readValue() but our implementation of automatic pairing doesn't so without the secure characteristic this code works fine but when pairing is required it fails.

Sadly even if we fix the implementation of automatic pairing on Windows to support concurrent calls other platforms (Android and macOS) do not and so your code will need to use a Promise queue either way.

ariccio commented 2 years ago

Alrighty, thanks so much for being responsive and helpful.

hthetiot commented 1 year ago

The implementation bug is that Windows supports multiple concurrent calls to readValue() but our implementation of automatic pairing doesn't so without the secure characteristic this code works fine but when pairing is required it fails.

I confirm, disabling Nimble "Security manager secure connections" solved my issue.