From eb1b156146bc338da9c6ded5a2c5beab22ac0ed8 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Wed, 19 Feb 2025 20:45:20 +0100 Subject: [PATCH] feat(appium): Add a command line parameter to configure HTTP server request timeout (#21003) --- packages/appium/docs/en/cli/args.md | 1 + packages/appium/lib/main.js | 6 ++-- packages/appium/test/fixtures/default-args.js | 1 + packages/base-driver/lib/express/server.js | 30 +++++++++++++++---- packages/schema/lib/appium-config-schema.js | 14 ++++++++- packages/schema/lib/appium-config.schema.json | 7 +++++ packages/types/lib/appium-config.ts | 5 ++++ 7 files changed, 55 insertions(+), 9 deletions(-) diff --git a/packages/appium/docs/en/cli/args.md b/packages/appium/docs/en/cli/args.md index 374802979..8ee5776bc 100644 --- a/packages/appium/docs/en/cli/args.md +++ b/packages/appium/docs/en/cli/args.md @@ -34,6 +34,7 @@ below. |`--driver`|Driver-specific configuration. Keys should correspond to driver package names|object||| |`--drivers-import-chunk-size`|The maximum amount of drivers that could be imported in parallel on server startup|number|`3`|| |`--keep-alive-timeout`|Number of seconds the Appium server should apply as both the keep-alive timeout and the connection timeout for all requests. Setting this to `0` disables the timeout.|integer|`600`|`-ka`| +|`--request-timeout`|Number of seconds the Appium server should apply for receiving the entire HTTP request from the client. A value of 0 disables the timeout. Set it to a non-zero value to protect against potential Denial-of-Service attacks in case the server is deployed without a reverse proxy in front. HTTP requests that are running longer than allowed by this timeout would be rejected with the status code 408.|integer|`3600`|| |`--local-timezone`|Use local timezone for timestamps|boolean|`false`|| |`--log`|Also send log output to this file|string||`-g`| |`--log-filters`|One or more log filtering rules|array||| diff --git a/packages/appium/lib/main.js b/packages/appium/lib/main.js index 36f7afe40..2726690f9 100755 --- a/packages/appium/lib/main.js +++ b/packages/appium/lib/main.js @@ -397,8 +397,10 @@ async function main(args) { extraMethodMap, cliArgs: parsedArgs, }; - if (parsedArgs.keepAliveTimeout) { - serverOpts.keepAliveTimeout = parsedArgs.keepAliveTimeout * 1000; + for (const timeoutArgName of ['keepAliveTimeout', 'requestTimeout']) { + if (_.isInteger(parsedArgs[timeoutArgName])) { + serverOpts[timeoutArgName] = parsedArgs[timeoutArgName] * 1000; + } } let server; const bidiServer = new WebSocketServer({noServer: true}); diff --git a/packages/appium/test/fixtures/default-args.js b/packages/appium/test/fixtures/default-args.js index 3fc3bda4c..f30801c8b 100644 --- a/packages/appium/test/fixtures/default-args.js +++ b/packages/appium/test/fixtures/default-args.js @@ -8,6 +8,7 @@ export default { denyInsecure: [], driversImportChunkSize: 3, keepAliveTimeout: 600, + requestTimeout: 3600, localTimezone: false, logFormat: 'text', loglevel: 'debug', diff --git a/packages/base-driver/lib/express/server.js b/packages/base-driver/lib/express/server.js index b741d1460..7f92588d5 100644 --- a/packages/base-driver/lib/express/server.js +++ b/packages/base-driver/lib/express/server.js @@ -76,7 +76,7 @@ async function createServer (app, cliArgs) { * @param {ServerOpts} opts * @returns {Promise} */ -async function server(opts) { +export async function server(opts) { const { routeConfiguringFunction, port, @@ -87,6 +87,7 @@ async function server(opts) { extraMethodMap = {}, serverUpdaters = [], keepAliveTimeout = KEEP_ALIVE_TIMEOUT_MS, + requestTimeout, } = opts; const app = express(); @@ -123,7 +124,13 @@ async function server(opts) { // want to block extensions' ability to add routes if they want. app.all('*', catch404Handler); - await startServer({httpServer, hostname, port, keepAliveTimeout}); + await startServer({ + httpServer, + hostname, + port, + keepAliveTimeout, + requestTimeout, + }); resolve(appiumServer); } catch (err) { @@ -136,7 +143,7 @@ async function server(opts) { * Sets up some Express middleware and stuff * @param {ConfigureServerOpts} opts */ -function configureServer({ +export function configureServer({ app, addRoutes, allowCors = true, @@ -267,13 +274,22 @@ function configureHttp({httpServer, reject, keepAliveTimeout, gracefulShutdownTi * @param {StartServerOpts} opts * @returns {Promise} */ -async function startServer({httpServer, port, hostname, keepAliveTimeout}) { +async function startServer({ + httpServer, + port, + hostname, + keepAliveTimeout, + requestTimeout, +}) { // If the hostname is omitted, the server will accept // connections on any IP address /** @type {(port: number, hostname?: string) => B} */ const start = B.promisify(httpServer.listen, {context: httpServer}); const startPromise = start(port, hostname); httpServer.keepAliveTimeout = keepAliveTimeout; + if (_.isInteger(requestTimeout)) { + httpServer.requestTimeout = Number(requestTimeout); + } // headers timeout must be greater than keepAliveTimeout httpServer.headersTimeout = keepAliveTimeout + 5 * 1000; await startPromise; @@ -284,7 +300,7 @@ async function startServer({httpServer, port, hostname, keepAliveTimeout}) { * @param {string} basePath * @returns {string} */ -function normalizeBasePath(basePath) { +export function normalizeBasePath(basePath) { if (!_.isString(basePath)) { throw new Error(`Invalid path prefix ${basePath}`); } @@ -302,7 +318,6 @@ function normalizeBasePath(basePath) { return basePath; } -export {server, configureServer, normalizeBasePath}; /** * Options for {@linkcode startServer}. @@ -311,6 +326,8 @@ export {server, configureServer, normalizeBasePath}; * @property {number} port - Port to run on * @property {number} keepAliveTimeout - Keep-alive timeout in milliseconds * @property {string} [hostname] - Optional hostname + * @property {number} [requestTimeout] - The timeout value in milliseconds for + * receiving the entire request from the client */ /** @@ -344,6 +361,7 @@ export {server, configureServer, normalizeBasePath}; * @property {MethodMap} [extraMethodMap] * @property {import('@appium/types').UpdateServerCallback[]} [serverUpdaters] * @property {number} [keepAliveTimeout] + * @property {number} [requestTimeout] */ /** diff --git a/packages/schema/lib/appium-config-schema.js b/packages/schema/lib/appium-config-schema.js index 148ff5a6c..328b1446b 100644 --- a/packages/schema/lib/appium-config-schema.js +++ b/packages/schema/lib/appium-config-schema.js @@ -117,11 +117,23 @@ export const AppiumConfigJsonSchema = /** @type {const} */ ({ appiumCliAliases: ['ka'], default: 600, description: - 'Number of seconds the Appium server should apply as both the keep-alive timeout and the connection timeout for all requests. A value of 0 disables the timeout.', + 'Number of seconds the Appium server should apply as both the keep-alive timeout and the connection timeout ' + + 'for all requests. A value of 0 disables the timeout.', minimum: 0, title: 'keep-alive-timeout config', type: 'integer', }, + 'request-timeout': { + default: 3600, + description: + 'Number of seconds the Appium server should apply for receiving the entire HTTP request from the client. ' + + 'A value of 0 disables the timeout. Set it to a non-zero value to protect against ' + + 'potential Denial-of-Service attacks in case the server is deployed without a reverse proxy in front. ' + + 'HTTP requests that are running longer than allowed by this timeout would be rejected with the status code 408.', + minimum: 0, + title: 'request-timeout config', + type: 'integer', + }, 'local-timezone': { default: false, description: 'Use local timezone for timestamps', diff --git a/packages/schema/lib/appium-config.schema.json b/packages/schema/lib/appium-config.schema.json index ad36a7418..a1cdf24f2 100644 --- a/packages/schema/lib/appium-config.schema.json +++ b/packages/schema/lib/appium-config.schema.json @@ -120,6 +120,13 @@ "title": "keep-alive-timeout config", "type": "integer" }, + "request-timeout": { + "default": 3600, + "description": "Number of seconds the Appium server should apply for receiving the entire HTTP request from the client. A value of 0 disables the timeout. Set it to a non-zero value to protect against potential Denial-of-Service attacks in case the server is deployed without a reverse proxy in front. HTTP requests that are running longer than allowed by this timeout would be rejected with the status code 408.", + "minimum": 0, + "title": "request-timeout config", + "type": "integer" + }, "local-timezone": { "default": false, "description": "Use local timezone for timestamps", diff --git a/packages/types/lib/appium-config.ts b/packages/types/lib/appium-config.ts index 74f654b62..e1518b72a 100644 --- a/packages/types/lib/appium-config.ts +++ b/packages/types/lib/appium-config.ts @@ -41,6 +41,10 @@ export type DenyInsecureConfig = string[]; * Number of seconds the Appium server should apply as both the keep-alive timeout and the connection timeout for all requests. A value of 0 disables the timeout. */ export type KeepAliveTimeoutConfig = number; +/** + * Number of seconds the Appium server should apply for receiving the entire HTTP request from the client. A value of 0 disables the timeout. Set it to a non-zero value to protect against potential Denial-of-Service attacks in case the server is deployed without a reverse proxy in front. HTTP requests that are running longer than allowed by this timeout would be rejected with the status code 408. + */ +export type RequestTimeoutConfig = number; /** * Use local timezone for timestamps */ @@ -193,6 +197,7 @@ export interface ServerConfig { "deny-insecure"?: DenyInsecureConfig; driver?: DriverConfig; "keep-alive-timeout"?: KeepAliveTimeoutConfig; + "request-timeout"?: RequestTimeoutConfig; "local-timezone"?: LocalTimezoneConfig; log?: LogConfig; "log-filters"?: LogFiltersConfig;