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

fix(NODE-6608): calculateObjectSize returns the wrong value for bigint #742

Merged
merged 9 commits into from
Dec 17, 2024

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Dec 16, 2024

Description

What is changing?

  • Introduce a bigint condition to the switch statement in calculateObjectSize that adds 8 to the size
  • Introduce a symbol condition that returns 0
  • Introduce a default condition that throws an error if there are future js types that the library cannot understand
Is there new documentation needed for these changes?

No

What is the motivation for this change?

This fixes a bug where the serializer produces something of a different size than what the calculate function claims it will.

Release Highlight

Fix calculateObjectSize not accounting for BigInt value size

BSON.calculateObjectSize was missing a condition for BigInt values, meaning it did not account for them in the same way that it would for Long values. This has been corrected.

We also added a new default condition that will catch any new values that may be returned by typeof in the future and will throw an error rather than returning an inaccurate size.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@nbbeeken nbbeeken marked this pull request as ready for review December 16, 2024 16:18
addaleax
addaleax previously approved these changes Dec 16, 2024
test/node/parser/calculate_size.test.ts Outdated Show resolved Hide resolved
addaleax
addaleax previously approved these changes Dec 16, 2024
@dariakp dariakp added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Dec 16, 2024
@dariakp dariakp self-assigned this Dec 16, 2024
@nbbeeken nbbeeken requested a review from dariakp December 16, 2024 22:12
@dariakp dariakp merged commit 1fed073 into main Dec 17, 2024
8 checks passed
@dariakp dariakp deleted the NODE-6608-calculateSize-bigint branch December 17, 2024 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Primary Review In Review with primary reviewer, not yet ready for team's eyes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants