From b209f5c63a8e78b9629cc861163962cd08c8cf88 Mon Sep 17 00:00:00 2001 From: chris48s Date: Fri, 8 Oct 2021 17:27:50 +0100 Subject: [PATCH] got --> node-fetch (#6914) --- core/base-service/base.js | 4 +- core/base-service/got.js | 96 ---------- core/base-service/got.spec.js | 102 ---------- core/base-service/node-fetch.js | 101 ++++++++++ core/base-service/node-fetch.spec.js | 180 ++++++++++++++++++ package-lock.json | 1 + package.json | 1 + services/dynamic/dynamic-json.tester.js | 5 +- .../mozilla-observatory.service.js | 2 +- services/opm/opm-version.service.js | 5 +- .../security-headers.service.js | 2 +- services/test-helpers.js | 7 +- 12 files changed, 297 insertions(+), 209 deletions(-) delete mode 100644 core/base-service/got.js delete mode 100644 core/base-service/got.spec.js create mode 100644 core/base-service/node-fetch.js create mode 100644 core/base-service/node-fetch.spec.js diff --git a/core/base-service/base.js b/core/base-service/base.js index a547814ea4..566d128678 100644 --- a/core/base-service/base.js +++ b/core/base-service/base.js @@ -20,7 +20,7 @@ import { Deprecated, } from './errors.js' import { validateExample, transformExample } from './examples.js' -import { fetchFactory } from './got.js' +import { sendRequest } from './node-fetch.js' import { makeFullUrl, assertValidRoute, @@ -432,7 +432,7 @@ class BaseService { ServiceClass: this, }) - const fetcher = fetchFactory(fetchLimitBytes) + const fetcher = sendRequest.bind(sendRequest, fetchLimitBytes) camp.route( regex, diff --git a/core/base-service/got.js b/core/base-service/got.js deleted file mode 100644 index 0120e216cd..0000000000 --- a/core/base-service/got.js +++ /dev/null @@ -1,96 +0,0 @@ -import got from 'got' -import { Inaccessible, InvalidResponse } from './errors.js' - -const userAgent = 'Shields.io/2003a' - -function requestOptions2GotOptions(options) { - const requestOptions = Object.assign({}, options) - const gotOptions = {} - const interchangableOptions = ['body', 'form', 'headers', 'method', 'url'] - - interchangableOptions.forEach(function (opt) { - if (opt in requestOptions) { - gotOptions[opt] = requestOptions[opt] - delete requestOptions[opt] - } - }) - - if ('qs' in requestOptions) { - gotOptions.searchParams = requestOptions.qs - delete requestOptions.qs - } - - if ('gzip' in requestOptions) { - gotOptions.decompress = requestOptions.gzip - delete requestOptions.gzip - } - - if ('strictSSL' in requestOptions) { - gotOptions.https = { - rejectUnauthorized: requestOptions.strictSSL, - } - delete requestOptions.strictSSL - } - - if ('auth' in requestOptions) { - gotOptions.username = requestOptions.auth.user - gotOptions.password = requestOptions.auth.pass - delete requestOptions.auth - } - - if (Object.keys(requestOptions).length > 0) { - throw new Error(`Found unrecognised options ${Object.keys(requestOptions)}`) - } - - return gotOptions -} - -async function sendRequest(gotWrapper, url, options) { - const gotOptions = requestOptions2GotOptions(options) - gotOptions.throwHttpErrors = false - gotOptions.retry = 0 - gotOptions.headers = gotOptions.headers || {} - gotOptions.headers['User-Agent'] = userAgent - try { - const resp = await gotWrapper(url, gotOptions) - return { res: resp, buffer: resp.body } - } catch (err) { - if (err instanceof got.CancelError) { - throw new InvalidResponse({ - underlyingError: new Error('Maximum response size exceeded'), - }) - } - throw new Inaccessible({ underlyingError: err }) - } -} - -function fetchFactory(fetchLimitBytes) { - const gotWithLimit = got.extend({ - handlers: [ - (options, next) => { - const promiseOrStream = next(options) - promiseOrStream.on('downloadProgress', progress => { - if ( - progress.transferred > fetchLimitBytes && - // just accept the file if we've already finished downloading - // the entire file before we went over the limit - progress.percent !== 1 - ) { - /* - TODO: we should be able to pass cancel() a message - https://github.com/sindresorhus/got/blob/main/documentation/advanced-creation.md#examples - but by the time we catch it, err.message is just "Promise was canceled" - */ - promiseOrStream.cancel('Maximum response size exceeded') - } - }) - - return promiseOrStream - }, - ], - }) - - return sendRequest.bind(sendRequest, gotWithLimit) -} - -export { requestOptions2GotOptions, fetchFactory } diff --git a/core/base-service/got.spec.js b/core/base-service/got.spec.js deleted file mode 100644 index 185052d163..0000000000 --- a/core/base-service/got.spec.js +++ /dev/null @@ -1,102 +0,0 @@ -import { expect } from 'chai' -import nock from 'nock' -import { requestOptions2GotOptions, fetchFactory } from './got.js' -import { Inaccessible, InvalidResponse } from './errors.js' - -describe('requestOptions2GotOptions function', function () { - it('translates valid options', function () { - expect( - requestOptions2GotOptions({ - body: 'body', - form: 'form', - headers: 'headers', - method: 'method', - url: 'url', - qs: 'qs', - gzip: 'gzip', - strictSSL: 'strictSSL', - auth: { user: 'user', pass: 'pass' }, - }) - ).to.deep.equal({ - body: 'body', - form: 'form', - headers: 'headers', - method: 'method', - url: 'url', - searchParams: 'qs', - decompress: 'gzip', - https: { rejectUnauthorized: 'strictSSL' }, - username: 'user', - password: 'pass', - }) - }) - - it('throws if unrecognised options are found', function () { - expect(() => - requestOptions2GotOptions({ body: 'body', foobar: 'foobar' }) - ).to.throw(Error, 'Found unrecognised options foobar') - }) -}) - -describe('got wrapper', function () { - it('should not throw an error if the response <= fetchLimitBytes', async function () { - nock('https://www.google.com') - .get('/foo/bar') - .once() - .reply(200, 'x'.repeat(100)) - const sendRequest = fetchFactory(100) - const { res } = await sendRequest('https://www.google.com/foo/bar') - expect(res.statusCode).to.equal(200) - }) - - it('should throw an InvalidResponse error if the response is > fetchLimitBytes', async function () { - nock('https://www.google.com') - .get('/foo/bar') - .once() - .reply(200, 'x'.repeat(101)) - const sendRequest = fetchFactory(100) - return expect( - sendRequest('https://www.google.com/foo/bar') - ).to.be.rejectedWith(InvalidResponse, 'Maximum response size exceeded') - }) - - it('should throw an Inaccessible error if the request throws a (non-HTTP) error', async function () { - nock('https://www.google.com').get('/foo/bar').replyWithError('oh no') - const sendRequest = fetchFactory(1024) - return expect( - sendRequest('https://www.google.com/foo/bar') - ).to.be.rejectedWith(Inaccessible, 'oh no') - }) - - it('should throw an Inaccessible error if the host can not be accessed', async function () { - this.timeout(5000) - nock.disableNetConnect() - const sendRequest = fetchFactory(1024) - return expect( - sendRequest('https://www.google.com/foo/bar') - ).to.be.rejectedWith( - Inaccessible, - 'Nock: Disallowed net connect for "www.google.com:443/foo/bar"' - ) - }) - - it('should pass a custom user agent header', async function () { - nock('https://www.google.com', { - reqheaders: { - 'user-agent': function (agent) { - return agent.startsWith('Shields.io') - }, - }, - }) - .get('/foo/bar') - .once() - .reply(200) - const sendRequest = fetchFactory(1024) - await sendRequest('https://www.google.com/foo/bar') - }) - - afterEach(function () { - nock.cleanAll() - nock.enableNetConnect() - }) -}) diff --git a/core/base-service/node-fetch.js b/core/base-service/node-fetch.js new file mode 100644 index 0000000000..de954b492a --- /dev/null +++ b/core/base-service/node-fetch.js @@ -0,0 +1,101 @@ +import { URL, URLSearchParams } from 'url' +import fetch from 'node-fetch' +import { Inaccessible, InvalidResponse } from './errors.js' + +const userAgent = 'Shields.io/2003a' + +function object2URLSearchParams(obj) { + const qs = {} + for (const [key, value] of Object.entries(obj)) { + if (value === undefined) { + continue + } else if (value === null) { + qs[key] = '' + } else if (['string', 'number', 'boolean'].includes(typeof value)) { + qs[key] = value + } + } + return new URLSearchParams(qs) +} + +function request2NodeFetch({ url, options }) { + const requestOptions = Object.assign({}, options) + const nodeFetchOptions = {} + const nodeFetchUrl = new URL(url) + const interchangableOptions = ['headers', 'method', 'body'] + + if ('body' in requestOptions && 'form' in requestOptions) { + throw new Error("Options 'form' and 'body' can not both be used") + } + + interchangableOptions.forEach(function (opt) { + if (opt in requestOptions) { + nodeFetchOptions[opt] = requestOptions[opt] + delete requestOptions[opt] + } + }) + nodeFetchOptions.headers = nodeFetchOptions.headers || {} + + if ('qs' in requestOptions) { + if (typeof requestOptions.qs === 'string') { + nodeFetchUrl.search = requestOptions.qs + delete requestOptions.qs + } else if (typeof requestOptions.qs === 'object') { + nodeFetchUrl.search = object2URLSearchParams(requestOptions.qs) + delete requestOptions.qs + } else if (requestOptions.qs == null) { + delete requestOptions.qs + } else { + throw new Error("Property 'qs' must be string, object or null") + } + } + + if ('gzip' in requestOptions) { + nodeFetchOptions.compress = requestOptions.gzip + delete requestOptions.gzip + } + + if ('auth' in requestOptions) { + const user = requestOptions.auth.user || '' + const pass = requestOptions.auth.pass || '' + const b64authStr = Buffer.from(`${user}:${pass}`).toString('base64') + nodeFetchOptions.headers.Authorization = `Basic ${b64authStr}` + delete requestOptions.auth + } + + if ('form' in requestOptions) { + nodeFetchOptions.body = object2URLSearchParams(requestOptions.form) + delete requestOptions.form + } + + if (Object.keys(requestOptions).length > 0) { + throw new Error(`Found unrecognised options ${Object.keys(requestOptions)}`) + } + + return { url: nodeFetchUrl.toString(), options: nodeFetchOptions } +} + +async function sendRequest(fetchLimitBytes, url, options) { + const { url: nodeFetchUrl, options: nodeFetchOptions } = request2NodeFetch({ + url, + options, + }) + nodeFetchOptions.headers['User-Agent'] = userAgent + nodeFetchOptions.size = fetchLimitBytes + nodeFetchOptions.follow = 10 + try { + const resp = await fetch(nodeFetchUrl, nodeFetchOptions) + const body = await resp.text() + resp.statusCode = resp.status + return { res: resp, buffer: body } + } catch (err) { + if (err.type === 'max-size') { + throw new InvalidResponse({ + underlyingError: new Error('Maximum response size exceeded'), + }) + } + throw new Inaccessible({ underlyingError: err }) + } +} + +export { request2NodeFetch, sendRequest } diff --git a/core/base-service/node-fetch.spec.js b/core/base-service/node-fetch.spec.js new file mode 100644 index 0000000000..0ee6f6745a --- /dev/null +++ b/core/base-service/node-fetch.spec.js @@ -0,0 +1,180 @@ +import { URLSearchParams } from 'url' +import { expect } from 'chai' +import nock from 'nock' +import { request2NodeFetch, sendRequest } from './node-fetch.js' +import { Inaccessible, InvalidResponse } from './errors.js' + +describe('request2NodeFetch function', function () { + it('translates simple options', function () { + expect( + request2NodeFetch({ + url: 'https://google.com/', + options: { + body: 'body', + headers: 'headers', + method: 'method', + gzip: 'gzip', + }, + }) + ).to.deep.equal({ + url: 'https://google.com/', + options: { + body: 'body', + headers: 'headers', + method: 'method', + compress: 'gzip', + }, + }) + }) + + it('translates auth to header', function () { + expect( + request2NodeFetch({ + url: 'https://google.com/', + options: { auth: { user: 'user', pass: 'pass' } }, + }) + ).to.deep.equal({ + url: 'https://google.com/', + options: { + headers: { + Authorization: 'Basic dXNlcjpwYXNz', + }, + }, + }) + expect( + request2NodeFetch({ + url: 'https://google.com/', + options: { auth: { user: 'user' } }, + }) + ).to.deep.equal({ + url: 'https://google.com/', + options: { + headers: { + Authorization: 'Basic dXNlcjo=', + }, + }, + }) + expect( + request2NodeFetch({ + url: 'https://google.com/', + options: { auth: { pass: 'pass' } }, + }) + ).to.deep.equal({ + url: 'https://google.com/', + options: { + headers: { + Authorization: 'Basic OnBhc3M=', + }, + }, + }) + }) + + it('translates form to body', function () { + expect( + request2NodeFetch({ + url: 'https://google.com/', + options: { + form: { foo: 'bar', baz: 1 }, + }, + }) + ).to.deep.equal({ + url: 'https://google.com/', + options: { + body: new URLSearchParams({ foo: 'bar', baz: 1 }), + headers: {}, + }, + }) + }) + + it('appends qs to URL', function () { + expect( + request2NodeFetch({ + url: 'https://google.com/', + options: { + qs: { foo: 'bar', baz: 1 }, + }, + }) + ).to.deep.equal({ + url: 'https://google.com/?foo=bar&baz=1', + options: { + headers: {}, + }, + }) + }) + + it('throws if unrecognised options are found', function () { + expect(() => + request2NodeFetch({ + url: 'https://google.com/', + options: { body: 'body', foobar: 'foobar' }, + }) + ).to.throw(Error, 'Found unrecognised options foobar') + }) + + it('throws if body and form are both specified', function () { + expect(() => + request2NodeFetch({ + url: 'https://google.com/', + options: { body: 'body', form: 'form' }, + }) + ).to.throw(Error, "Options 'form' and 'body' can not both be used") + }) +}) + +describe('sendRequest', function () { + it('should not throw an error if the response <= fetchLimitBytes', async function () { + nock('https://www.google.com') + .get('/foo/bar') + .once() + .reply(200, 'x'.repeat(100)) + const { res } = await sendRequest(100, 'https://www.google.com/foo/bar') + expect(res.statusCode).to.equal(200) + }) + + it('should throw an InvalidResponse error if the response is > fetchLimitBytes', async function () { + nock('https://www.google.com') + .get('/foo/bar') + .once() + .reply(200, 'x'.repeat(101)) + return expect( + sendRequest(100, 'https://www.google.com/foo/bar') + ).to.be.rejectedWith(InvalidResponse, 'Maximum response size exceeded') + }) + + it('should throw an Inaccessible error if the request throws a (non-HTTP) error', async function () { + nock('https://www.google.com').get('/foo/bar').replyWithError('oh no') + return expect( + sendRequest(1024, 'https://www.google.com/foo/bar') + ).to.be.rejectedWith(Inaccessible, 'oh no') + }) + + it('should throw an Inaccessible error if the host can not be accessed', async function () { + this.timeout(5000) + nock.disableNetConnect() + return expect( + sendRequest(1024, 'https://www.google.com/foo/bar') + ).to.be.rejectedWith( + Inaccessible, + 'Nock: Disallowed net connect for "www.google.com:443/foo/bar"' + ) + }) + + it('should pass a custom user agent header', async function () { + nock('https://www.google.com', { + reqheaders: { + 'user-agent': function (agent) { + return agent.startsWith('Shields.io') + }, + }, + }) + .get('/foo/bar') + .once() + .reply(200) + await sendRequest(1024, 'https://www.google.com/foo/bar') + }) + + afterEach(function () { + nock.cleanAll() + nock.enableNetConnect() + }) +}) diff --git a/package-lock.json b/package-lock.json index f8dcc16816..b29de5c5d0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -40,6 +40,7 @@ "lodash.times": "^4.3.2", "moment": "^2.29.1", "node-env-flag": "^0.1.0", + "node-fetch": "^2.6.1", "parse-link-header": "^1.0.1", "path-to-regexp": "^6.2.0", "pretty-bytes": "^5.6.0", diff --git a/package.json b/package.json index d093b0cc59..d3916d5ed9 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "lodash.times": "^4.3.2", "moment": "^2.29.1", "node-env-flag": "^0.1.0", + "node-fetch": "^2.6.1", "parse-link-header": "^1.0.1", "path-to-regexp": "^6.2.0", "pretty-bytes": "^5.6.0", diff --git a/services/dynamic/dynamic-json.tester.js b/services/dynamic/dynamic-json.tester.js index 7d34bdc1db..40c91376e7 100644 --- a/services/dynamic/dynamic-json.tester.js +++ b/services/dynamic/dynamic-json.tester.js @@ -27,7 +27,7 @@ t.create('Malformed url') ) .expectBadge({ label: 'Package Name', - message: 'inaccessible', + message: 'invalid', color: 'lightgrey', }) @@ -136,7 +136,8 @@ t.create('request should set Accept header') ) .expectBadge({ label: 'custom badge', message: 'test' }) .after(() => { - expect(headers).to.have.property('accept', 'application/json') + expect(headers).to.have.property('accept') + expect(headers.accept).to.deep.equal(['application/json']) }) t.create('query with lexical error') diff --git a/services/mozilla-observatory/mozilla-observatory.service.js b/services/mozilla-observatory/mozilla-observatory.service.js index 8f436056c2..0b8ad060c9 100644 --- a/services/mozilla-observatory/mozilla-observatory.service.js +++ b/services/mozilla-observatory/mozilla-observatory.service.js @@ -103,7 +103,7 @@ export default class MozillaObservatory extends BaseJsonService { options: { method: 'POST', qs: { host }, - form: { hidden: !publish }, + form: { hidden: (!publish).toString() }, }, }) } diff --git a/services/opm/opm-version.service.js b/services/opm/opm-version.service.js index 6d9f41c438..fa1a42ad77 100644 --- a/services/opm/opm-version.service.js +++ b/services/opm/opm-version.service.js @@ -40,11 +40,10 @@ export default class OpmVersion extends BaseService { }) // TODO: set followRedirect to false and intercept 302 redirects - const location = res.request.redirects[0] - if (!location) { + if (!res.redirected) { throw new NotFound({ prettyMessage: 'module not found' }) } - const version = location.match(`${moduleName}-(.+).opm`)[1] + const version = res.url.match(`${moduleName}-(.+).opm`)[1] if (!version) { throw new InvalidResponse({ prettyMessage: 'version invalid' }) } diff --git a/services/security-headers/security-headers.service.js b/services/security-headers/security-headers.service.js index 0b935a871e..45f8a2b751 100644 --- a/services/security-headers/security-headers.service.js +++ b/services/security-headers/security-headers.service.js @@ -83,7 +83,7 @@ export default class SecurityHeaders extends BaseService { }, }) - const grade = res.headers['x-grade'] + const grade = res.headers.get('x-grade') if (!grade) { throw new NotFound({ prettyMessage: 'not available' }) diff --git a/services/test-helpers.js b/services/test-helpers.js index 876467ebed..f66a38e764 100644 --- a/services/test-helpers.js +++ b/services/test-helpers.js @@ -1,7 +1,7 @@ import bytes from 'bytes' import nock from 'nock' import config from 'config' -import { fetchFactory } from '../core/base-service/got.js' +import { sendRequest } from '../core/base-service/node-fetch.js' const runnerConfig = config.util.toObject() function cleanUpNockAfterEach() { @@ -31,7 +31,10 @@ function noToken(serviceClass) { } } -const sendAndCacheRequest = fetchFactory(bytes(runnerConfig.public.fetchLimit)) +const sendAndCacheRequest = sendRequest.bind( + sendRequest, + bytes(runnerConfig.public.fetchLimit) +) const defaultContext = { sendAndCacheRequest }