Skip to content

Commit b680ac8

Browse files
authored
[openApiDiff.ts] Replace exec with execFile (#370)
- fixes #254
1 parent 9a13012 commit b680ac8

File tree

5 files changed

+31
-184
lines changed

5 files changed

+31
-184
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## 0.12.0 2025-08-29
44

55
- replaced dependency 'request' with built-in fetch() API
6+
- replaced child_process.exec() with execFile()
67
- require Node >= 20
78

89
## 0.11.0 2025-08-11

package-lock.json

Lines changed: 0 additions & 21 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
"lodash": "^4.17.21",
2626
"minimist": "^1.2.8",
2727
"set-value": "^4.1.0",
28-
"shell-quote": "^1.8.3",
2928
"source-map": "^0.7.4",
3029
"tslib": "^2.6.3",
3130
"winston": "^3.13.0",
@@ -39,7 +38,6 @@
3938
"@types/js-yaml": "^3.12.1",
4039
"@types/json-pointer": "^1.0.30",
4140
"@types/node": "^20.0.0",
42-
"@types/shell-quote": "^1.7.5",
4341
"@types/yargs": "^13.0.0",
4442
"eslint": "^8.57.0",
4543
"jest": "^29.7.0",

src/lib/validators/openApiDiff.ts

Lines changed: 30 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,21 @@
22
// Licensed under the MIT License. See License.txt in the project root for license information.
33

44
import * as asyncFs from "@ts-common/fs"
5-
import * as jsonParser from "@ts-common/json-parser"
65
import { getFilePosition } from "@ts-common/source-map"
76
import * as child_process from "child_process"
87
import * as fs from "fs"
98
import JSON_Pointer from "json-pointer"
109
import * as jsonRefs from "json-refs"
1110
import * as os from "os"
1211
import * as path from "path"
13-
import { quote } from "shell-quote"
1412
import * as sourceMap from "source-map"
1513
import * as util from "util"
1614
import { log } from "../util/logging"
1715
import { ResolveSwagger } from "../util/resolveSwagger"
1816
import { pathToJsonPointer } from "../util/utils"
1917
const _ = require("lodash")
2018

21-
const exec = util.promisify(child_process.exec)
19+
const execFile = util.promisify(child_process.execFile)
2220

2321
export type Options = {
2422
readonly consoleLogLevel?: unknown
@@ -83,28 +81,6 @@ const updateChangeProperties = (change: ChangeProperties, pf: ProcessedFile): Ch
8381
}
8482
}
8583

