Azure / openapi-diff

Command line tool to detect breaking changes between two openapi specifications
MIT License
255 stars 36 forks source link

Path strings should be quoted or have whitespace escaped #254

Open jantache-microsoft opened 1 year ago

jantache-microsoft commented 1 year ago

Environment:

When I run oad compare my-old-spec.json my-new-spec.json as per the instructions, I get the file not found errors. If I manually create these files out of curiosity, I still get an error.

# Initial error
> oad compare ..\..\stable\2022-01-01\redisenterprise.json .\redisenterprise.json
File "C:\Users\jantache\AppData\Local\Temp\old.json" not found.

# Error after manually creating old.json
> oad compare ..\..\stable\2022-01-01\redisenterprise.json .\redisenterprise.json
File "C:\Users\jantache\AppData\Local\Temp\new.json" not found.

# Error after manually creating both new.json and old.json
> oad compare ..\..\stable\2022-01-01\redisenterprise.json .\redisenterprise.json
Error: Command failed: dotnet C:\Program Files\WindowsPowerShell\Modules\nvm\2.5.1\vs\v14.18.1\node_modules\oad\lib\dlls\OpenApiDiff.dll -o C:\Users\jantache\AppData\Local\Temp\old.json -n C:\Users\jantache\AppData\Local\Temp\new.json -JsonValidationMessages
Could not execute because the specified command or file was not found.
Possible reasons for this include:
  * You misspelled a built-in dotnet command.
  * You intended to execute a .NET program, but dotnet-C:\Program does not exist.
  * You intended to run a global tool, but a dotnet-prefixed executable with this name could not be found on the PATH.

    at ChildProcess.exithandler (child_process.js:383:12)
    at ChildProcess.emit (events.js:400:28)
    at maybeClose (internal/child_process.js:1058:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:293:5) {
  killed: false,
  code: 1,
  signal: null,
  cmd: 'dotnet C:\\Program Files\\WindowsPowerShell\\Modules\\nvm\\2.5.1\\vs\\v14.18.1\\node_modules\\oad\\lib\\dlls\\OpenApiDiff.dll -o C:\\Users\\jantache\\AppData\\Local\\Temp\\old.json -n C:\\Users\\jantache\\AppData\\Local\\Temp\\new.json -JsonValidationMessages'
}
jianyexi commented 1 year ago

I think you need to follow this https://github.com/Azure/openapi-diff#how-to-install to install the latest version tool

jantache-microsoft commented 1 year ago

Thanks.

So I shouldn't expect the version that's currently in the npm repository to work? The docs I'm following suggest to install this tool via npm rather than installing manually via the repository. I could request these docs be updated, but on further inspection, the version on npm is quite old.

Would it be possible to make a new npm release? I'm fine to try manual installation, but installing via npm seems better for future users.

jianyexi commented 1 year ago

I think the doc has been updated for long time, currently we push the tool to '@azure/oad', not 'oad'. You can still retrieve the latest version from npm

jantache-microsoft commented 1 year ago

Thanks, that helps.

I've just uninstalled the old version and installed the correct latest from @azure/oad. It looks like it still doesn't run correctly when the path contains spaces due to an unescaped path string.

> oad --version
0.10.2
Error: Cannot find module 'C:\Program'
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:985:15)
    at Function.Module._load (node:internal/modules/cjs/loader:833:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:22:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

    at ChildProcess.exithandler (child_process.js:383:12)
    at ChildProcess.emit (events.js:400:28)
    at maybeClose (internal/child_process.js:1058:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:293:5) {
  killed: false,
  code: 1,
  signal: null,
  cmd: 'node C:\\Program Files\\WindowsPowerShell\\Modules\\nvm\\2.5.1\\vs\\v14.18.1\\node_modules\\@azure\\oad\\node_modules\\autorest\\dist\\app.js --v2 --input-file=..\\..\\stable\\2022-01-01\\redisenterprise.json --output-artifact=swagger-document.json --output-artifact=swagger-document.map --output-file=old --output-folder=C:\\Users\\jantache\\AppData\\Local\\Temp',
  stdout: '',
  stderr: 'node:internal/modules/cjs/loader:988\r\n' +
    '  throw err;\r\n' +
    '  ^\r\n' +
    '\r\n' +
    "Error: Cannot find module 'C:\\Program'\r\n" +
    '    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:985:15)\r\n' +
    '    at Function.Module._load (node:internal/modules/cjs/loader:833:27)\r\n' +
    '    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)\r\n' +
    '    at node:internal/main/run_main_module:22:47 {\r\n' +
    "  code: 'MODULE_NOT_FOUND',\r\n" +
    '  requireStack: []\r\n' +
    '}\r\n'
}
jianyexi commented 1 year ago

Thanks, it's a bug, we should escape the space in the return of this function https://github.com/Azure/openapi-diff/blob/3e91e3a4bd10b2c865ab5fe83e1c0fd008004935/src/lib/validators/openApiDiff.ts#L149

jantache-microsoft commented 1 year ago

Actually, a better change as opposed to quoting or escaping whitespace would be to use less a less fallible API like execFileSync instead of execSync (or async variants)