Skip to content

Avoid null reference on cache read - #402

Merged
bartelink merged 2 commits into
jet:ddb-orderingfrom
EducationPerfect:ddb-ordering
Jul 3, 2023
Merged

bartelink merged 2 commits into
jet:ddb-orderingfrom
EducationPerfect:ddb-ordering

Conversation

@epNickColeman

Copy link
Copy Markdown
Contributor

Handle nulls when unpacking Equinox.DynamoStore.Core.Token.isStale

@bartelink
bartelink merged commit 6174c8d into jet:ddb-ordering Jul 3, 2023
@bartelink

Copy link
Copy Markdown
Collaborator

Thanks, not sure how I convinced myself that was not the problem!

Comment thread tests/Equinox.CosmosStore.Integration/AccessStrategies.fs
Comment thread src/Equinox.DynamoStore/DynamoStore.fs
@bartelink

Copy link
Copy Markdown
Collaborator

@epNickColeman The expectation of any valid StreamToken is that the .value is non-null

On reflection (OK, on eyeballing the code with sufficient sleep in the cold light of day), the technically correct and/or complete thing to do here is to have all Unpack implementations validate this precondition, and throw an ArgumentNullException if passed an invalid value.

The fault is hence with the caching logic - it's generating invalid StreamToken values and then passing those to isStale. The test stub impl of isStale was not inspecting the token at all (which is something that should be covered...)

Will adjust this in the context of #403

bartelink pushed a commit that referenced this pull request Jul 5, 2023
* Handle nulls when unpacking Equinox.DynamoStore.Core.Token.isStale
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.

2 participants