86-
/**
87-
* Safely escapes shell arguments for cross-platform compatibility
88-
* @param arg The argument to escape
89-
* @returns The safely escaped argument
90-
*/
91-
function escapeShellArg(arg: string): string {
92-
if (typeof arg !== "string") {
93-
throw new Error("Argument must be a string")
94-
}
95-
96-
if (process.platform === "win32") {
97-
// For Windows cmd.exe, wrap in double quotes and escape internal quotes
98-
// This handles paths with spaces and special characters safely
99-
// Double quotes are escaped by doubling them in Windows
100-
return `"${arg.replace(/"/g, '""')}"`
101-
} else {
102-
// On Unix-like systems, use shell-quote for proper escaping
103-
// shell-quote handles all edge cases including spaces, special chars, etc.
104-
return quote([arg])
105-
}
106-
}
107-
10884
/**
10985
* @class
11086
* Open API Diff class.
@@ -165,19 +141,19 @@ export class OpenApiDiff {
165141
}
166142

167143
/**
168-
* Gets path to the autorest application.
144+
* Gets file and args to the autorest application.
169145
*
170-
* @returns {string} Path to the autorest app.js file.
146+
* @returns {{ file: string; args: string[] }} File and args to the autorest app.js file.
171147
*/
172-
public autoRestPath(): string {
173-
log.silly(`autoRestPath is being called`)
148+
public autoRestFileArgs(): { file: string; args: string[] } {
149+
log.silly(`autoRestFileArgs is being called`)
174150

175151
// When oad is installed globally
176152
{
177153
const result = path.join(__dirname, "..", "..", "..", "node_modules", "autorest", "dist", "app.js")
178154
if (fs.existsSync(result)) {
179155
log.silly(`Found autoRest:${result} `)
180-
return `node ${escapeShellArg(result)}`
156+
return { file: "node", args: [result] }
181157
}
182158
}
183159

@@ -186,7 +162,7 @@ export class OpenApiDiff {
186162
const result = path.join(__dirname, "..", "..", "..", "..", "..", "autorest", "dist", "app.js")
187163
if (fs.existsSync(result)) {
188164
log.silly(`Found autoRest:${result} `)
189-
return `node ${escapeShellArg(result)}`
165+
return { file: "node", args: [result] }
190166
}
191167
}
192168

@@ -195,12 +171,12 @@ export class OpenApiDiff {
195171
const result = path.resolve("node_modules/.bin/autorest")
196172
if (fs.existsSync(result)) {
197173
log.silly(`Found autoRest:${result} `)
198-
return escapeShellArg(result)
174+
return { file: result, args: [] }
199175
}
200176
}
201177

202178
// Assume that autorest is in the path
203-
return "autorest"
179+
return { file: "autorest", args: [] }
204180
}
205181

206182
/**
@@ -211,7 +187,7 @@ export class OpenApiDiff {
211187
public openApiDiffDllPath(): string {
212188
log.silly(`openApiDiffDllPath is being called`)
213189

214-
return escapeShellArg(path.join(__dirname, "..", "..", "..", "dlls", "OpenApiDiff.dll"))
190+
return path.join(__dirname, "..", "..", "..", "dlls", "OpenApiDiff.dll")
215191
}
216192

217193
/**
@@ -250,16 +226,24 @@ export class OpenApiDiff {
250226
const outputFolder = await fs.promises.mkdtemp(path.join(os.tmpdir(), "oad-"))
251227
const outputFilePath = path.join(outputFolder, `${outputFileName}.json`)
252228
const outputMapFilePath = path.join(outputFolder, `${outputFileName}.map`)
253-
// Cross-platform shell argument escaping - behavior is validated in shellEscapingTest.ts
254-
const autoRestCmd = tagName
255-
? `${this.autoRestPath()} ${escapeShellArg(swaggerPath)} --v2 --tag=${escapeShellArg(tagName)} --output-artifact=swagger-document.json` +
256-
` --output-artifact=swagger-document.map --output-file=${escapeShellArg(outputFileName)} --output-folder=${escapeShellArg(outputFolder)}`
257-
: `${this.autoRestPath()} --v2 --input-file=${escapeShellArg(swaggerPath)} --output-artifact=swagger-document.json` +
258-
` --output-artifact=swagger-document.map --output-file=${escapeShellArg(outputFileName)} --output-folder=${escapeShellArg(outputFolder)}`
259229

260-
log.debug(`Executing: "${autoRestCmd}"`)
230+
const { file: autoRestFile, args: autoRestArgs } = this.autoRestFileArgs()
231+
232+
const swaggerArgs = tagName ? [swaggerPath, `--tag=${tagName}`] : [`--input-file=${swaggerPath}`]
233+
234+
const commonArgs = [
235+
"--v2",
236+
"--output-artifact=swagger-document.json",
237+
"--output-artifact=swagger-document.map",
238+
`--output-file=${outputFileName}`,
239+
`--output-folder=${outputFolder}`
240+
]
241+
242+
const args = [...autoRestArgs, ...swaggerArgs, ...commonArgs]
243+
244+
log.debug(`Executing: "${autoRestFile} ${args.join(" ")}"`)
261245

262-
const { stderr } = await exec(autoRestCmd, {
246+
const { stderr } = await execFile(autoRestFile, args, {
263247
encoding: "utf8",
264248
maxBuffer: 1024 * 1024 * 64,
265249
env: { ...process.env, NODE_OPTIONS: "--max-old-space-size=8192" }
@@ -319,10 +303,11 @@ export class OpenApiDiff {
319303
throw new Error(`File "${newSwagger}" not found.`)
320304
}
321305

322-
const cmd = `${this.dotNetPath()} ${this.openApiDiffDllPath()} -o ${oldSwagger} -n ${newSwagger}`
306+
const file = this.dotNetPath()
307+
const args = [this.openApiDiffDllPath(), "-o", oldSwagger, "-n", newSwagger]
323308

324-
log.debug(`Executing: "${cmd}"`)
325-
const { stdout } = await exec(cmd, { encoding: "utf8", maxBuffer: 1024 * 1024 * 64 })
309+
log.debug(`Executing: "${file} ${args.join(" ")}"`)
310+
const { stdout } = await execFile(file, args, { encoding: "utf8", maxBuffer: 1024 * 1024 * 64 })
326311
const resultJson = JSON.parse(stdout) as Messages
327312

328313
const updatedJson = resultJson.map(message => ({

src/test/shellEscapingTest.ts

Lines changed: 0 additions & 116 deletions
This file was deleted.

0 commit comments

Comments
 (0)