fix(app): quote the url in the generated curl command - #2842
Conversation
`CurlGenerator` interpolated `params.url` into the snippet unquoted, so any
shell metacharacter in the URL broke the command that "Copy Request as Curl"
puts on the clipboard. A query string is the common case:
http.url = https://api.example.com/v1/search?q=checkout&page=2
pastes as `curl https://api.example.com/v1/search?q=checkout&page=2 \` — bash
splits on the `&`, so curl gets one argument ending at `q=checkout`, `page=2`
is dropped, and the backslash continuation is severed from the method and the
headers, which run as a second command and fail with `-X: command not found`.
`escapeStringPosix`, the Chrome DevTools escaper this file already uses for
`--data-raw`, is applied to the URL as well.
The same argument applies to header values, which `getCurlHeaders` wraps in
double quotes while escaping only `\` and `"`, leaving `$` and backticks live.
That is left alone here to keep this to one concern.
🦋 Changeset detectedLatest commit: 8682f56 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@shuvamk is attempting to deploy a commit to the HyperDX Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThe PR updates “Copy Request as Curl” to POSIX-quote request URLs and adds regression coverage for query strings, shell metacharacters, and single quotes.
Confidence Score: 4/5The PR does not yet appear safe to merge because generating the HTTP request curl command can still crash when its URL is nullish. The HTTP panel passes its dynamically sourced URL to the new string escaper before checking whether request metadata is absent, so a nullish URL causes render-time curl generation to throw. Files Needing Attention: packages/app/src/utils/curlGenerator.ts and packages/app/src/components/NetworkPropertyPanel.tsx
|
| Filename | Overview |
|---|---|
| packages/app/src/utils/curlGenerator.ts | Routes the URL through the existing POSIX shell escaper. |
| packages/app/src/utils/tests/curlGenerator.test.ts | Adds regression tests for query strings and shell metacharacters in URLs. |
| .changeset/quote-curl-url.md | Documents the curl URL quoting fix as an application patch. |
Reviews (2): Last reviewed commit: "Merge branch 'main' into claude/quote-cu..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ): string { | ||
| let curlSnippet = 'curl '; | ||
| curlSnippet += params.url; | ||
| curlSnippet += escapeStringPosix(params.url); |
There was a problem hiding this comment.
When an HTTP event has a method or status but no http.url, NetworkPropertyPanel still calls CurlGenerator, and the new escapeStringPosix(params.url) call dereferences the nullish value, causing the HTTP Request panel to throw instead of displaying the available request metadata.
There was a problem hiding this comment.
@shuvamk would you mind addressing this one? Either here or with guards in NetworkPropertySubpanel?
2 participants
Summary
CurlGenerator(packages/app/src/utils/curlGenerator.ts:209) appendsparams.urlto the snippet unquoted, so Copy Request as Curl in the HTTP Request panel puts a command on the clipboard that the shell re-splits. Forhttp.url = https://api.example.com/v1/search?q=checkout&page=2it generates:Pasted into bash, the
&ends the command: curl is backgrounded with the single argumenthttps://api.example.com/v1/search?q=checkout,page=2is silently dropped, and the continuation is severed from the method and the headers, which run as a second command and fail with-X: command not found(exit 127). Spaces,;,$(…)and backticks in a URL go wrong the same way.The file already has
escapeStringPosix— the Chrome DevTools POSIX escaper — and uses it for--data-raw. This applies it to the URL as well.The same argument arguably applies to header values:
getCurlHeaderswraps them in double quotes but escapes only\and", leaving$and backticks live. I left that alone to keep this to one concern — happy to send it separately if you want it.Tests: a new
packages/app/src/utils/__tests__/curlGenerator.test.tscovers the query-string case, three metacharacter URLs and the ANSI-C quoting branch. All five fail onmainand pass with this change.make ci-unitandmake ci-lintare green locally, withpackages/applint warnings unchanged at 661. Changeset included.Screenshots or video
N/A — the change is to copied text, not to anything rendered.
How to test on Vercel preview
Preview routes: /search
Steps:
/search, pick a source with HTTP spans, and click a row whosehttp.urlcarries two or more query parameters.curlinvocation, instead of the shell cutting it at the first&and reporting-X: command not found.References