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

Fixes issue #94521: Make sure to always specify Uint32Array length #94529

Merged
merged 1 commit into from
Apr 6, 2020

Conversation

alexdima
Copy link
Member

@alexdima alexdima commented Apr 6, 2020

This PR fixes #94521 for the 1.44 release.

When the decodeSemanticTokensDto was called, it was possible that it was called with a VSBuffer that uses a Uint8Array which is misaligned on a backing ArrayBuffer. Think about a large data blob (a message from the ext host) and only a portion of it is the encoded semantic tokens dto.

The code handled the misaligned case before (see the check uint8Arr.byteOffset % 4 === 0), but the handling was incomplete because a length was not passed to the Uint32Array ctor. When the length is not passed in, the Uint32Array will try to use the ArrayBuffer.byteLength / 4, but that might be beyond the length of the passed in VSBuffer into memory of the backing ArrayBuffer, which might not be a multiple of 4.

The unit tests show this case clearly, but unfortunately there are no steps given this has to do with the way in which the extension host can end up serializing a batch of messages into a single blob...

@alexdima alexdima self-assigned this Apr 6, 2020
@alexdima alexdima added the candidate Issue identified as probable candidate for fixing in the next release label Apr 6, 2020
@alexdima alexdima added this to the March 2020 milestone Apr 6, 2020
@alexdima alexdima changed the title Fixes issue #94521: Make sure to always specify Uint32Array length in… Fixes issue #94521: Make sure to always specify Uint32Array length Apr 6, 2020
@alexdima alexdima requested review from aeschli and jrieken April 6, 2020 12:09
@alexdima alexdima removed the request for review from aeschli April 6, 2020 12:47
@alexdima alexdima merged commit c5eaf6d into release/1.44 Apr 6, 2020
@alexdima alexdima deleted the alex/issue/94521 branch April 6, 2020 13:41
@github-actions github-actions bot locked and limited conversation to collaborators May 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
candidate Issue identified as probable candidate for fixing in the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants