npm / pacote

npm fetcher
ISC License
351 stars 41 forks source link

[BUG] pacote ignores premature end of HTTP request. #55

Open jdmarshall opened 3 years ago

jdmarshall commented 3 years ago

What / Why

There's a failure mode I've seen both with npm install, and an internal tool that uses pacote directly.

If the requests terminates prematurely, pacote tries to unpack the file anyway instead of aborting.

When

Intermittently, but especially when talking to Artifactory.

Where

npm private repository running in Artifactory client running npm 6.9.0, but also reproduced with other versions

How

Current Behavior

In npm, this shows up as parse errors trying to read package.json files in the npm cache, and finding EOS. This is especially bad because now the npm cache is poisoned. Occasionally it shows up as hash failures, but that seems to happen less often of late.

In our tool, it showed up as premature end of tarball, until I added integrity checking and some logging. The sha is wrong, and you can see that the bytes transferred is a random fraction of the actual payload for the same url on successful runs.

Steps to Reproduce

I'm not sure I have one, but it seems to come in clusters. I suspect that the Artifactory machine is oversubscribed at these times. Or proxy server shenanigans.

Expected Behavior

pacote should throw an error and npm should abort with that error. pacote should call the extract() callback with an error about the http request terminating, rather than trying to extract the file anyway (which is likely the cause of the npm error)

Who

References

travisby commented 3 years ago

We are experiencing this issue with artifactory as well. A pcap hasn't really shown a failed/disconnected request. My best guess is there's a timeout waiting for the next N bytes of data and when pacote doesn't get it, it considers the stream closed and goes to extract.

Interestingly enough this was all-of-a-sudden change for us. We've been using artifactory+npm for years without an issue 🤔 . So far we haven't tied it to a particular version of npm/node/pacote

jdmarshall commented 3 years ago

@travisby the lion's share of this problem started over the summer but I think it was happening intermittently much earlier.

Could be that Artifactory changed, and an old bug in pacote started rearing its head.