From 3b71aaa5b8bae23ea0597912b5328a5785ab377c Mon Sep 17 00:00:00 2001 From: Vidal Ortega <43078681+vidorteg@users.noreply.github.com> Date: Thu, 4 Aug 2022 10:02:29 -0700 Subject: [PATCH] Breaking : Remove 'request' dependency (#5243) This PR removes 'request' as a (direct) dependencies for the packages/* folder, this is due it being deprecated. node-fetch is used as a substitution. --- packages/connector-jsdom/src/connector.ts | 15 +- .../connector-jsdom/src/evaluate-runner.ts | 1 - packages/connector-puppeteer/src/connector.ts | 3 +- packages/hint-html-checker/package.json | 2 +- packages/hint-html-checker/src/hint.ts | 10 +- packages/hint-html-checker/tests/tests.ts | 4 +- packages/hint-html-checker/tsconfig.json | 1 + packages/hint-https-only/src/hint.ts | 27 +++- packages/hint-no-broken-links/package.json | 1 - packages/hint-no-broken-links/src/hint.ts | 5 +- packages/hint-no-broken-links/tests/tests.ts | 10 +- packages/hint-sri/src/hint.ts | 7 +- packages/utils-connector-tools/package.json | 5 +- packages/utils-connector-tools/src/index.ts | 1 + .../src/requester-options.ts | 6 + .../utils-connector-tools/src/requester.ts | 146 +++++++++++------- .../utils-connector-tools/tests/requester.ts | 50 +++++- packages/utils-network/package.json | 4 +- packages/utils-network/src/request-async.ts | 49 ++++-- .../utils-network/src/request-json-async.ts | 5 +- packages/utils-network/tests/request-async.ts | 33 ++-- .../utils-tests-helpers/src/hint-runner.ts | 6 +- scripts/dist-management/download-dist.js | 24 ++- scripts/update-3rd-party.ts | 22 ++- yarn.lock | 47 +++--- 25 files changed, 313 insertions(+), 171 deletions(-) create mode 100644 packages/utils-connector-tools/src/requester-options.ts diff --git a/packages/connector-jsdom/src/connector.ts b/packages/connector-jsdom/src/connector.ts index 871573e8959..516079fa40c 100644 --- a/packages/connector-jsdom/src/connector.ts +++ b/packages/connector-jsdom/src/connector.ts @@ -54,6 +54,7 @@ const debug: debug.IDebugger = d(__filename); const defaultOptions = { ignoreHTTPSErrors: false, + strictSSL: false, waitFor: 5000 }; @@ -90,10 +91,13 @@ export default class JSDOMConnector implements IConnector { const requesterOptions = { rejectUnauthorized: !this._options.ignoreHTTPSErrors, - strictSSL: !this._options.ignoreHTTPSErrors, ...this._options.requestOptions || {} }; + if (this._options.strictSSL && this._options.ignoreHTTPSErrors) { + this._options.strictSSL = false; + } + this.request = new Requester(requesterOptions); this.server = server; this._timeout = server.timeout; @@ -127,9 +131,12 @@ export default class JSDOMConnector implements IConnector { } const r: Requester = new Requester({ - headers: customHeaders, - rejectUnauthorized: !this._options.ignoreHTTPSErrors, - strictSSL: !this._options.ignoreHTTPSErrors + /* + * TODO: Can be strong typed with IRequesterOptions once package has + * been published. + */ + headers: customHeaders as any, + rejectUnauthorized: !this._options.ignoreHTTPSErrors }); return r.get(uri); diff --git a/packages/connector-jsdom/src/evaluate-runner.ts b/packages/connector-jsdom/src/evaluate-runner.ts index 86c209a05a0..d4a59bd3dd6 100644 --- a/packages/connector-jsdom/src/evaluate-runner.ts +++ b/packages/connector-jsdom/src/evaluate-runner.ts @@ -16,7 +16,6 @@ const run = async (data: { options: any; source: string }) => { const { options = {}, source } = data; const requesterOptions = { rejectUnauthorized: !options.ignoreHTTPSErrors, - strictSSL: !options.ignoreHTTPSErrors, ...options.requestOptions }; diff --git a/packages/connector-puppeteer/src/connector.ts b/packages/connector-puppeteer/src/connector.ts index 924a3b701c4..fdd1ec64a6f 100644 --- a/packages/connector-puppeteer/src/connector.ts +++ b/packages/connector-puppeteer/src/connector.ts @@ -389,8 +389,7 @@ export default class PuppeteerConnector implements IConnector { const options = { headers, // we sync the ignore SSL error options with `request`. This is neeeded for local https tests - rejectUnauthorized: !this._options.ignoreHTTPSErrors, - strictSSL: !this._options.ignoreHTTPSErrors + rejectUnauthorized: !this._options.ignoreHTTPSErrors }; const request = new Requester(options); diff --git a/packages/hint-html-checker/package.json b/packages/hint-html-checker/package.json index 75b8f0aa416..252cbd968e0 100644 --- a/packages/hint-html-checker/package.json +++ b/packages/hint-html-checker/package.json @@ -19,9 +19,9 @@ "description": "hint that that validates HTML pages using the Nu HTML checker", "devDependencies": { "@hint/parser-html": "^3.1.0", + "@hint/utils-connector-tools": "^4.0.35", "@hint/utils-tests-helpers": "^6.5.1", "@types/node": "^17.0.14", - "@types/request": "^2.48.8", "@typescript-eslint/eslint-plugin": "^4.33.0", "@typescript-eslint/parser": "^4.33.0", "ava": "^4.0.1", diff --git a/packages/hint-html-checker/src/hint.ts b/packages/hint-html-checker/src/hint.ts index 4b9ed8082ac..d7ec2b54bb4 100644 --- a/packages/hint-html-checker/src/hint.ts +++ b/packages/hint-html-checker/src/hint.ts @@ -10,11 +10,11 @@ */ import uniqBy = require('lodash/uniqBy'); -import { OptionsWithUrl } from 'request'; import { debug as d } from '@hint/utils-debug'; import { HintContext, IHint } from 'hint'; import { ProblemLocation, Severity } from '@hint/utils-types'; +import type { IRequestOptions } from '@hint/utils-connector-tools'; import { HTMLEvents, HTMLParse } from '@hint/parser-html'; @@ -140,12 +140,12 @@ export default class HtmlCheckerHint implements IHint { { severity: Severity.warning }); }; - const requestRetry = async (options: OptionsWithUrl, retries: number = 3): Promise => { + const requestRetry = async (url: string, options: IRequestOptions, retries: number = 3): Promise => { const requestAsync = (await import('@hint/utils-network')).requestAsync; const delay = (await import('@hint/utils')).delay; try { - return await requestAsync(options); + return await requestAsync(url, options); } catch (e) { if (retries === 0) { throw e; @@ -153,7 +153,7 @@ export default class HtmlCheckerHint implements IHint { await delay(500); - return await requestRetry(options, retries - 1); + return await requestRetry(url, options, retries - 1); } }; @@ -166,7 +166,7 @@ export default class HtmlCheckerHint implements IHint { return { event: data, failed: false, - promise: options.body ? requestRetry(options) : Promise.resolve({ messages: [] }) + promise: options.body ? requestRetry(options.url, options) : Promise.resolve({ messages: [] }) }; }; diff --git a/packages/hint-html-checker/tests/tests.ts b/packages/hint-html-checker/tests/tests.ts index 5428bf8f7a6..89ab46baca4 100644 --- a/packages/hint-html-checker/tests/tests.ts +++ b/packages/hint-html-checker/tests/tests.ts @@ -75,7 +75,7 @@ const configCheckerMessages = { const htmlCheckerMock = (response: any) => { return { '@hint/utils-network': { - requestAsync(scanOptions: any) { + requestAsync(url: string, scanOptions: any) { let responseMessages; if (response.pass) { // No errors/warnings are detected in the target html @@ -83,7 +83,7 @@ const htmlCheckerMock = (response: any) => { } if (response.error) { // Errors/warnings are detected in the target html - const isDefaultChecker = scanOptions.url === defaultValidator; + const isDefaultChecker = url === defaultValidator; responseMessages = isDefaultChecker ? defaultCheckerMessages : configCheckerMessages; diff --git a/packages/hint-html-checker/tsconfig.json b/packages/hint-html-checker/tsconfig.json index cc5cb80a505..14cc2ef5479 100644 --- a/packages/hint-html-checker/tsconfig.json +++ b/packages/hint-html-checker/tsconfig.json @@ -16,6 +16,7 @@ { "path": "../hint" }, { "path": "../parser-html" }, { "path": "../utils" }, + { "path": "../utils-connector-tools" }, { "path": "../utils-debug" }, { "path": "../utils-i18n" }, { "path": "../utils-network" }, diff --git a/packages/hint-https-only/src/hint.ts b/packages/hint-https-only/src/hint.ts index 13c331a7210..fe90e2ac2ce 100644 --- a/packages/hint-https-only/src/hint.ts +++ b/packages/hint-https-only/src/hint.ts @@ -4,7 +4,7 @@ import * as URL from 'url'; -import { ElementFound, FetchEnd, HintContext, IHint, Response } from 'hint'; +import { ElementFound, FetchEnd, FetchError, HintContext, IHint, Response } from 'hint'; import { isDataURI, isHTTPS } from '@hint/utils-network'; import { debug as d } from '@hint/utils-debug'; @@ -105,6 +105,30 @@ export default class HttpsOnlyHint implements IHint { } }; + const validateFetchError = (fetchError: FetchError) => { + const resource = fetchError.resource; + + if (!reportedUrls.has(resource) && !isDataURI(resource) && + fetchError.error && fetchError.error.code && + fetchError.error.code === 'ERR_INVALID_PROTOCOL') { + reportedUrls.add(resource); + + if (fetchError.hops && fetchError.hops.length > 0) { + context.report( + resource, + getMessage('shouldNotBeRedirected', context.language), + { severity: Severity.error } + ); + } else { + context.report( + resource, + getMessage('shouldBeHTTPS', context.language), + { severity: Severity.error } + ); + } + } + }; + /** Returns an array with all the URLs in the given `srcset` attribute or an empty string if none. */ const parseSrcSet = (srcset: string | null): string[] => { if (!srcset) { @@ -193,6 +217,7 @@ export default class HttpsOnlyHint implements IHint { }; context.on('fetch::end::*', validateFetchEnd); + context.on('fetch::error', validateFetchError); context.on('element::img', validateElementSrcs); context.on('element::audio', validateElementSrcs); context.on('element::video', validateElementSrcs); diff --git a/packages/hint-no-broken-links/package.json b/packages/hint-no-broken-links/package.json index a54d0a736d7..99f1be95fce 100644 --- a/packages/hint-no-broken-links/package.json +++ b/packages/hint-no-broken-links/package.json @@ -21,7 +21,6 @@ "@hint/utils-dom": "^2.2.0", "@hint/utils-tests-helpers": "^6.5.1", "@types/node": "^17.0.14", - "@types/request": "^2.48.8", "@typescript-eslint/eslint-plugin": "^4.33.0", "@typescript-eslint/parser": "^4.33.0", "ava": "^4.0.1", diff --git a/packages/hint-no-broken-links/src/hint.ts b/packages/hint-no-broken-links/src/hint.ts index 9277ea99aa0..6b7311789d2 100644 --- a/packages/hint-no-broken-links/src/hint.ts +++ b/packages/hint-no-broken-links/src/hint.ts @@ -14,8 +14,7 @@ import { Severity } from '@hint/utils-types'; import { isRegularProtocol } from '@hint/utils-network'; import { HTMLElement } from '@hint/utils-dom'; import { debug as d } from '@hint/utils-debug'; -import { Requester } from '@hint/utils-connector-tools'; -import { CoreOptions } from 'request'; +import { IRequestOptions, Requester } from '@hint/utils-connector-tools'; import meta from './meta'; import { getMessage } from './i18n.import'; @@ -33,7 +32,7 @@ export default class NoBrokenLinksHint implements IHint { public constructor(context: HintContext) { - const options: CoreOptions = { method: context.hintOptions && context.hintOptions.method ? context.hintOptions.method : 'GET' }; + const options: IRequestOptions = { method: context.hintOptions && context.hintOptions.method ? context.hintOptions.method : 'GET' }; const requester = new Requester(options); const brokenStatusCodes = [404, 410, 500, 503]; diff --git a/packages/hint-no-broken-links/tests/tests.ts b/packages/hint-no-broken-links/tests/tests.ts index 4ee348f5ab2..5471805a53f 100644 --- a/packages/hint-no-broken-links/tests/tests.ts +++ b/packages/hint-no-broken-links/tests/tests.ts @@ -72,15 +72,15 @@ const bodyWithMailTo = `
`; const bodyWithInvalidUrl = `
-About +About
`; const bodyWithBrokenDnsPrefetchLinkTag = `
- +
`; const bodyWithBrokenPreconnectLinkTag = `
- +
`; const bodyWithInvalidDomainDnsPrefetchLinkTag = `
@@ -291,7 +291,7 @@ const tests: HintTest[] = [ name: `This test should fail as it has a loop`, reports: [ { - message: `'http://localhost/1.mp4' could not be fetched using GET method (redirect loop detected).`, + message: `'https://localhost/1.mp4' could not be fetched using GET method (redirect loop detected).`, severity: Severity.error }, { @@ -323,4 +323,4 @@ const tests: HintTest[] = [ } ]; -testHint(hintPath, tests); +testHint(hintPath, tests, {https: true}); diff --git a/packages/hint-sri/src/hint.ts b/packages/hint-sri/src/hint.ts index 2d0e2c6f431..57216ddac2f 100644 --- a/packages/hint-sri/src/hint.ts +++ b/packages/hint-sri/src/hint.ts @@ -415,11 +415,10 @@ export default class SRIHint implements IHint { } try { - response.body.content = await requestAsync({ - gzip: true, + response.body.content = await requestAsync(resource, { + headers: {'content-encoding': 'gzip'}, method: 'GET', - rejectUnauthorized: false, - url: resource + rejectUnauthorized: false }); return true; diff --git a/packages/utils-connector-tools/package.json b/packages/utils-connector-tools/package.json index 0c57ee9d122..74511b29133 100644 --- a/packages/utils-connector-tools/package.json +++ b/packages/utils-connector-tools/package.json @@ -16,12 +16,14 @@ "@hint/utils-types": "^1.2.0", "data-urls": "^3.0.2", "iconv-lite": "^0.6.3", - "request": "^2.88.2" + "https": "^1.0.0", + "node-fetch": "2" }, "description": "hint tools for connectors", "devDependencies": { "@hint/utils-create-server": "^3.4.21", "@types/node": "^17.0.14", + "@types/node-fetch": "^2.6.2", "@typescript-eslint/eslint-plugin": "^4.33.0", "@typescript-eslint/parser": "^4.33.0", "ava": "^4.0.1", @@ -31,6 +33,7 @@ "eslint-plugin-markdown": "^2.2.1", "npm-run-all": "^4.1.5", "nyc": "^15.1.0", + "proxyquire": "^2.1.3", "rimraf": "^3.0.2", "typescript": "^4.5.5" }, diff --git a/packages/utils-connector-tools/src/index.ts b/packages/utils-connector-tools/src/index.ts index 1971ebb4278..c2abd20d12b 100644 --- a/packages/utils-connector-tools/src/index.ts +++ b/packages/utils-connector-tools/src/index.ts @@ -2,3 +2,4 @@ export * from './normalize-headers'; export * from './redirects'; export * from './requester'; export * from './resolver'; +export * from './requester-options'; diff --git a/packages/utils-connector-tools/src/requester-options.ts b/packages/utils-connector-tools/src/requester-options.ts new file mode 100644 index 00000000000..c89bd5fa145 --- /dev/null +++ b/packages/utils-connector-tools/src/requester-options.ts @@ -0,0 +1,6 @@ +import * as fetch from 'node-fetch'; + +export interface IRequestOptions extends fetch.RequestInit { + rejectUnauthorized?: boolean; + strictSSL?: boolean; +} diff --git a/packages/utils-connector-tools/src/requester.ts b/packages/utils-connector-tools/src/requester.ts index abc6619dbee..68fbd25a7b1 100644 --- a/packages/utils-connector-tools/src/requester.ts +++ b/packages/utils-connector-tools/src/requester.ts @@ -1,5 +1,5 @@ /** - * @fileoverview Abstraction over [`request`](https://github.com/request/request) + * @fileoverview Custom `request` implementation * that allow us to handle certain cumbersome scenarios such us: * - Count redirects * - Decode responses that are not `utf-8` @@ -11,7 +11,8 @@ import * as url from 'url'; import { promisify } from 'util'; import * as zlib from 'zlib'; -import * as request from 'request'; +import fetch, {RequestInit} from 'node-fetch'; +import * as https from 'https'; import * as iconv from 'iconv-lite'; import parseDataURL = require('data-urls'); // Using `require` as `data-urls` exports a function. @@ -23,6 +24,7 @@ import { toLowerCaseKeys } from '@hint/utils-string'; import { NetworkData } from 'hint'; import { RedirectManager } from './redirects'; +import { IRequestOptions } from './requester-options'; interface IDecompressor { (content: Buffer): Promise } @@ -51,33 +53,31 @@ const identity = (buff: Buffer): Promise => { return Promise.resolve(Buffer.from(buff)); }; -const defaults = { - encoding: null, - followRedirect: false, +const defaults: IRequestOptions = { + compress: false, headers: { 'Accept-Encoding': 'gzip, deflate, br', 'Accept-Language': 'en-US,en;q=0.8,es;q=0.6,fr;q=0.4', 'Cache-Control': 'no-cache', - DNT: 1, + DNT: '1', Pragma: 'no-cache', 'User-Agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.131 Safari/537.36' }, - jar: true, - time: true, + redirect: 'manual', + rejectUnauthorized: false, + strictSSL: false, timeout: 30000 }; export class Requester { /** The valid status codes for redirects we follow. */ private static validRedirects = [301, 302, 303, 307, 308] - /** Internal `request` object. */ - private _request: request.RequestAPI; /** Internal `redirectManager`. */ private _redirects: RedirectManager = new RedirectManager(); /** Maximum number of redirects */ private _maxRedirects: number = 10; /** Internal options for request */ - private _options: request.CoreOptions; + private _options: IRequestOptions; /** Tries to decompress the given `Buffer` `content` using the given `decompressor` */ private async tryToDecompress(decompressor: IDecompressor, content: Buffer): Promise { @@ -160,11 +160,12 @@ export class Requester { return rawBody; } - public constructor(customOptions?: request.CoreOptions) { + public constructor(customOptions?: IRequestOptions) { if (customOptions) { - customOptions.followRedirect = false; - customOptions.rejectUnauthorized = false; - this._maxRedirects = customOptions.maxRedirects || this._maxRedirects; + customOptions.redirect = 'manual'; + if (customOptions.follow && customOptions.follow >= 0) { + this._maxRedirects = customOptions.follow; + } if (customOptions.headers) { /* @@ -179,14 +180,12 @@ export class Requester { } } - const options: request.CoreOptions = { + const options: RequestInit = { ...defaults, ...customOptions }; this._options = options; - - this._request = request.defaults(options); } /** Return the redirects for a given `uri`. */ @@ -238,58 +237,100 @@ export class Requester { const requestedUrls: Set = new Set(); const getUri = (uriString: string): Promise => { + let rawBodyResponse: Buffer; + requestedUrls.add(uriString); - return new Promise((resolve: Function, reject: Function) => { - const byteChunks: Buffer[] = []; - let rawBodyResponse: Buffer; + return new Promise(async (resolve: Function, reject: Function) => { + try { + let isHTTPS = false; - this._request({ uri: uriString }, async (err, response) => { - if (err) { - debug(`Request for ${uriString} failed\n${err}`); + if (uriString.startsWith('https')) { + isHTTPS = true; + } + + if (this._options.strictSSL || isHTTPS) { + let httpsAgentOptions; + + // This might be set explicitely to false, so we need to validate only if it has a value. + if (this._options.rejectUnauthorized !== undefined) { + httpsAgentOptions = {rejectUnauthorized: this._options.rejectUnauthorized}; + } - return reject({ - error: err, - uri: uriString - }); + const httpsAgent = new https.Agent(httpsAgentOptions); + + this._options.agent = httpsAgent; } + const response = await fetch(uriString, this._options); + + rawBodyResponse = await response.buffer(); + // We check if we need to redirect and call ourselves again with the new target - if (Requester.validRedirects.includes(response.statusCode)) { - if (!response.headers.location) { - return reject({ + if (Requester.validRedirects.includes(response.status)) { + if (!response.headers.get('location')) { + const error = { error: new Error('Redirect location undefined'), uri: uriString - }); + }; + + return reject(error); } - const newUri = url.resolve(uriString, response.headers.location as string); + const newUri = url.resolve(uriString, response.headers.get('location') as string); if (requestedUrls.has(newUri)) { return reject(new Error(`'${uriString}' could not be fetched using ${this._options.method || 'GET'} method (redirect loop detected).`)); } this._redirects.add(newUri, uriString); + console.log(`redirects`, newUri); const currentRedirectNumber = this._redirects.calculate(newUri).length; + console.log(`currentRedirectNumber`, currentRedirectNumber); + if (currentRedirectNumber > this._maxRedirects) { return reject(new Error(`The number of redirects(${currentRedirectNumber}) exceeds the limit(${this._maxRedirects}).`)); } + debug(`Redirect found for ${uriString}`); + try { - debug(`Redirect found for ${uriString}`); const results = await getUri(newUri); return resolve(results); - } catch (e) { + } catch (e){ return reject(e); } } - const contentEncoding: string | null = normalizeHeaderValue(response.headers as HttpHeaders, 'content-encoding'); + const responseHeaders: HttpHeaders = {}; + let requestHeaders: HttpHeaders = {}; + + Array.from(response.headers, ([name, value]) => { + responseHeaders[name] = value; + + return {name, value}; + }); + + if (this._options.headers) { + if (this._options.headers.entries) { + const castedHeaders = (this._options.headers.entries as Function)(); + + for (const [key, value] of castedHeaders) { + if (typeof value === 'string') { + requestHeaders[key] = value; + } + } + } else if (typeof this._options.headers === typeof [[]]) { + requestHeaders = this._options.headers as HttpHeaders; + } + } + + const contentEncoding: string | null = normalizeHeaderValue(responseHeaders, 'content-encoding'); const rawBody: Buffer | null = await this.decompressResponse(contentEncoding, rawBodyResponse); - const contentTypeData = await getContentTypeData(null, uri, response.headers as HttpHeaders, rawBody as Buffer); + const contentTypeData = await getContentTypeData(null, uri, responseHeaders, rawBody as Buffer); const charset = contentTypeData.charset || ''; const mediaType = contentTypeData.mediaType || ''; const hops: string[] = this._redirects.calculate(uriString); @@ -297,7 +338,7 @@ export class Requester { const networkData: NetworkData = { request: { - headers: response.request.headers, + headers: requestHeaders, url: hops[0] || uriString }, response: { @@ -309,32 +350,23 @@ export class Requester { } }, charset, - headers: response.headers as HttpHeaders, + headers: responseHeaders, hops, mediaType, - statusCode: response.statusCode, + statusCode: response.status, url: uriString } }; return resolve(networkData); - }) - /* - * Somehow the Buffer body from `callback(err, resp, body)` is different than the one we get - * if we do this method. Even though both output the same result after decompressing, - * the real bytes sent over the wire for the content are these ones. - * - * See: https://github.com/request/request/tree/6f286c81586a90e6a9d97055f131fdc68e523120#examples. - */ - .on('response', (response) => { - response - .on('data', (data: Buffer) => { - byteChunks.push(data); - }) - .on('end', () => { - rawBodyResponse = Buffer.concat(byteChunks); - }); - }); + } catch (err) { + const error = { + error: err, + uri: uriString + }; + + return reject(error); + } }); }; diff --git a/packages/utils-connector-tools/tests/requester.ts b/packages/utils-connector-tools/tests/requester.ts index f97b2b47a65..eea43c7e085 100644 --- a/packages/utils-connector-tools/tests/requester.ts +++ b/packages/utils-connector-tools/tests/requester.ts @@ -2,6 +2,7 @@ import { promisify } from 'util'; import * as zlib from 'zlib'; import * as iconv from 'iconv-lite'; +import * as proxyquire from 'proxyquire'; import anyTest, { TestFn, ExecutionContext } from 'ava'; import { Server } from '@hint/utils-create-server'; import { NetworkData } from 'hint'; @@ -216,7 +217,7 @@ test(`Requester follows all hops, reports the right number and returns the final }); test(`Throws an error if number of hops exceeds the redirect limit`, async (t) => { - const maxRedirectsRequester = new Requester({ maxRedirects: 4 }); + const maxRedirectsRequester = new Requester({ follow: 4 }); const server = await Server.create({ configuration: hopsServerConfig }); t.plan(1); @@ -241,8 +242,8 @@ test(`Aborts the request if it exceeds the time limit to get response`, async (t try { await timeoutRequester.get(`http://localhost:${server.port}/timeout`) as NetworkData; } catch (e) { - t.is((e as any).error.code, 'ESOCKETTIMEDOUT'); - t.is((e as any).uri, `http://localhost:${server.port}/timeout`); + t.is((e as any).error.type, 'request-timeout'); + t.is((e as any).error.message, `network timeout at: http://localhost:${server.port}/timeout`); } await server.stop(); @@ -256,7 +257,7 @@ test(`Requester returns and exception if a loop is detected`, async (t) => { t.plan(1); try { - await requester.get(`http://localhost:${server.port}/hop301`) as NetworkData; // eslint-disable-line no-unused-expressions + await requester.get(`http://localhost:${server.port}/hop301`) as NetworkData; } catch (e) { t.is((e as Error).message, `'http://localhost:${server.port}/hop301' could not be fetched using GET method (redirect loop detected).`); } @@ -272,10 +273,49 @@ test(`Requester returns and exception if a loop is detected after few redirects` t.plan(1); try { - await requester.get(`http://localhost:${server.port}/hop301`) as NetworkData; // eslint-disable-line no-unused-expressions + await requester.get(`http://localhost:${server.port}/hop301`) as NetworkData; } catch (e) { t.is((e as Error).message, `'http://localhost:${server.port}/hop303' could not be fetched using GET method (redirect loop detected).`); } await server.stop(); }); + +test(`Requester fails if an invalid protocol is used in the config`, async (t) => { + const requester = new Requester({ rejectUnauthorized: false, strictSSL: true }); + const server = await Server.create({ configuration: hopsServerConfig }); + + try { + await requester.get(`http://localhost:${server.port}/hop301`) as NetworkData; + } catch (e) { + t.is((e as any).error.code, `ERR_INVALID_PROTOCOL`); + } + + await server.stop(); +}); + +test(`Requester data request are sucessfully processed`, async (t) => { + const server = await Server.create({ configuration: hopsServerConfig }); + + const script = proxyquire('../src/requester', { + 'data-urls': () => { + return { + body: 'test', + mimeType: { + parameters: { + get: () => { + ''; + } + } + } + }; + } + }); + + const requester = new script.Requester(); + const result = await requester.get(`data://fa.il`) as NetworkData; + + t.is(result.response.body.content, 'test'); + + await server.stop(); +}); diff --git a/packages/utils-network/package.json b/packages/utils-network/package.json index 92e2455ba9d..4e4bb952247 100644 --- a/packages/utils-network/package.json +++ b/packages/utils-network/package.json @@ -16,10 +16,12 @@ "@hint/utils-types": "^1.2.0", "content-type": "^1.0.4", "lodash": "^4.17.21", - "request": "^2.88.2" + "https": "^1.0.0", + "node-fetch": "2" }, "description": "utils for network", "devDependencies": { + "@types/node-fetch": "2.x", "ava": "^4.0.1", "eslint": "^7.32.0", "eslint-plugin-import": "^2.26.0", diff --git a/packages/utils-network/src/request-async.ts b/packages/utils-network/src/request-async.ts index a46477c1fbd..1f47bc2cfa4 100644 --- a/packages/utils-network/src/request-async.ts +++ b/packages/utils-network/src/request-async.ts @@ -1,15 +1,40 @@ -import * as request from 'request'; +import fetch, { RequestInit } from 'node-fetch'; +import * as https from 'https'; + +/** + * Mostly used for legacy support on a breaking change and should be considered + * deprecated. Need to be in sync with @hint/utils-connector-tools, but it cannot be + * used because it creates a circular dependency. + * + * Required as RequestInit options do not support SSL. + */ +interface IRequestOptions extends RequestInit { + rejectUnauthorized?: boolean; + strictSSL?: boolean; +} /** Convenience wrapper for asynchronously request an URL. */ -export const requestAsync = (options: string | request.Options): Promise => { - return new Promise((resolve, reject) => { - // `as any` because typescript complains about the type of options. - request(options as any, (error: any, res: any, body: any) => { - if (error) { - return reject(error); - } - - return resolve(body); - }); - }); +export const requestAsync = async (url: string, options: IRequestOptions = {}): Promise => { + let isHTTPS = false; + + if (url.startsWith('https')) { + isHTTPS = true; + } + + if (options.strictSSL || isHTTPS) { + let httpsAgentOptions; + + // This might be set explicitely to false, so we need to validate only if it has a value. + if (options.rejectUnauthorized !== undefined) { + httpsAgentOptions = {rejectUnauthorized: options.rejectUnauthorized}; + } + + const httpsAgent = new https.Agent(httpsAgentOptions); + + options.agent = httpsAgent; + } + + const response = await fetch(url, options); + + return await response.text(); }; diff --git a/packages/utils-network/src/request-json-async.ts b/packages/utils-network/src/request-json-async.ts index 0c26cbab209..614f254da09 100644 --- a/packages/utils-network/src/request-json-async.ts +++ b/packages/utils-network/src/request-json-async.ts @@ -3,10 +3,9 @@ import { requestAsync } from './request-async'; /** Request response in the json format from an endpoint. */ export const requestJSONAsync = (uri: string, options: object): Promise => { const params = { - json: true, - uri, + headers: { 'Content-Type': 'application/json' }, ...options }; - return requestAsync(params); + return requestAsync(uri, params); }; diff --git a/packages/utils-network/tests/request-async.ts b/packages/utils-network/tests/request-async.ts index 74cd3cfbf5d..05d387174bf 100644 --- a/packages/utils-network/tests/request-async.ts +++ b/packages/utils-network/tests/request-async.ts @@ -2,31 +2,38 @@ import test from 'ava'; import * as sinon from 'sinon'; import * as proxyquire from 'proxyquire'; -test('requestAsync should fails if the request fails', async (t) => { - const requestStub = sinon.stub(); - const { requestAsync } = proxyquire('../src/request-async', { request: requestStub }); +test.only('requestAsync should fails if the request fails', async (t) => { - requestStub.callsFake((options, callback) => { - callback('error'); + const fetchStub = sinon.stub(); + const { requestAsync } = proxyquire('../src/request-async', {'node-fetch': { default: fetchStub }}); + const expectedError = new Error('expected error'); + + fetchStub.callsFake((options) => { + throw expectedError; }); t.plan(1); try { - await requestAsync(); + await requestAsync('https://example.com'); } catch (err) { - t.is(err, 'error'); + t.is(err, expectedError); } }); test('requestAsync should works if the request works', async (t) => { - const requestStub = sinon.stub(); - const { requestAsync } = proxyquire('../src/request-async', { request: requestStub }); + const fetchStub = sinon.stub(); + const { requestAsync } = proxyquire('../src/request-async', {'node-fetch': { default: fetchStub }}); - requestStub.callsFake((options, callback) => { - callback(null, null, 'result'); + fetchStub.callsFake((options) => { + return { body: 'result' }; }); - const result = await requestAsync({ url: 'https://example.com' }); + t.plan(1); + try { + const result = await requestAsync('https://example.com'); - t.is(result, 'result'); + t.is(result, 'result'); + } catch (err) { + t.fail(`Not expecting an exception: ${err}`); + } }); diff --git a/packages/utils-tests-helpers/src/hint-runner.ts b/packages/utils-tests-helpers/src/hint-runner.ts index b6b93f120e1..6eb2872d1ef 100644 --- a/packages/utils-tests-helpers/src/hint-runner.ts +++ b/packages/utils-tests-helpers/src/hint-runner.ts @@ -143,10 +143,9 @@ const requestSource = async (url: string, connector: string): Promise => * Allow us to use our self-signed cert for testing. * https://github.com/request/request/issues/418#issuecomment-23058601 */ - return await requestAsync({ + return await requestAsync(url, { rejectUnauthorized: false, - strictSSL: false, - url + strictSSL: false }); } catch (e) { // Some tests deliberately use invalid URLs (e.g. `test:`). @@ -545,6 +544,7 @@ export const testHint = (hintId: string, hintTests: HintTest[], configs: { [key: const target = serverUrl ? serverUrl : `${configs.https ? 'https' : 'http'}://localhost:${server.port}/`; engine = await createConnector(t, hintTest, connector); + const results = await engine.executeOn(new URL(target)); const sources = new Map(); diff --git a/scripts/dist-management/download-dist.js b/scripts/dist-management/download-dist.js index bb1be242d24..b1abe8891c8 100644 --- a/scripts/dist-management/download-dist.js +++ b/scripts/dist-management/download-dist.js @@ -1,7 +1,7 @@ const path = require('path'); const fs = require('fs'); -const request = require('request'); +const fetch = require('node-fetch'); const unzipper = require('unzipper'); const { exec } = require('../utils/exec'); @@ -51,24 +51,22 @@ const repoClean = async () => { const download = (fileName) => { const file = fs.createWriteStream(fileName); - return new Promise((resolve, reject) => { - - request(`https://github.com/webhintio/hint/releases/download/dist/${fileName}`, (err, res) => { - if (err) { - reject(err); - - return; - } + return new Promise(async (resolve, reject) => { + try { + const res = await fetch(`https://github.com/webhintio/hint/releases/download/dist/${fileName}`); + res.pipe(file); + resolve(); if (res.statusCode !== 200) { reject(new Error(`Artifacts for ${fileName} aren't available`)); return; } - }) - .pipe(file) - .on('finish', resolve) - .on('error', reject); + } catch (err) { + reject(err); + + return; + } }); }; diff --git a/scripts/update-3rd-party.ts b/scripts/update-3rd-party.ts index af1fd6dc5e9..212d53fe2b0 100644 --- a/scripts/update-3rd-party.ts +++ b/scripts/update-3rd-party.ts @@ -1,8 +1,6 @@ import * as path from 'path'; -import { promisify } from 'util'; import * as fs from 'fs'; - -import * as req from 'request'; +import fetch from 'node-fetch'; import { compile } from 'json-schema-to-typescript'; @@ -11,8 +9,6 @@ type Transform = { replacement: string; } | ((content: string, location: string) => Promise | string); -const request = promisify(req) as (options: req.OptionsWithUrl) => Promise; - const cleanSchemaObject = (obj: any) => { if (!obj || typeof obj !== 'object') { return; @@ -38,13 +34,13 @@ const cleanSchemaObject = (obj: any) => { const inlineRemoteRefs = async (json: any): Promise => { for (const entry of json.allOf) { if (entry.$ref && entry.$ref.startsWith('https://')) { - const res = await request({ url: entry.$ref }) as req.Response; + const res = await fetch(entry.$ref); - if (res.body.message) { - throw new Error(res.body.message); + if (res.body && (res.body as any).message) { + throw new Error((res.body as any).message); } - const refJson = JSON.parse(res.body); + const refJson = await res.json(); json.properties = { ...json.properties, ...refJson.properties }; json.definitions = { ...json.definitions, ...refJson.definitions }; @@ -90,13 +86,13 @@ const replaceId = (content: string, location: string): string => { }; const downloadFile = async (downloadURL: string, downloadLocation: string, transforms: Transform[] = []) => { - const res = await request({ url: downloadURL }) as req.Response; + const res = await fetch(downloadURL); - if (res.body.message) { - throw new Error(res.body.message); + if (res.body && (res.body as any).message) { + throw new Error((res.body as any).message); } - let body = res.body as string; + let body = await res.text(); for (const transform of transforms) { if (typeof transform === 'function') { diff --git a/yarn.lock b/yarn.lock index 1a0e18c2a77..becd19108c8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -606,11 +606,6 @@ "@types/node" "*" "@types/responselike" "*" -"@types/caseless@*": - version "0.12.2" - resolved "https://registry.yarnpkg.com/@types/caseless/-/caseless-0.12.2.tgz#f65d3d6389e01eeb458bd54dc8f52b95a9463bc8" - integrity sha512-6ckxMjBBD8URvjB6J3NcnuAn5Pkl7t3TizAg+xdlzzQGSPSmBcXf8KoIH0ua/i+tio+ZRUHEXp0HEmvaR4kt0w== - "@types/chokidar@^2.1.3": version "2.1.3" resolved "https://registry.yarnpkg.com/@types/chokidar/-/chokidar-2.1.3.tgz#123ab795dba6d89be04bf076e6aecaf8620db674" @@ -869,6 +864,14 @@ resolved "https://registry.yarnpkg.com/@types/ms/-/ms-0.7.31.tgz#31b7ca6407128a3d2bbc27fe2d21b345397f6197" integrity sha512-iiUgKzV9AuaEkZqkOLDIvlQiL6ltuZd9tGcW3gwpnX8JbuiuhFlEGmmFXEXkN50Cvq7Os88IY2v0dkDqXYWVgA== +"@types/node-fetch@2.x", "@types/node-fetch@^2.6.2": + version "2.6.2" + resolved "https://registry.yarnpkg.com/@types/node-fetch/-/node-fetch-2.6.2.tgz#d1a9c5fd049d9415dce61571557104dec3ec81da" + integrity sha512-DHqhlq5jeESLy19TYhLakJ07kNumXWjcDdxXsLUMJZ6ue8VZJj4kLPQVE/2mdHh3xZziNF1xppu5lwmS53HR+A== + dependencies: + "@types/node" "*" + form-data "^3.0.0" + "@types/node@*", "@types/node@>= 8", "@types/node@^17.0.14": version "17.0.14" resolved "https://registry.yarnpkg.com/@types/node/-/node-17.0.14.tgz#33b9b94f789a8fedd30a68efdbca4dbb06b61f20" @@ -966,16 +969,6 @@ "@types/node" "*" safe-buffer "*" -"@types/request@^2.48.8": - version "2.48.8" - resolved "https://registry.yarnpkg.com/@types/request/-/request-2.48.8.tgz#0b90fde3b655ab50976cb8c5ac00faca22f5a82c" - integrity sha512-whjk1EDJPcAR2kYHRbFl/lKeeKYTi05A15K9bnLInCVroNDCtXce57xKdI0/rQaA3K+6q0eFyUBPmqfSndUZdQ== - dependencies: - "@types/caseless" "*" - "@types/node" "*" - "@types/tough-cookie" "*" - form-data "^2.5.0" - "@types/responselike@*", "@types/responselike@^1.0.0": version "1.0.0" resolved "https://registry.yarnpkg.com/@types/responselike/-/responselike-1.0.0.tgz#251f4fe7d154d2bad125abe1b429b23afd262e29" @@ -4582,13 +4575,13 @@ fork-ts-checker-webpack-plugin@^7.2.9: semver "^7.3.5" tapable "^2.2.1" -form-data@^2.5.0: - version "2.5.0" - resolved "https://registry.yarnpkg.com/form-data/-/form-data-2.5.0.tgz#094ec359dc4b55e7d62e0db4acd76e89fe874d37" - integrity sha512-WXieX3G/8side6VIqx44ablyULoGruSde5PNTxoUyo5CeyAMX6nVWUd0rgist/EuX655cjhUhTo1Fo3tRYqbcA== +form-data@^3.0.0: + version "3.0.1" + resolved "https://registry.yarnpkg.com/form-data/-/form-data-3.0.1.tgz#ebd53791b78356a99af9a300d4282c4d5eb9755f" + integrity sha512-RHkBKtLWUVwd7SqRIvCZMEvAMoGUp0XU+seQiZejj0COz3RI3hWP4sCv3gZWWLjJTd7rGwcsF5eKZGii0r/hbg== dependencies: asynckit "^0.4.0" - combined-stream "^1.0.6" + combined-stream "^1.0.8" mime-types "^2.1.12" form-data@^4.0.0: @@ -5245,6 +5238,11 @@ https-proxy-agent@5, https-proxy-agent@5.0.0, https-proxy-agent@^5.0.0: agent-base "6" debug "4" +https@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/https/-/https-1.0.0.tgz#3c37c7ae1a8eeb966904a2ad1e975a194b7ed3a4" + integrity sha512-4EC57ddXrkaF0x83Oj8sM6SLQHAWXw90Skqu2M4AEWENZ3F02dFJE/GARA8igO79tcgYqGrD7ae4f5L3um2lgg== + human-signals@^1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/human-signals/-/human-signals-1.1.1.tgz#c5b1cd14f50aeae09ab6c59fe63ba3395fe4dfa3" @@ -7251,6 +7249,13 @@ node-addon-api@^4.3.0: resolved "https://registry.yarnpkg.com/node-addon-api/-/node-addon-api-4.3.0.tgz#52a1a0b475193e0928e98e0426a0d1254782b77f" integrity sha512-73sE9+3UaLYYFmDsFZnqCInzPyh3MqIwZO9cw58yIqAZhONrrabrYyYe3TuIqtIiOuTXVhsGau8hcrhhwSsDIQ== +node-fetch@2: + version "2.6.7" + resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.7.tgz#24de9fba827e3b4ae44dc8b20256a379160052ad" + integrity sha512-ZjMPFEfVx5j+y2yF35Kzx5sF7kDzxuDj6ziH4FFbOp87zKDZNx8yExJIb05OGF4Nlt9IHFIMBkRl41VdvcNdbQ== + dependencies: + whatwg-url "^5.0.0" + node-fetch@2.6.1: version "2.6.1" resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052" @@ -8711,7 +8716,7 @@ release-zalgo@^1.0.0: dependencies: es6-error "^4.0.1" -request@2.88.2, request@^2.88.2: +request@2.88.2: version "2.88.2" resolved "https://registry.yarnpkg.com/request/-/request-2.88.2.tgz#d73c918731cb5a87da047e207234146f664d12b3" integrity sha512-MsvtOrfG9ZcrOwAW+Qi+F6HbD0CWXEh9ou77uOb7FM2WPhwT7smM833PzanhJLsgXjN89Ir6V2PczXNnMpwKhw==