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.

Sign up for GitHub

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

Jump to bottom

fix: avoid setting body for GET requests #3643

Merged
merged 4 commits into from Feb 24, 2023
+82 −9
Merged

fix: avoid setting body for GET requests #3643

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
07cdf46
fix: avoid setting body for GET requests
melodyVoid Feb 21, 2023
d3ce8a5
add missing deps for run test locally
juanpicado Feb 22, 2023
2395bd6
test(up-storage): add unit test about uplink is npmmirror
botao-tal Feb 23, 2023
f71f4b7
chore(deps): update @verdaccio/core
botao-tal Feb 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 2 additions & 6 deletions src/lib/up-storage.ts
View file
Open in desktop
Expand Up @@ -215,10 +215,11 @@ class ProxyStorage implements IProxy {
}
}
: undefined;
let requestOptions = {
let requestOptions: request.OptionsWithUrl = {
url: uri,
method: method,
headers: headers,
body: json,
proxy: this.proxy,
encoding: null,
gzip: true,
Expand All @@ -227,11 +228,6 @@ class ProxyStorage implements IProxy {
agentOptions: this.agent_options,
};

// GET requests should not have a body, otherwise the request might fail(return 413 status code)
if (method.toUpperCase() !== 'GET') {
requestOptions.body = json
}

juanpicado marked this conversation as resolved.
Show resolved Hide resolved
if (this.ca) {
requestOptions = Object.assign({}, requestOptions, {
ca: this.ca,
Expand Down
18 changes: 14 additions & 4 deletions test/unit/modules/uplinks/up-storage.spec.ts
View file
Open in desktop
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