Skip to content

Update RTCSession configuration to non deprecated names. #3267

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

Merged
merged 4 commits into from
May 19, 2025

Conversation

toger5
Copy link
Contributor

@toger5 toger5 commented May 13, 2025

As introduced in: matrix-org/matrix-js-sdk#4714

Is blocked until we bump the js-sdk to a version supporting the new names.

This pr now also updates the js-sdk version

@toger5 toger5 requested a review from a team as a code owner May 13, 2025 19:55
@toger5 toger5 requested a review from AndrewFerr May 13, 2025 19:55
@toger5 toger5 added the PR-Task Release note category. A PR that is hidden from release note. label May 13, 2025
@toger5 toger5 force-pushed the toger5/update-to-js-sdk-non-deprecated-names branch from 62fa425 to 1c975ab Compare May 13, 2025 20:02
@toger5 toger5 force-pushed the toger5/update-to-js-sdk-non-deprecated-names branch from 2aecd64 to 7f90a0b Compare May 14, 2025 10:02
@fkwp fkwp added the X-Blocked Cannot be merged due to external dependencies label May 14, 2025
Copy link
Member

@BillCarsonFr BillCarsonFr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what is the correct way to fix the ArrayBuffer thing... I put some suggestions

@@ -44,7 +44,7 @@ export class MatrixKeyProvider extends BaseKeyProvider {
encryptionKeyIndex: number,
participantId: string,
): void => {
createKeyMaterialFromBuffer(encryptionKey).then(
createKeyMaterialFromBuffer(encryptionKey.buffer as ArrayBuffer).then(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This warning is a bit annoying, a .buffer is not necessarly an ArrayBuffer?
I wonder why we are having this warning only now... TBH I don't understand

At the end it is calling subtle crypto importKey that accepts An ArrayBuffer, a TypedArray, a DataView...

I don't know what is the best way to fix that.
We change our API to use ArrayBuffer? We call directly import key? or we request a change to createKeyMaterialFromBuffer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So could be that:

   crypto.subtle.importKey('raw', encryptionKey, 'HKDF', false, [
      'deriveBits',
      'deriveKey',
    ]).then(

If not what do we do if encryptionKey.buffer is not an ArrayBuffer? Maybe we could throw an error ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a good idea to me. I just added a commit patching this.

@toger5 toger5 requested a review from BillCarsonFr May 19, 2025 15:02
@toger5 toger5 force-pushed the toger5/update-to-js-sdk-non-deprecated-names branch from e54e966 to 832972b Compare May 19, 2025 15:06
@toger5
Copy link
Contributor Author

toger5 commented May 19, 2025

I just tried a to-device call with this and .dev and they seem to understand their keys (so we dont break the key format in a unexpected way. Which is the thing i was most concerned about)

@toger5 toger5 enabled auto-merge (squash) May 19, 2025 16:01
@toger5 toger5 disabled auto-merge May 19, 2025 16:03
@toger5 toger5 merged commit 28246ef into livekit May 19, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Task Release note category. A PR that is hidden from release note. X-Blocked Cannot be merged due to external dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants