-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
[BUG] tarball data currupted when installing from 'chunked encoding' server. #3884
Comments
I think that my proxy implementation is correct because files downloaded with curl have the correct checksum and extract cleanly.
Our actual private registry is written in java, so I don't think it's a bug in common. |
I've traced thru pacote, minipass-fetch, and make-fetch-happen and the stream's integrity is correct thru here https://github.com/everett1992/make-fetch-happen/blob/chunked-encoding/lib/remote.js#L53 But the stream here is wrong https://github.com/everett1992/make-fetch-happen/blob/chunked-encoding/lib/cache/entry.js#L288 |
I can reliably cause tar integrity errors while downloading packages from npm thru an http proxy that uses chunked encoding. The error seems to be in make-fetch-happen, minipass-fetch or one of the minipass libraries. I don't understand the root cause but I'm unable to reproduce the issue after applying this patch. My guess is that something in the stack expects res.body to be a Minipass instance but not MinipassPipeline, or creating the pipeline starts body flowing before all listeners are connected, missing some chunks of data. fixes npm/cli#3884 To reproduce 1. Start the [chunked-encoding proxy](https://github.com/everett1992/make-fetch-happen/blob/chunked-encoding/proxy.js) 2. In a new terminal create a npm project `mkdir chunked-encoding-error && cd chunked-encoding-error && echo '{}' > package.json` 3. configure npm to use the registry `npm config --location project set registry http://localhost:4000 4. remove node_modules, and npm cache 5. install any package from the registry (this should fail). - I recommend a package without dependencies, like lodash or typescript, debugging is easier. - The error isn't 100% consistent, you may want to run this step in a loop ``` while rm ~/.npm/_cacache node_modules package-lock.json -rf && npm install --no-save lodash; do; done ``` [chunked-encoding proxy]: https://github.com/everett1992/make-fetch-happen/blob/chunked-encoding/proxy.js
For some context, Caleb (and I) are build engineers at Amazon. This is blocking us from moving our company onto NPM 7 because of the way we handle internal repository mirrors. |
Related to npm/cli#3884, npm/make-fetch-happen#63 npm v7+ throws integrity and zlib corrupted errors when installing from a registry that uses chunked encoding. This package reproduces that error using make-fetch-happen and tar. The error only occurs when fetch is passed integrity and the server uses chunked encoding. Only the `extract with integirty` test fails.
Reproduces isaacs#27, cause of npm/cli#3884
Reproduces isaacs#27, cause of npm/cli#3884
4c5a106 handled a convoluted case where there is a chunk in the buffer AND we're in a flowing state during a write call which caused out of order writes. The fix was to flush the buffer before emitting the new chunk, but it didn't account for destinations pausing the stream after flushing part of the buffer. This caused issues in npm/pacote/npm-registry-fetch. That specific issue is demonstrated in everett1992/make-fetch-happen-tar-extract-error and occurs when make-fetch-happen res.body is piped to a tar.extract stream. Fixes isaacs#27 npm/cli#3884 make-fetch-happen#63
4c5a106 handled a convoluted case where there is a chunk in the buffer AND we're in a flowing state during a write call which caused out of order writes. The fix was to flush the buffer before emitting the new chunk, but it didn't account for destinations pausing the stream after flushing part of the buffer. This caused issues in npm/pacote/npm-registry-fetch. That specific issue is demonstrated in everett1992/make-fetch-happen-tar-extract-error and occurs when make-fetch-happen res.body is piped to a tar.extract stream. Fixes #27 npm/cli#3884 make-fetch-happen#63 PR-URL: #28 Credit: @everett1992 Close: #28 Reviewed-by: @isaacs
npm is now on the version of minipass that had this fixed. isaacs/minipass#28. Closing as fixed, please reopen if this is still an issue. |
Our private Registry service had same problem after the migration of |
Is there an existing issue for this?
Current Behavior
While using npm v7/8 to install packages from our private npm registry we'll regularly see tar extract and integrity errors
I can reproduce these errors using a chunked-encoding proxy in front of registry.npmjs.org. I can resolve the issue by changing make-fetch-happen's integrity check from a MinipassPipeline to a basic stream. I don't know if that really fixes the issue or just changes the timing enough to hide it.
everett1992/make-fetch-happen@4c26ad6#diff-e11a8e3d6413e0d326dca64c22417f2670168171c83d2db6da41fb7eae868aa0
Expected Behavior
npm should be able to install with any valid http server.
Steps To Reproduce
In this environment...
I've reproduced this on Ubuntu with node 16, npm v7 and npm v8. I can't reproduce with v6 but one user has reported the issue with npm v6.
With this config...
proxy.js
Environment
The text was updated successfully, but these errors were encountered: