diff --git a/config/config.default.js b/config/config.default.js index 53a913a20a..534fba9891 100644 --- a/config/config.default.js +++ b/config/config.default.js @@ -303,8 +303,8 @@ module.exports = appInfo => { * @property {Number} httpsAgent.freeSocketTimeout - httpss agent socket keepalive max free time, default is 4000 ms. * @property {Number} httpsAgent.maxSockets - https agent max socket number of one host, default is `Number.MAX_SAFE_INTEGER` @ses https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER * @property {Number} httpsAgent.maxFreeSockets - https agent max free socket number of one host, default is 256. - * @property {Boolean} useHttpClientNext - use urllib@3 HttpClient - * @property {Boolean} allowH2 - Allow to use HTTP2 first, only work on `useHttpClientNext = true` + * @property {Boolean} useHttpClientNext - use urllib@3 HttpClient, default is false + * @property {Boolean} allowH2 - use urllib@4 HttpClient and enable H2, default is false. Only works on Node.js >= 18 */ config.httpclient = { enableDNSCache: false, diff --git a/lib/core/httpclient_next.js b/lib/core/httpclient_next.js index 7d0fc4afea..0d0d7e48d8 100644 --- a/lib/core/httpclient_next.js +++ b/lib/core/httpclient_next.js @@ -1,7 +1,16 @@ -const { HttpClient } = require('urllib-next'); const ms = require('humanize-ms'); + const SSRF_HTTPCLIENT = Symbol('SSRF_HTTPCLIENT'); +const mainNodejsVersion = parseInt(process.versions.node.split('.')[0]); +let HttpClient; +if (mainNodejsVersion >= 18) { + // urllib@4 only works on Node.js >= 18 + HttpClient = require('urllib4').HttpClient; +} else { + HttpClient = require('urllib-next').HttpClient; +} + class HttpClientNext extends HttpClient { constructor(app, options) { normalizeConfig(app); diff --git a/lib/egg.js b/lib/egg.js index a70c69fae8..8c1b4e9064 100644 --- a/lib/egg.js +++ b/lib/egg.js @@ -292,7 +292,7 @@ class EggApplication extends EggCore { */ createHttpClient(options) { let httpClient; - if (this.config.httpclient.useHttpClientNext) { + if (this.config.httpclient.useHttpClientNext || this.config.httpclient.allowH2) { httpClient = new this.HttpClientNext(this, options); } else if (this.config.httpclient.enableDNSCache) { httpClient = new DNSCacheHttpClient(this, options); diff --git a/package.json b/package.json index 734fbdb851..48bbb375f1 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,8 @@ "onelogger": "^1.0.0", "sendmessage": "^2.0.0", "urllib": "^2.33.0", - "urllib-next": "npm:urllib@^3.26.0", + "urllib-next": "npm:urllib@^3.27.1", + "urllib4": "npm:urllib@^4.3.0", "utility": "^2.1.0", "ylru": "^1.3.2" }, @@ -77,7 +78,6 @@ "egg-view-nunjucks": "^2.3.0", "eslint": "^8.23.1", "eslint-config-egg": "^12.0.0", - "findlinks": "^2.2.0", "formstream": "^1.1.1", "jsdoc": "^3.6.11", "koa": "^2.13.4", @@ -85,11 +85,9 @@ "node-libs-browser": "^2.2.1", "pedding": "^1.1.0", "prettier": "^2.7.1", - "puppeteer": "^19.11.1", "react": "^16.14.0", "react-dom": "^16.14.0", "react-router": "^5.3.4", - "runscript": "^1.5.3", "sdk-base": "^4.2.1", "spy": "^1.0.0", "supertest": "^6.2.4", diff --git a/test/doc.test.js b/test/doc.test.js deleted file mode 100644 index de53ea206b..0000000000 --- a/test/doc.test.js +++ /dev/null @@ -1,41 +0,0 @@ -'use strict'; - -const path = require('path'); -const findlinks = require('findlinks'); -const assert = require('assert'); -const runscript = require('runscript'); -const utils = require('./utils'); -const puppeteer = require('puppeteer'); - -describe('test/doc.test.js', () => { - /** - * This unit test is ONLY working for the latest nodejs version (v18.0.0) - * or higher version, bcoz on windows, it takes quite a lot of time - * to generate the whole doc. We only need some of the test cases to check - * whether the links inside the doc gets fine or not. - */ - it('should have no broken urls (based on non-windows platform and node\'s version >=18)', async function() { - - const mainNodejsVersion = parseInt(process.versions.node.split('.')[0]); - - if (process.platform === 'linux' && mainNodejsVersion >= 18) { - const cwd = path.dirname(__dirname); - const dumi = path.join(cwd, 'node_modules', '.bin', 'dumi'); - await runscript(`cross-env NODE_OPTIONS=--openssl-legacy-provider APP_ROOT=./site ${dumi} build`, - { - cwd, - }); - const app = utils.cluster({ - baseDir: 'apps/docapp', - }); - app.coverage(false); - await app.ready(); - const result = await findlinks({ src: app.url, logger: console, puppeteer }); - if (result.fail !== 0) console.log(result); - assert(result.fail === 0); - app.close(); - } else { - this.skip(); - } - }).timeout(10 * 60 * 1000); -}); diff --git a/test/fixtures/apps/httpclient-allowH2/app.js b/test/fixtures/apps/httpclient-allowH2/app.js new file mode 100644 index 0000000000..b3d7873ba9 --- /dev/null +++ b/test/fixtures/apps/httpclient-allowH2/app.js @@ -0,0 +1,15 @@ +const assert = require('assert'); + +module.exports = app => { + class CustomHttpClient extends app.HttpClientNext { + async request(url, opt) { + assert(/^http/.test(url), 'url should start with http, but got ' + url); + return await super.request(url, opt); + } + + async curl(url, opt) { + return await this.request(url, opt); + } + } + app.HttpClientNext = CustomHttpClient; +}; diff --git a/test/fixtures/apps/httpclient-http2/config/config.default.js b/test/fixtures/apps/httpclient-allowH2/config/config.default.js similarity index 55% rename from test/fixtures/apps/httpclient-http2/config/config.default.js rename to test/fixtures/apps/httpclient-allowH2/config/config.default.js index 77834b89ad..e477cdb9db 100644 --- a/test/fixtures/apps/httpclient-http2/config/config.default.js +++ b/test/fixtures/apps/httpclient-allowH2/config/config.default.js @@ -1,4 +1,6 @@ exports.httpclient = { - useHttpClientNext: true, allowH2: true, + request: { + timeout: 99, + }, }; diff --git a/test/fixtures/apps/httpclient-allowH2/package.json b/test/fixtures/apps/httpclient-allowH2/package.json new file mode 100644 index 0000000000..d788b7620b --- /dev/null +++ b/test/fixtures/apps/httpclient-allowH2/package.json @@ -0,0 +1,3 @@ +{ + "name": "httpclient-allowH2" +} diff --git a/test/fixtures/apps/httpclient-http2/app.js b/test/fixtures/apps/httpclient-http2/app.js deleted file mode 100644 index 29c5a62b35..0000000000 --- a/test/fixtures/apps/httpclient-http2/app.js +++ /dev/null @@ -1,21 +0,0 @@ -'use strict'; - -const assert = require('assert'); - -module.exports = app => { - class CustomHttpClient extends app.HttpClientNext { - request(url, opt) { - return new Promise(resolve => { - assert(/^http/.test(url), 'url should start with http, but got ' + url); - resolve(); - }).then(() => { - return super.request(url, opt); - }); - } - - curl(url, opt) { - return this.request(url, opt); - } - } - app.HttpClientNext = CustomHttpClient; -}; diff --git a/test/fixtures/apps/httpclient-http2/package.json b/test/fixtures/apps/httpclient-http2/package.json deleted file mode 100644 index 56b5d28da8..0000000000 --- a/test/fixtures/apps/httpclient-http2/package.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "name": "httpclient-overwrite" -} diff --git a/test/fixtures/apps/httpclient-tracer/app.js b/test/fixtures/apps/httpclient-tracer/app.js index 0b6d6e866c..80fd82b24c 100644 --- a/test/fixtures/apps/httpclient-tracer/app.js +++ b/test/fixtures/apps/httpclient-tracer/app.js @@ -28,14 +28,14 @@ module.exports = app => { }); assert(res.status === 200); - res = await httpclient.request('https://github.com', { + res = await httpclient.request('https://registry.npmmirror.com', { method: 'GET', timeout: 20000, }); assert(res.status === 200); - res = await httpclient.request('https://www.npmjs.com', { + res = await httpclient.request('https://npmmirror.com', { method: 'GET', timeout: 20000, }); diff --git a/test/lib/core/httpclient.test.js b/test/lib/core/httpclient.test.js index e0b26b0da3..09f3b6f258 100644 --- a/test/lib/core/httpclient.test.js +++ b/test/lib/core/httpclient.test.js @@ -1,5 +1,4 @@ const assert = require('node:assert'); -const { sensitiveHeaders } = require('node:http2'); const mm = require('egg-mock'); const urllib = require('urllib'); const Httpclient = require('../../../lib/core/httpclient'); @@ -305,30 +304,64 @@ describe('test/lib/core/httpclient.test.js', () => { describe('overwrite httpclient support allowH2=true', () => { let app; before(() => { - app = utils.app('apps/httpclient-http2'); + app = utils.app('apps/httpclient-allowH2'); return app.ready(); }); after(() => app.close()); it('should work on http2', async () => { - const res = await app.httpclient.request(url); + const res = await app.httpclient.request(url, { + timeout: 5000, + }); assert.equal(res.status, 200); assert.equal(res.data.toString(), 'GET /'); - assert.equal(sensitiveHeaders in res.headers, false); + // assert.equal(sensitiveHeaders in res.headers, false); const res2 = await app.httpclient.request('https://registry.npmmirror.com/urllib/latest', { dataType: 'json', + timeout: 5000, }); assert.equal(res2.status, 200); assert.equal(res2.data.name, 'urllib'); - assert.equal(sensitiveHeaders in res2.headers, true); + // assert.equal(sensitiveHeaders in res2.headers, true); }); - it('should assert url', () => { - return app.httpclient.curl('unknown url') - .catch(err => { - assert(err); - assert(err.message.includes('url should start with http, but got unknown url')); + it('should set request default global timeout to 99ms', async () => { + await assert.rejects(async () => { + await app.httpclient.curl(`${url}/timeout`); + }, err => { + assert.equal(err.name, 'HttpClientRequestTimeoutError'); + assert.match(err.message, /timeout for 99 ms/); + return true; + }); + }); + + it('should request http1.1 success', async () => { + const result = await app.httpclient.curl(`${url}`, { + dataType: 'text', + }); + assert.equal(result.status, 200); + assert.equal(result.data, 'GET /'); + }); + + it('should request http2 success', async () => { + for (let i = 0; i < 10; i++) { + const result = await app.httpclient.curl('https://registry.npmmirror.com', { + dataType: 'json', + timeout: 5000, }); + assert.equal(result.status, 200); + assert.equal(result.headers['content-type'], 'application/json; charset=utf-8'); + assert.equal(result.data.sync_model, 'all'); + } + }); + + it('should assert url', async () => { + await assert.rejects(async () => { + await app.httpclient.curl('unknown url'); + }, err => { + assert.match(err.message, /url should start with http, but got unknown url/); + return true; + }); }); }); @@ -618,13 +651,13 @@ describe('test/lib/core/httpclient.test.js', () => { }); assert(res.status === 200); - res = await httpclient.request('https://github.com', { + res = await httpclient.request('https://registry.npmmirror.com', { method: 'GET', timeout: 20000, }); assert(res.status === 200); - res = await httpclient.request('https://www.npmjs.com', { + res = await httpclient.request('https://npmmirror.com', { method: 'GET', timeout: 20000, });