Skip to content

Added trimming to ComputeHashValue to handle names over 4096 bytes. - #4941

Merged
OsirisTerje merged 3 commits into
nunit:mainfrom
WillRayAtPropertyMe:Issue4937_PartitionComputeHashValue
Mar 1, 2025
Merged

OsirisTerje merged 3 commits into
nunit:mainfrom
WillRayAtPropertyMe:Issue4937_PartitionComputeHashValue

Conversation

@WillRayAtPropertyMe

Copy link
Copy Markdown
Contributor

This PR addresses #4937 by adding in trimming for names above 4096 bytes, including a unit test to check for this case.

@WillRayAtPropertyMe

Copy link
Copy Markdown
Contributor Author

@dotnet-policy-service agree company="PropertyMe"

@mikkelbu mikkelbu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't know much about Encoder, but as far as I can tell this looks sound.

Is it possible to have a test that it actually uses the first 4096 bytes in hashvalue (I guess it don't have to be exactly 4096)? AFAICT the added tests would also pass if the code was accidentally changed to e.g. only use the first couple bytes.

@OsirisTerje
OsirisTerje merged commit 514da11 into nunit:main Mar 1, 2025
@WillRayAtPropertyMe

Copy link
Copy Markdown
Contributor Author

Hey @mikkelbu the two tests provided are both over 4096 bytes. Just to clarify, the concern is if those tests were changed to be less than 4096 bytes - they would still pass?

@mikkelbu

Copy link
Copy Markdown
Member

Hey @mikkelbu the two tests provided are both over 4096 bytes. Just to clarify, the concern is if those tests were changed to be less than 4096 bytes - they would still pass?

Sorry about the long answer time. I was just wondering if it would make sense to have a test that checks that two names that are distinct (but after 4096 bytes) are considered equal by ComputeHashValue), but I don't think this add much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PartitionFilter.ComputeHasValue throws exception when test name is greater than 4096 characters

3 participants