diff --git a/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js b/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js index 111f53fd964..c5b00b81b82 100644 --- a/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js +++ b/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js @@ -1,11 +1,10 @@ import { buildHar } from '@usebruno/common'; -import { stripOrigin } from '@usebruno/common/utils'; +import { stripOrigin, buildQueryString } from '@usebruno/common/utils'; import { getAllVariables, getTreePathFromCollectionToItem, mergeHeaders } from 'utils/collections/index'; import { resolveInheritedAuth } from 'utils/auth'; import { get } from 'lodash'; import { interpolateUrl, interpolateUrlPathParams, prependDefaultScheme } from 'utils/url/index'; import { parse } from 'url'; -import { stringify } from 'query-string'; // curl --digest / --ntlm are surface-level snippet adjustments, not part of // the HAR contract — keep them at this layer. @@ -96,8 +95,12 @@ const generateSnippet = async ({ language, item, collection, shouldInterpolate = * their own bytes displayed; GenerateCodeItem no longer sets it. */ const displayRawUrl = item.rawUrl || rawUrl; - const parsed = parse(encodedUrl, true, true); - const search = stringify(parsed.query, { sort: false }); + // Anchor on har.queryString (the same structured source HTTPSnippet encoded from), + // not a re-flattened/re-split `encodedUrl` — a value containing a literal `&` splits + // into bogus extra params under a naive query-string reparse, so the anchor no longer + // matches what HTTPSnippet actually rendered and the swap below silently no-ops. + const parsed = parse(encodedUrl, false, true); + const search = buildQueryString(har.queryString, { encode: true }); const httpSnippetPath = search ? `${parsed.pathname}?${search}` : parsed.pathname; let desiredPath; diff --git a/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js b/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js index 600d814eda6..683660bc367 100644 --- a/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js +++ b/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js @@ -950,6 +950,25 @@ describe('generateSnippet – encodeUrl setting', () => { expect(result).toContain('time=10:30'); }); + it('should preserve a literal & inside a query value when encodeUrl is false', async () => { + const rawUrl = 'https://echo.usebruno.com/?search=bruno&test'; + const item = makeItem(rawUrl, { encodeUrl: false }); + item.request.params = [{ name: 'search', value: 'bruno&test', type: 'query', enabled: true }]; + + const result = await generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); + expect(result).toContain('search=bruno&test'); + expect(result).not.toContain('%26'); + }); + + it('should encode a literal & inside a query value when encodeUrl is true', async () => { + const rawUrl = 'https://echo.usebruno.com/?search=bruno&test'; + const item = makeItem(rawUrl, { encodeUrl: true }); + item.request.params = [{ name: 'search', value: 'bruno&test', type: 'query', enabled: true }]; + + const result = await generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); + expect(result).toContain('search=bruno%26test'); + }); + it('should encode URL when encodeUrl is true', async () => { const rawUrl = 'https://example.com/api?token=abc123==&type=test'; const item = makeItem(rawUrl, { encodeUrl: true }); diff --git a/packages/bruno-common/src/generate-code/har/index.ts b/packages/bruno-common/src/generate-code/har/index.ts index c8c06a81c1b..6ba2ad29b09 100644 --- a/packages/bruno-common/src/generate-code/har/index.ts +++ b/packages/bruno-common/src/generate-code/har/index.ts @@ -623,7 +623,32 @@ export async function buildHar(input: BuildHarInput): Promise { const leadingToken = needsSyntheticScheme ? hashedUrl.match(/^[A-Za-z0-9._-]+/)?.[0] ?? '' : ''; const urlForPipeline = needsSyntheticScheme ? syntheticScheme + hashedUrl : hashedUrl; - // Step 3 — Hash path-param positions via `patternHasher`. The URL now + // Step 3 — Query string array. HAR's queryString is the single source of truth for + // what HTTPSnippet renders into the URL slot, and — via the `queryParams` + // override below — also for what `encodeUrl()` renders for `encodedUrl`. + // A flattened URL string can't tell a literal `&` inside a value apart + // from the separator between params (`search=bruno&test` mis-splits into + // two params on a naive re-parse), so both the HAR queryString and the + // encoded URL must be built from this same structured array rather than + // by re-parsing the URL text. + // + // Hashing the assembled values is what keeps `{{var}}` alive in a query when the + // caller wants templates preserved. HTTPSnippet runs encodeURIComponent over every + // queryString value, which would render `{{apiKey}}` as `%7B%7BapiKey%7D%7D` with + // nothing left for `unhash` to map back. Hash tokens are alphanumeric+dash, so they + // survive that pass untouched. Hashing here rather than in `working.params` also + // covers the auth-driven params `buildQueryString` appends (apikey in queryparams). + const queryValueRestorers: ((input: string) => string)[] = []; + const harQueryString = buildQueryString(working, hashedUrl).map((param) => { + if (shouldInterpolate || typeof param.value !== 'string') { + return param; + } + const { hashed, restore } = patternHasher(param.value); + queryValueRestorers.push(restore); + return { ...param, value: hashed }; + }); + + // Step 4 — Hash path-param positions via `patternHasher`. The URL now // contains opaque `bruno-var-hash-XXX` tokens instead of `:id`, so the // next `encodeUrl()` pass can encode non-path-param chars in the path // without touching path-param positions. `restorePathParams` (returned @@ -632,13 +657,14 @@ export async function buildHar(input: BuildHarInput): Promise { const encodeFlag = working.settings?.encodeUrl === true; const { url: urlWithPlaceholders, restore: restorePathParams } = hashPathParamPositions(urlForPipeline, working.pathParams); - // Step 4 — Apply `encodeUrl()` to the URL with placeholders. Placeholders + // Step 5 — Apply `encodeUrl()` to the URL with placeholders. Placeholders // are alphanumeric+dash, so `encodeURIComponent` (used per path segment - // inside `encodeUrl`) leaves them untouched. The rest of the path and - // query are encoded per the existing content-blind contract (PR #5507). - const encodedUrlWithPlaceholders = encodeUrl(urlWithPlaceholders); + // inside `encodeUrl`) leaves them untouched. The rest of the path is + // encoded per the existing content-blind contract (PR #5507); the query + // is rebuilt from `harQueryString` rather than re-parsed off the URL. + const encodedUrlWithPlaceholders = encodeUrl(urlWithPlaceholders, { queryParams: harQueryString }); - // Step 5 — Restore placeholders. `rawUrl` always uses raw values (for the + // Step 6 — Restore placeholders. `rawUrl` always uses raw values (for the // toggle-OFF display swap upstream). `encodedUrl` uses single-encoded // values when toggle is ON, raw when OFF. const rawUrl = restorePathParams(urlWithPlaceholders, { encode: false }); @@ -649,7 +675,7 @@ export async function buildHar(input: BuildHarInput): Promise { throw new Error('invalid request url'); } - // Step 5 — Auth → headers. Append to request headers. Request-signing auth (EdgeGrid) must + // Step 7 — Auth → headers. Append to request headers. Request-signing auth (EdgeGrid) must // sign the same `encodedUrl` the snippet transmits, or the signature won't cover the sent bytes. const authHeaders = await authToHeaders(working.auth, variables, input.oauth2Credentials, input.collectionUid, { method: working.method || 'GET', @@ -659,37 +685,15 @@ export async function buildHar(input: BuildHarInput): Promise { }); const allHeaders = mergeAndDedupeHeaders(working.headers, authHeaders); - // Step 6 — Finalize headers (filter enabled, lowercase, default content-type). + // Step 8 — Finalize headers (filter enabled, lowercase, default content-type). const harHeaders = finalizeHeaders(working, allHeaders); - // Step 7 — Query string array. HAR's queryString is the single source of - // truth for what HTTPSnippet renders into the URL slot. The URL itself - // (next step) has its query stripped to avoid the legacy-polyfill merge bug. - // The fallback source is the *hashed* URL rather than the encoded one, so that - // when it is used its values carry user-typed bytes: feeding the encoded URL - // here would double-encode (`:` → `%3A` from encodeUrl, then `%3A` → `%253A` - // from HTTPSnippet's encodeURIComponent pass). - // - // Hashing the assembled values is what keeps `{{var}}` alive in a query when the - // caller wants templates preserved. HTTPSnippet runs encodeURIComponent over every - // queryString value, which would render `{{apiKey}}` as `%7B%7BapiKey%7D%7D` with - // nothing left for `unhash` to map back. Hash tokens are alphanumeric+dash, so they - // survive that pass untouched. Hashing here rather than in `working.params` also - // covers the auth-driven params `buildQueryString` appends (apikey in queryparams). - const queryValueRestorers: ((input: string) => string)[] = []; - const harQueryString = buildQueryString(working, hashedUrl).map((param) => { - if (shouldInterpolate || typeof param.value !== 'string') { - return param; - } - const { hashed, restore } = patternHasher(param.value); - queryValueRestorers.push(restore); - return { ...param, value: hashed }; - }); - - // Step 8 — Strip the URL's query before storing in HAR (the bracket-key fix). + // Step 9 — Strip the URL's query before storing in HAR (the bracket-key fix). + // `harQueryString` (built above, before path-param hashing) is the sole + // source of truth for what HTTPSnippet renders into the URL slot. const harUrl = stripQueryStringFromUrl(encodedUrl); - // Step 9 — Assemble. + // Step 10 — Assemble. const har: HarRequest = { method: working.method || 'GET', url: harUrl, diff --git a/packages/bruno-common/src/utils/url/index.spec.ts b/packages/bruno-common/src/utils/url/index.spec.ts index ffc1ea8eec9..49941318add 100644 --- a/packages/bruno-common/src/utils/url/index.spec.ts +++ b/packages/bruno-common/src/utils/url/index.spec.ts @@ -394,6 +394,16 @@ describe('buildQueryString', () => { const result = buildQueryString(params, { encode: true }); expect(result).toBe('tag=test%23abc&a=x%26y&b=x%3Dy&c=x%3Fy&d=x%2By&e=hello%20world'); }); + + it('should encode & in a value to %26 when encode is true (Encode URL toggle ON)', () => { + const params = [{ name: 'search', value: 'bruno&test' }]; + expect(buildQueryString(params, { encode: true })).toBe('search=bruno%26test'); + }); + + it('should leave & in a value untouched when encode is false (Encode URL toggle OFF)', () => { + const params = [{ name: 'search', value: 'bruno&test' }]; + expect(buildQueryString(params)).toBe('search=bruno&test'); + }); }); describe('safeDecodeURIComponent', () => { diff --git a/packages/bruno-common/src/utils/url/index.ts b/packages/bruno-common/src/utils/url/index.ts index 2a30315e266..bc9277c72c4 100644 --- a/packages/bruno-common/src/utils/url/index.ts +++ b/packages/bruno-common/src/utils/url/index.ts @@ -69,6 +69,17 @@ interface BuildQueryStringOptions { encode?: boolean; } +interface EncodeUrlOptions { + /** + * Structured query params to encode instead of re-splitting the URL's query + * string on `&`. A flattened URL can't tell a literal `&` inside a value + * apart from the separator between params, so a value like `bruno&test` + * mis-splits into two params unless the caller supplies the original + * structured array (e.g. the request's `params`) here. + */ + queryParams?: QueryParam[]; +} + interface ExtractQueryParamsOptions { decode?: boolean; /** @@ -183,7 +194,7 @@ const encodePathSegments = (path: string): string => // encoding `#` as data is the predictable choice. To send a URL with a // literal `#section` fragment, toggle OFF — OFF preserves the user's URL // byte-for-byte. -const encodeUrl = (url: string): string => { +const encodeUrl = (url: string, { queryParams }: EncodeUrlOptions = {}): string => { if (!url || typeof url !== 'string') { return url; } @@ -209,19 +220,8 @@ const encodeUrl = (url: string): string => { if (queryIdx >= 0) { // stripFragment: false so `#` in the query value is treated as a literal // byte and gets encoded to `%23` by the encodeURIComponent below. - const params = parseQueryParams(queryString, { decode: false, stripFragment: false }); - const rebuilt = params - .map(({ name, value }) => { - const encodedName = encodeURIComponent(name); - if (value === undefined) { - return encodedName; - } - const encodedValue = encodeURIComponent(value); - return `${encodedName}=${encodedValue}`; - }) - .filter((pair) => pair.length > 0 && !pair.startsWith('=')) - .join('&'); - result += `?${rebuilt}`; + const params = queryParams ?? parseQueryParams(queryString, { decode: false, stripFragment: false }); + result += `?${buildQueryString(params, { encode: true })}`; } return result; @@ -346,6 +346,7 @@ export { isSameOrigin, type QueryParam, type BuildQueryStringOptions, + type EncodeUrlOptions, type ExtractQueryParamsOptions, type MockResponseRouteKeyInput };