Skip to content

Commit

Permalink
fix: avoid setting body for GET requests (#3643)
Browse files Browse the repository at this point in the history
* fix: avoid setting body for GET requests

When making a GET request to certain uplinks, such as https://registry.npmmirror.com, setting the body field can result in a 413 error. Previously, the code was setting the body field for all requests, including GET requests.

This commit fixes the issue by checking the request method and avoiding setting the body field for GET requests. This ensures that GET requests are not affected by the issue and can be made without error.

Fixes #3601

* add missing deps for run test locally

* test(up-storage): add unit test about uplink is npmmirror

Cause thers is a bug in `isObject` function from `@verdaccio/core`, when `options.json` is `true`
GET request body will be string 'true', some uplinks might return 413 status code such as
https://registry.npmmirror.com

fix #3601

* chore(deps): update @verdaccio/core

---------

Co-authored-by: Juan Picado <juanpicado19@gmail.com>
Co-authored-by: botao <botao@tal.com>
  • Loading branch information
3 people committed Feb 24, 2023
1 parent 164d9d2 commit e4573c7
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 9 deletions.
24 changes: 22 additions & 2 deletions .pnp.cjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file not shown.
Binary file not shown.
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -20,7 +20,7 @@
},
"dependencies": {
"@verdaccio/config": "6.0.0-6-next.61",
"@verdaccio/core": "6.0.0-6-next.61",
"@verdaccio/core": "6.0.0-6-next.62",
"@verdaccio/local-storage": "10.3.1",
"@verdaccio/logger-7": "6.0.0-6-next.6",
"@verdaccio/middleware": "6.0.0-6-next.40",
Expand Down
2 changes: 1 addition & 1 deletion src/lib/up-storage.ts
Expand Up @@ -215,7 +215,7 @@ class ProxyStorage implements IProxy {
}
}
: undefined;
let requestOptions = {
let requestOptions: request.OptionsWithUrl = {
url: uri,
method: method,
headers: headers,
Expand Down
18 changes: 14 additions & 4 deletions test/unit/modules/uplinks/up-storage.spec.ts
Expand Up @@ -12,7 +12,7 @@ import configExample from '../../partials/config';

setup({});

describe('UpStorge', () => {
describe('UpStorage', () => {
const mockServerPort = 55547;
let mockRegistry;
const uplinkDefault = {
Expand All @@ -39,7 +39,7 @@ describe('UpStorge', () => {
expect(proxy).toBeDefined();
});

describe('UpStorge::getRemoteMetadata', () => {
describe('UpStorage::getRemoteMetadata', () => {
test('should be get remote metadata', (done) => {
const proxy = generateProxy();

Expand Down Expand Up @@ -72,9 +72,19 @@ describe('UpStorge', () => {
done();
});
});

test('should be get remote metadata with json when uplink is npmmirror', (done) => {
const proxy = generateProxy({ url: 'https://registry.npmmirror.com' });

proxy.getRemoteMetadata('jquery', { json: true }, (err, data) => {
expect(err).toBeNull();
expect(data.name).toBe('jquery');
done();
});
});
});

describe('UpStorge::fetchTarball', () => {
describe('UpStorage::fetchTarball', () => {
test('should fetch a tarball from uplink', (done) => {
const proxy = generateProxy();
const tarball = `http://${DOMAIN_SERVERS}:${mockServerPort}/jquery/-/jquery-1.5.1.tgz`;
Expand Down Expand Up @@ -147,7 +157,7 @@ describe('UpStorge', () => {
}, 10000);
});

describe('UpStorge::isUplinkValid', () => {
describe('UpStorage::isUplinkValid', () => {
describe('valid use cases', () => {
const validateUpLink = (
url: string,
Expand Down
23 changes: 22 additions & 1 deletion yarn.lock
Expand Up @@ -2840,6 +2840,20 @@ __metadata:
languageName: node
linkType: hard

"@verdaccio/core@npm:6.0.0-6-next.62":
version: 6.0.0-6-next.62
resolution: "@verdaccio/core@npm:6.0.0-6-next.62"
dependencies:
ajv: 8.11.2
core-js: 3.28.0
http-errors: 1.8.1
http-status-codes: 2.2.0
process-warning: 1.0.0
semver: 7.3.8
checksum: 628c35522e48b57aa01b9fa4ee869da7d3c8b9ac43905fe783a44d6e9f49b85a2298ee153b947c759cbc237ed9aba350fab2dd6698e74ebff6b9740a949d5718
languageName: node
linkType: hard

"@verdaccio/file-locking@npm:10.3.0":
version: 10.3.0
resolution: "@verdaccio/file-locking@npm:10.3.0"
Expand Down Expand Up @@ -4326,6 +4340,13 @@ __metadata:
languageName: node
linkType: hard

"core-js@npm:3.28.0":
version: 3.28.0
resolution: "core-js@npm:3.28.0"
checksum: 3155fd0ec16d0089106b145e9595280a4ea4bde0d7ff26aa14364cd4f1c203baf6620c3025acd284f363d08b9f21104101692766ca9a36ffeee7307bdf3e1881
languageName: node
linkType: hard

"core-js@npm:^2.6.5":
version: 2.6.12
resolution: "core-js@npm:2.6.12"
Expand Down Expand Up @@ -10588,7 +10609,7 @@ __metadata:
"@typescript-eslint/parser": 5.49.0
"@verdaccio-scope/verdaccio-auth-foo": 0.0.2
"@verdaccio/config": 6.0.0-6-next.61
"@verdaccio/core": 6.0.0-6-next.61
"@verdaccio/core": 6.0.0-6-next.62
"@verdaccio/local-storage": 10.3.1
"@verdaccio/logger-7": 6.0.0-6-next.6
"@verdaccio/middleware": 6.0.0-6-next.40
Expand Down

0 comments on commit e4573c7

Please sign in to comment.