-
Couldn't load subscription status.
- Fork 9.2k
Fixes #10540 — Vertical bar character | not escaped in cURL (CMD) request snippet body #10596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…ippet body for Windows CMD compatibility
…er-pipe-not-escaped-curl-cmd-snippet
|
@MichakrawSB Please can you review this pr |
|
Thanks for this fix! The fix itself seems fine to me, but could you please remove changes unrelated to the issue? Also, could you provide a test for your fix? It should be located under |
…dd unit test. Remove unrelated changes.
…scaped-curl-cmd-snippet' of https://github.com/Divyanshu-Mishra9620/swagger-ui into fix-swagger-api#10540-Vertical-bar-character-pipe-not-escaped-curl-cmd-snippet
…er-pipe-not-escaped-curl-cmd-snippet
Title: Fix: Escape vertical bar (|) in cURL (CMD) request snippet body (#10540)Description:Escapes the vertical bar character | in cURL (CMD) request snippets with a caret ^ for Windows CMD compatibility. Removes unrelated changes and unnecessary files from the PR.These .LICENSE.txt files in the dist folder are automatically generated by Webpack when building the project. Testing:All tests pass, including the new unit test for CMD snippet escaping. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the tests! I left a few comments requesting some changes.
As for the changes to files in the dist folder - they are updated only during the release process, as can be seen in the commit history here.
Co-authored-by: Oliwia Rogala <oliwia.rogala97@gmail.com>
Removed test case for escaping vertical bar in bash.
Hey @glowcloud made the changes as suggested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I requested some last changes:
- I tested the PR locally and the linting did not pass for the test file. Our style guide prefers using double quotes when possible and avoids semicolons - I left comments related to that.
- As mentioned before, we do not accept changes to any files in
distfolder through PRs. Please remove any changes done there.
|
|
||
| describe("escapeCMD", () => { | ||
| it("escapes vertical bar | with caret ^ for CMD", () => { | ||
| const input = 'foo|bar'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const input = 'foo|bar'; | |
| const input = "foo|bar" |
| describe("escapeCMD", () => { | ||
| it("escapes vertical bar | with caret ^ for CMD", () => { | ||
| const input = 'foo|bar'; | ||
| const output = escapeCMD(input); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const output = escapeCMD(input); | |
| const output = escapeCMD(input) |
| it("escapes vertical bar | with caret ^ for CMD", () => { | ||
| const input = 'foo|bar'; | ||
| const output = escapeCMD(input); | ||
| expect(output).toContain('^|'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| expect(output).toContain('^|'); | |
| expect(output).toContain("^|") |
| const input = 'foo|bar'; | ||
| const output = escapeCMD(input); | ||
| expect(output).toContain('^|'); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }); | |
| }) |
| }); | ||
|
|
||
| it("escapes other CMD special characters", () => { | ||
| expect(escapeCMD('foo^bar')).toContain('^^'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| expect(escapeCMD('foo^bar')).toContain('^^'); | |
| expect(escapeCMD("foo^bar")).toContain("^^") |
|
|
||
| it("escapes other CMD special characters", () => { | ||
| expect(escapeCMD('foo^bar')).toContain('^^'); | ||
| expect(escapeCMD('foo"bar')).toContain('""'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| expect(escapeCMD('foo"bar')).toContain('""'); | |
| expect(escapeCMD('foo"bar')).toContain('""') |
| it("escapes other CMD special characters", () => { | ||
| expect(escapeCMD('foo^bar')).toContain('^^'); | ||
| expect(escapeCMD('foo"bar')).toContain('""'); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }); | |
| }) |
| expect(escapeCMD('foo^bar')).toContain('^^'); | ||
| expect(escapeCMD('foo"bar')).toContain('""'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }); | |
| }) |
- Add vertical bar (|) escaping in escapeCMD function - Update test to use double quotes and no semicolons per project style guide - Revert dev-helper-initializer.js to previous commented state Fixes swagger-api#10540
- Remove all changes to dist/ folder as they are not accepted in PRs - Remove unrelated changes to source files not part of the bug fix - Keep only the vertical bar escaping fix and its test
|
@glowcloud Hey i have fixed, if the issue still exist i will work on that. |
| const regionId = createHtmlReadyId(`${method}${path}_responses`) | ||
| const controlId = `${regionId}_select` | ||
|
|
||
| return (!nonExtensionResponses || !nonExtensionResponses.size) ? null : ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what was the reason of removing this code ? is this related to the vertical bar character problem ?
@Divyanshu-Mishra9620 It look like there are still changes unrelated to the bug, have a look at my comment for example. |
…) and revert unrelated changes
This PR fixes issue #10540 by ensuring the vertical bar (|) character is properly escaped in CMD request snippets.
|
…er-pipe-not-escaped-curl-cmd-snippet
Problem
When generating cURL (CMD) request snippets, the vertical bar character | in the request body was not escaped. In Windows CMD, this causes the string after | to be interpreted as a command, resulting in errors.
Solution
Updated the CMD snippet generator to escape | as ^| in the request body.
Enabled request snippets in the dev environment for easier testing.
Testing
Verified that the generated cURL (CMD) snippet now escapes | as ^|.
Confirmed that pasting the snippet in Windows CMD no longer causes command parsing errors.
Before
After the change=> .replace(/|/g, "^|")