Enable encodeUrl setting to control URL encoding in generated snippets#7187
Enable encodeUrl setting to control URL encoding in generated snippets#7187sanish-bruno wants to merge 2 commits intousebruno:mainfrom
Conversation
…ncoding in generated snippets
sanish-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
WalkthroughAdds URL encoding handling to the snippet generator: new URL parsing/formatting logic computes encoded vs raw paths and conditionally replaces encoded segments in generated snippets when Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (3)
16-21:searchandqueryin theformat()call are dead code.According to Node.js docs, when
urlObject.searchisundefinedandurlObject.queryis an Object,queryis used viaquerystring.stringify. However, the value ofurlObject.searchis appended to the result when it is defined — and critically, whenurlObject.pathis defined it takes precedence over bothpathnameandsearch. Since line 20 always setspath, thesearch(line 17) andquery: parsed.query(line 19) fields are never consumed byurl.formatand can be removed.♻️ Simplified format call
return format({ ...parsed, - search, - query: parsed.query, path: search ? `${parsed.pathname}?${search}` : parsed.pathname });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js` around lines 16 - 21, The call to format(...) includes dead fields search and query because path is always set (path takes precedence over pathname/search), so remove the unused search and query properties from the object; update the snippet that returns format({...parsed, search, query: parsed.query, path: search ? `${parsed.pathname}?${search}` : parsed.pathname}) to only include parsed and the computed path (use parsed.pathname when building path), i.e. eliminate search and query so url.format will consume the path correctly.
7-8: Replace deprecatedurl/querystringAPIs with WHATWGURL+URLSearchParams.
url.parse,url.format, and thequerystringmodule are all marked Stability: 0 – Deprecated in Node.js docs; the recommended path is the WHATWG URL API.A modern equivalent for
getEncodedUrlwould be:♻️ Proposed refactor using WHATWG URL + URLSearchParams
-import { parse, format } from 'url'; -import { stringify } from 'querystring'; - -const getEncodedUrl = (rawUrl) => { - const parsed = parse(rawUrl, true, true); - if (!parsed.query || Object.keys(parsed.query).length === 0) { - return rawUrl; - } - const search = stringify(parsed.query); - return format({ - ...parsed, - search, - query: parsed.query, - path: search ? `${parsed.pathname}?${search}` : parsed.pathname - }); -}; +/** + * Re-encodes the query parameters of a URL using the same percent-encoding + * pipeline that HTTPSnippet applies internally, so the encoded form can be + * located and replaced in the generated snippet. + * `@param` {string} rawUrl + * `@returns` {string} + */ +const getEncodedUrl = (rawUrl) => { + try { + const url = new URL(rawUrl); + if (!url.search) return rawUrl; + // URLSearchParams re-encodes the already-parsed query params + const params = new URLSearchParams(url.search); + url.search = params.toString(); + return url.toString(); + } catch { + return rawUrl; + } +};Note: the only backward-incompatible difference between
querystringandURLSearchParamsis thatURLSearchParamsuses+for spaces instead of%20. Verify that the new encoding matches what HTTPSnippet actually outputs for your target character set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js` around lines 7 - 8, The code currently imports and uses deprecated Node APIs (parse/format from 'url' and stringify from 'querystring'); replace them with the WHATWG URL API and URLSearchParams by removing those imports, creating a new URL(urlString) or new URL(base) instance, use urlObj.searchParams (or new URLSearchParams(params)) to build/encode query params, and use urlObj.toString() or urlObj.href instead of format; update any helper like getEncodedUrl to construct and return urlObj.href and verify encoding differences (spaces as '+' vs '%20') so the output matches HTTPSnippet expectations.
10-10: Missing JSDoc ongetEncodedUrl.The function's purpose — simulating HTTPSnippet's internal URL encoding pipeline to later locate and undo it — is non-obvious from its name alone. A JSDoc block here would clarify intent.
As per coding guidelines: "Add JSDoc comments to abstractions for additional details."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js` at line 10, Add a JSDoc block above the getEncodedUrl function that explains its purpose (it simulates HTTPSnippet's internal URL encoding pipeline so callers can later locate and undo that encoding), documents the parameter (rawUrl: string) and return type (string), and notes any important behavior or edge cases (e.g., which characters are transformed or that it mirrors HTTPSnippet's encoding). Reference the function name getEncodedUrl in the comment so readers can quickly connect the doc to the implementation.packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js (1)
915-929:getEncodedUrlin the test file is a verbatim duplicate of the implementation — the tests verify their own round-trip, not HTTPSnippet's actual encoding.The
beforeEachmock on lines 947-954 producescurl -X GET '${getEncodedUrl(url)}'using the same duplicated function, so these tests are essentially asserting that "encode then decode" is a no-op. They don't verify that the implementation correctly reverses HTTPSnippet's encoding, which may differ (e.g., encoding of+,@,:, etc.).Two options to consider:
- Export
getEncodedUrlfromsnippet-generator.jsand import it in the spec (removes duplication, tightens coupling to implementation consciously).- Use a hard-coded, hand-crafted encoded URL string in the mock (e.g.,
'token=abc123%3D%3D') to simulate the real HTTPSnippet output, making the tests independent of the helper's logic.Option 2 makes test failures much more actionable. Based on learnings, "Write behaviour-driven tests, not implementation-driven ones. Tests should validate real expected output and observable behaviour."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js` around lines 915 - 929, The test currently duplicates the implementation of getEncodedUrl (in snippet-generator.spec.js), causing the spec to assert its own round-trip rather than actual HTTPSnippet output; replace the duplicated helper with a concrete encoded URL in the beforeEach mock (e.g., use a hand-crafted encoded token/query string like 'token=abc123%3D%3D' or other examples covering +, @, :, etc.) so the mock generates curl -X GET '...encoded...' values that reflect real HTTPSnippet encoding; alternatively, if you prefer tight coupling, export getEncodedUrl from snippet-generator.js and import it into the spec (remove the local function and reference the exported getEncodedUrl) — pick one approach and update the beforeEach mock and assertions accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js`:
- Around line 98-107: The current use of result.replaceAll(encodedUrl, rawUrl)
can corrupt bodies/headers; instead replace only the URL token that httpsnippet
emits. In the block using settings, request, encodedUrl=getEncodedUrl(rawUrl)
and result, change the unconditional replaceAll to a targeted replace: build an
escaped regex that matches the encodedUrl when it appears as a quoted token
(e.g., /(["'`])<escapedEncodedUrl>\1/) or the exact pattern httpsnippet uses
around the URL and call result.replace(regex, (m, q) => q + rawUrl + q) so only
the quoted URL instance is swapped; keep the encodedUrl !== rawUrl guard and
preserve behavior when settings?.encodeUrl === true. Ensure the regex is escaped
to avoid special-chars in encodedUrl.
---
Nitpick comments:
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js`:
- Around line 16-21: The call to format(...) includes dead fields search and
query because path is always set (path takes precedence over pathname/search),
so remove the unused search and query properties from the object; update the
snippet that returns format({...parsed, search, query: parsed.query, path:
search ? `${parsed.pathname}?${search}` : parsed.pathname}) to only include
parsed and the computed path (use parsed.pathname when building path), i.e.
eliminate search and query so url.format will consume the path correctly.
- Around line 7-8: The code currently imports and uses deprecated Node APIs
(parse/format from 'url' and stringify from 'querystring'); replace them with
the WHATWG URL API and URLSearchParams by removing those imports, creating a new
URL(urlString) or new URL(base) instance, use urlObj.searchParams (or new
URLSearchParams(params)) to build/encode query params, and use urlObj.toString()
or urlObj.href instead of format; update any helper like getEncodedUrl to
construct and return urlObj.href and verify encoding differences (spaces as '+'
vs '%20') so the output matches HTTPSnippet expectations.
- Line 10: Add a JSDoc block above the getEncodedUrl function that explains its
purpose (it simulates HTTPSnippet's internal URL encoding pipeline so callers
can later locate and undo that encoding), documents the parameter (rawUrl:
string) and return type (string), and notes any important behavior or edge cases
(e.g., which characters are transformed or that it mirrors HTTPSnippet's
encoding). Reference the function name getEncodedUrl in the comment so readers
can quickly connect the doc to the implementation.
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js`:
- Around line 915-929: The test currently duplicates the implementation of
getEncodedUrl (in snippet-generator.spec.js), causing the spec to assert its own
round-trip rather than actual HTTPSnippet output; replace the duplicated helper
with a concrete encoded URL in the beforeEach mock (e.g., use a hand-crafted
encoded token/query string like 'token=abc123%3D%3D' or other examples covering
+, @, :, etc.) so the mock generates curl -X GET '...encoded...' values that
reflect real HTTPSnippet encoding; alternatively, if you prefer tight coupling,
export getEncodedUrl from snippet-generator.js and import it into the spec
(remove the local function and reference the exported getEncodedUrl) — pick one
approach and update the beforeEach mock and assertions accordingly.
There was a problem hiding this comment.
replaceAll is fragile — could corrupt body/header content containing the same encoded URL.
If encodedUrl appears in the snippet anywhere beyond the URL position (e.g., a request body field with a redirect URI or an Authorization header that embeds the URL), replaceAll silently replaces all occurrences. This is a realistic scenario for OAuth redirect-URI parameters.
A safer approach is to replace only within the quoted URL token (the known position where httpsnippet places it in the output):
🛡️ Proposed targeted replacement
-if (encodedUrl !== rawUrl) {
- result = result.replaceAll(encodedUrl, rawUrl);
-}
+if (encodedUrl !== rawUrl) {
+ // Replace only the first occurrence (the URL position in the snippet)
+ result = result.replace(encodedUrl, rawUrl);
+}Even replace (first-match) is a string heuristic; for full safety, a structured post-processing step on the language-specific template would be ideal, but that requires deeper httpsnippet integration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js`
around lines 98 - 107, The current use of result.replaceAll(encodedUrl, rawUrl)
can corrupt bodies/headers; instead replace only the URL token that httpsnippet
emits. In the block using settings, request, encodedUrl=getEncodedUrl(rawUrl)
and result, change the unconditional replaceAll to a targeted replace: build an
escaped regex that matches the encodedUrl when it appears as a quoted token
(e.g., /(["'`])<escapedEncodedUrl>\1/) or the exact pattern httpsnippet uses
around the URL and call result.replace(regex, (m, q) => q + rawUrl + q) so only
the quoted URL instance is swapped; keep the encodedUrl !== rawUrl guard and
preserve behavior when settings?.encodeUrl === true. Ensure the regex is escaped
to avoid special-chars in encodedUrl.
There was a problem hiding this comment.
The concern is that the encoded path (e.g., /api?token=abc123%3D%3D) could match in headers or body
sections, not just the URL position.
I don't think the regex approach is the right fix here. Here's why:
- Raw HTTP has no quotes around the path: GET /api?token=abc123%3D%3D HTTP/1.1 — a quoted-token regex like
/(["'])\1/` would miss this entirely. - Some shell output is unquoted too — curl's quote() function skips quoting when the URL matches
/^[a-z0-9-_/.@%^=:]+$/i, which encoded URLs with % characters do match. - The quoting varies wildly across 39+ targets — single quotes (curl), double quotes (python, java, go),
no quotes (raw HTTP), template literals (some JS targets). A regex trying to handle all of these would be
more fragile than replaceAll. - The false positive scenario is practically unrealistic — for a false match, the user's header value or
request body would need to contain the exact string /api?token=abc123%3D%3D. That's their own URL's encoded
path appearing verbatim in their own body. Even if it did happen (e.g., a Referer header), replacing it
would be the correct behavior since the intent is "don't encode my URLs."
The current replaceAll is simpler, works across all targets regardless of quoting style, and the false
positive risk is theoretical rather than practical. I'd recommend keeping it as-is.
There was a problem hiding this comment.
@sanish-bruno Fair points — you're right that a regex approach would need to handle raw HTTP, unquoted curl output, and 39+ target-specific quote styles, making it more complex than replaceAll. The false-positive scenario (encoded path appearing in headers/body) is indeed rare, and when encodeUrl: false is set, replacing those occurrences aligns with user intent anyway.
Your implementation is pragmatic and covers the multi-target breadth. Keeping it as-is makes sense. 👍
…r better clarity and functionality
There was a problem hiding this comment.
🧹 Nitpick comments (5)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js (5)
946-947: Hoistrequire('url')out of theconvertmock closure.
const { parse } = require('url')is re-executed on everyconvert()call. While Node's require cache makes this free at runtime, it's cleaner to hoist it to thedescribescope alongside thegetEncodedPathhelper's requires (same suggestion as above).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js` around lines 946 - 947, Hoist the require for the URL parser out of the convert mock closure by moving "const { parse } = require('url')" into the surrounding describe scope (next to the existing getEncodedPath helper requires), then update the convert mock to use that parse variable; this avoids re-running require on every convert() invocation while keeping the same behavior.
1008-1016: Consider adding a test forencodeUrl: null.The PR states "any non-true value" should skip encoding, and the production guard (
settings?.encodeUrl !== true) correctly handlesnull. The suite currently coversfalseand absent settings (undefined), butnullis a distinct value that could appear in serialized settings (e.g., from JSON deserialization) and isn't explicitly exercised.♻️ Suggested additional test
+ it('should preserve raw URL when encodeUrl is explicitly null', () => { + const rawUrl = 'https://example.com/auth?redirect=https://other.com/callback'; + const item = makeItem(rawUrl, { encodeUrl: null }); + + const result = generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); + expect(result).toContain('redirect=https://other.com/callback'); + expect(result).not.toContain('%3A'); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js` around lines 1008 - 1016, Add a new spec case to assert that encodeUrl: null is treated like any non-true value: in the snippet-generator test file add another it(...) similar to the existing "preserve raw URL when settings are absent" case but pass item with settings: { encodeUrl: null } (use makeItem to construct the item) and call generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); then expect the result to contain the raw redirect URL and not contain encoded tokens (e.g., '%3A'); this verifies generateSnippet's guard settings?.encodeUrl !== true handles null correctly.
916-917: Deprecated Node.js APIs used in test helper — mirrors production intentionally, but worth tracking.
url.parseandquerystring.parse/stringifyhave been deprecated since at least Node.js LTS v14. The test helper at lines 916–917 replicates this deliberately to stay in sync with the productiongetEncodedPath, which uses the same APIs to faithfully simulate HTTPSnippet's encoding pipeline. That's acceptable for now, but if the production implementation is ever migrated tonew URL()/URLSearchParams, these requires will need updating in lock-step to keep the mock accurate.The
require()calls inside the function body can also be hoisted to describe-scope:♻️ Hoist requires to describe scope
+ const { parse } = require('url'); + const { stringify } = require('querystring'); const getEncodedPath = (url) => { - const { parse } = require('url'); - const { stringify } = require('querystring'); const parsed = parse(url, true, true); ... };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js` around lines 916 - 917, Test helper uses deprecated Node APIs: the require('url').parse and require('querystring').stringify calls in the snippet-generator.spec.js test should be hoisted to describe-scope and flagged for future migration; update the spec by moving the const { parse } = require('url') and const { stringify } = require('querystring') out of the inner function into the top of the describe block (or module scope) and add a brief inline comment noting these mirror getEncodedPath’s current implementation and should be migrated to new URL()/URLSearchParams in lock-step with that production function.
970-976: Add absence assertions for encoded characters in the email test.The
+(decoded to space byurl.parse, then re-encoded as%20) and@(encoded as%40) are the exact characters this test is meant to guard. Without explicitnot.toContainassertions, a regression that partially encodes the URL would still pass. Every other test in this suite mirrors this pattern (e.g., lines 967, 985–987).♻️ Proposed addition
expect(result).toContain('email=test+alias@example.com'); + // %40 = encoded '@' + expect(result).not.toContain('%40'); + // %20 = encoded space (url.parse decodes '+' to space, querystring re-encodes as %20) + expect(result).not.toContain('%20');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js` around lines 970 - 976, The test 'should preserve email with plus alias and @ when encodeUrl is false' currently only asserts the decoded email string; add negative assertions to ensure no partial encoding occurs by asserting that the generated snippet from generateSnippet (called with item from makeItem and collection baseCollection) does not contain the encoded forms '%20' and '%40' (i.e., add expect(result).not.toContain('%20') and expect(result).not.toContain('%40')) so regressions that partially encode '+' or '@' fail.
910-924: RenamegetEncodedPathto avoid name collision with the production helper.The local
getEncodedPath(lines 915–924) shares its name with the production function insnippet-generator.js, but has a fundamentally different return type: the production version returns{ encodedPath, rawPath } | null, while this one returns a plainstring. The divergence is intentional (this one is only used to drive the mock), but the shared name will confuse future readers who expect the same contract.♻️ Suggested rename
- const getEncodedPath = (url) => { + const simulateHttpSnippetEncoding = (url) => { const { parse } = require('url'); const { stringify } = require('querystring'); const parsed = parse(url, true, true); if (!parsed.query || Object.keys(parsed.query).length === 0) { return parsed.path; } const search = stringify(parsed.query); return search ? `${parsed.pathname}?${search}` : parsed.pathname; };Then update all call sites in the
beforeEachmock (getEncodedPath(url)→simulateHttpSnippetEncoding(url)).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js` around lines 910 - 924, Rename the local test helper getEncodedPath to avoid colliding with the production helper in snippet-generator.js: change its name to simulateHttpSnippetEncoding (or similar) and update all uses in the spec's beforeEach mock from getEncodedPath(url) to simulateHttpSnippetEncoding(url); ensure the function signature/behavior remains unchanged but the new name is used everywhere in this test file so readers won't conflate it with the production getEncodedPath that returns { encodedPath, rawPath } | null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js`:
- Around line 946-947: Hoist the require for the URL parser out of the convert
mock closure by moving "const { parse } = require('url')" into the surrounding
describe scope (next to the existing getEncodedPath helper requires), then
update the convert mock to use that parse variable; this avoids re-running
require on every convert() invocation while keeping the same behavior.
- Around line 1008-1016: Add a new spec case to assert that encodeUrl: null is
treated like any non-true value: in the snippet-generator test file add another
it(...) similar to the existing "preserve raw URL when settings are absent" case
but pass item with settings: { encodeUrl: null } (use makeItem to construct the
item) and call generateSnippet({ language, item, collection: baseCollection,
shouldInterpolate: false }); then expect the result to contain the raw redirect
URL and not contain encoded tokens (e.g., '%3A'); this verifies
generateSnippet's guard settings?.encodeUrl !== true handles null correctly.
- Around line 916-917: Test helper uses deprecated Node APIs: the
require('url').parse and require('querystring').stringify calls in the
snippet-generator.spec.js test should be hoisted to describe-scope and flagged
for future migration; update the spec by moving the const { parse } =
require('url') and const { stringify } = require('querystring') out of the inner
function into the top of the describe block (or module scope) and add a brief
inline comment noting these mirror getEncodedPath’s current implementation and
should be migrated to new URL()/URLSearchParams in lock-step with that
production function.
- Around line 970-976: The test 'should preserve email with plus alias and @
when encodeUrl is false' currently only asserts the decoded email string; add
negative assertions to ensure no partial encoding occurs by asserting that the
generated snippet from generateSnippet (called with item from makeItem and
collection baseCollection) does not contain the encoded forms '%20' and '%40'
(i.e., add expect(result).not.toContain('%20') and
expect(result).not.toContain('%40')) so regressions that partially encode '+' or
'@' fail.
- Around line 910-924: Rename the local test helper getEncodedPath to avoid
colliding with the production helper in snippet-generator.js: change its name to
simulateHttpSnippetEncoding (or similar) and update all uses in the spec's
beforeEach mock from getEncodedPath(url) to simulateHttpSnippetEncoding(url);
ensure the function signature/behavior remains unchanged but the new name is
used everywhere in this test file so readers won't conflate it with the
production getEncodedPath that returns { encodedPath, rawPath } | null.
| const getEncodedPath = (rawUrl) => { | ||
| const parsed = parse(rawUrl, true, true); | ||
| if (!parsed.query || Object.keys(parsed.query).length === 0) { | ||
| return null; | ||
| } | ||
| const search = stringify(parsed.query); | ||
| const encodedPath = search ? `${parsed.pathname}?${search}` : parsed.pathname; | ||
| return { encodedPath, rawPath: parsed.path }; | ||
| }; |
There was a problem hiding this comment.
@sanish-bruno, Found these consistent issues:
Spaces stay encoded even when encodeUrl is false
For example: https://example.com/api?search=hello world
Even with encodeUrl: false, the generated snippet still contains %20 instead of preserving the space as written.
Hash fragment lost in URLs with #
For example: https://example.com/api?token=abc==#section
With encodeUrl: false, the #section fragment is not preserved correctly and gets lost in the generated snippet.
2 participants
Summary
Jira
Fixes #5756 — Generate Code now respects the
encodeUrlsetting on requests.Previously, the "Generate Code" feature always produced encoded query parameters (e.g.,
token=abc123%3D%3D) regardless of the request'sencodeUrlsetting. This happened because HTTPSnippet internally encodes all query parameters viaurl.parse()→querystring.stringify()→url.format(), and there's no API to disable this behavior.What changed
snippet-generator.jsgetEncodedPath()helper that replicates HTTPSnippet's internal encoding pipeline using the same Node.js modules (url.parse,querystring.stringify) to compute the encoded path+query portionencodeUrlis not explicitlytrue, the encoded path+query in the output is replaced with the raw path+queryhttp.client, raw HTTP) use it directlyitem.draft.settingsfirst (for unsaved changes), falling back toitem.settings— same pattern used in request executionencodeUrldefaults tofalsein the UI whenundefined/null, so we treat any non-truevalue as "don't encode"snippet-generator.spec.js?token=abc123==) —=encoded as%3D+and@(?email=test+alias@example.com) —@encoded as%40?redirect=https://other.com/callback) —:as%3A,/as%2F?tags=a,b,c&time=10:30) —,as%2C,:as%3AencodeUrl: truehttp.client) are handled correctlyHow it works
encodeUrl !== true, compute the encoded path+query using the same pipeline as HTTPSnippet (url.parse→querystring.stringify)This single replacement covers all target types because the encoded path is always present in the output — either as part of the full URL (curl, fetch) or standalone (http.client, raw HTTP). Targets that use separate
url+paramsdict (like python-requests) already have decoded values, so no replacement is needed.Test plan
npm run test --workspace=packages/bruno-app -- --testPathPattern=snippet-generatorhttps://example.com/api?token=abc123==encodeUrl: falsein request Settings tabtoken=abc123==(not%3D%3D)encodeUrl: true→ verify curl showstoken=abc123%3D%3Dtoken=abc123==(not%3D%3D)Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit