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: Do not blow up if the opts object is mutated #4

Closed
wants to merge 5 commits into from

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Oct 24, 2019

Pacote has a use case where the integrity value may not be known at the
outset, but is later established, either via the dist.integrity in a
packument, or by the x-local-hash header value when make-fetch-happen
loads a response from the cache.

In these cases, we have already started an integrity stream at the
beginning of the request, and don't get the expected integrity until
after the integrity stream is created, resulting in a spurious
EINTEGRITY error.

This patch makes ssri responsive to (and resilient against) updates to
the integrity and size options after the stream has started.

Pacote has a use case where the integrity value may not be known at the
outset, but is later established, either via the dist.integrity in a
packument, or by the x-local-hash header value when make-fetch-happen
loads a response from the cache.

In these cases, we have already started an integrity stream at the
beginning of the request, and don't get the expected integrity until
_after_ the integrity stream is created, resulting in a spurious
EINTEGRITY error.

This patch makes ssri responsive to (and resilient against) updates to
the integrity and size options after the stream has started.
@isaacs isaacs requested a review from a team as a code owner October 24, 2019 20:31
This is going to be used in Pacote to upgrade the Fetcher.integrity
value with the data that comes out of cacache, but in such a way that we
don't accidentally suppress integrity errors.
test/mutable-opts-resilience.js Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Show resolved Hide resolved
@mikemimik mikemimik self-requested a review October 24, 2019 23:48
Copy link

@mikemimik mikemimik left a comment

Choose a reason for hiding this comment

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

LGTM

@isaacs isaacs closed this in 0572c1d Oct 24, 2019
@wraithgar wraithgar deleted the isaacs/mutable-options branch June 10, 2021 16:37
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.

None yet

2 participants