Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,22 @@ import { getAllVariables, getTreePathFromCollectionToItem, mergeHeaders } from '
import { resolveInheritedAuth } from 'utils/auth';
import { get } from 'lodash';
import { interpolateAuth, interpolateHeaders, interpolateBody, interpolateParams } from './interpolation';
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
});
};
Comment on lines +13 to +21
Copy link
Collaborator

@gopu-bruno gopu-bruno Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.


const addCurlAuthFlags = (curlCommand, auth) => {
if (!auth || !curlCommand) return curlCommand;
Expand Down Expand Up @@ -79,6 +95,17 @@ const generateSnippet = ({ language, item, collection, shouldInterpolate = false
result = addCurlAuthFlags(result, effectiveAuth);
}

// Respect encodeUrl setting: when not explicitly true, replace HTTPSnippet's encoded URL with the raw URL
// encodeUrl defaults to false in the UI when undefined/null
const settings = item.draft ? get(item, 'draft.settings') : get(item, 'settings');
if (settings?.encodeUrl !== true) {
const rawUrl = request.url;
const encodedUrl = getEncodedUrl(rawUrl);
if (encodedUrl !== rawUrl) {
result = result.replaceAll(encodedUrl, rawUrl);
}
}
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. 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.
  2. 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.
  3. 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.
  4. 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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. 👍


return result;
} catch (error) {
console.error('Error generating code snippet:', error);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -906,3 +906,127 @@ describe('generateSnippet – digest and NTLM auth curl export', () => {
expect(result).toMatch(/^curl --digest --user 'myuser'/);
});
});

describe('generateSnippet – encodeUrl setting', () => {
const language = { target: 'shell', client: 'curl' };
const baseCollection = { root: { request: { auth: { mode: 'none' }, headers: [] } } };

// Replicate HTTPSnippet's internal encoding pipeline
const getEncodedUrl = (url) => {
const { parse, format } = require('url');
const { stringify } = require('querystring');
const parsed = parse(url, true, true);
if (!parsed.query || Object.keys(parsed.query).length === 0) {
return url;
}
const search = stringify(parsed.query);
return format({
...parsed,
search,
query: parsed.query,
path: search ? `${parsed.pathname}?${search}` : parsed.pathname
});
};

const makeItem = (url, settings, draft) => ({
uid: 'enc-req',
request: {
method: 'GET',
url,
headers: [],
body: { mode: 'none' },
auth: { mode: 'none' }
},
...(settings !== undefined && { settings }),
...(draft !== undefined && { draft })
});

beforeEach(() => {
jest.clearAllMocks();
// Mock HTTPSnippet to simulate encoding (same pipeline as the real library)
require('httpsnippet').HTTPSnippet = jest.fn().mockImplementation((harRequest) => ({
convert: jest.fn(() => {
const method = harRequest?.method || 'GET';
const url = harRequest?.url || 'http://example.com';
const encodedUrl = getEncodedUrl(url);
return `curl -X ${method} '${encodedUrl}'`;
})
}));
});

it('should preserve equals signs in query values when encodeUrl is false', () => {
const rawUrl = 'https://example.com/api?token=abc123==&type=test';
const item = makeItem(rawUrl, { encodeUrl: false });

const result = generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false });
expect(result).toContain('token=abc123==');
// %3D = encoded '='
expect(result).not.toContain('%3D');
});

it('should preserve email with plus alias and @ when encodeUrl is false', () => {
const rawUrl = 'https://example.com/invite?email=test+alias@example.com';
const item = makeItem(rawUrl, { encodeUrl: false });

const result = generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false });
expect(result).toContain('email=test+alias@example.com');
});

it('should preserve redirect URL with colons and slashes when encodeUrl is false', () => {
const rawUrl = 'https://example.com/auth?redirect=https://other.com/callback&scope=read';
const item = makeItem(rawUrl, { encodeUrl: false });

const result = generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false });
expect(result).toContain('redirect=https://other.com/callback');
// %3A = encoded ':'
expect(result).not.toContain('%3A');
// %2F = encoded '/'
expect(result).not.toContain('%2F');
});

it('should preserve comma-separated values when encodeUrl is false', () => {
const rawUrl = 'https://example.com/filter?tags=a,b,c&time=10:30';
const item = makeItem(rawUrl, { encodeUrl: false });

const result = generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false });
expect(result).toContain('tags=a,b,c');
expect(result).toContain('time=10:30');
});

it('should encode URL when encodeUrl is true', () => {
const rawUrl = 'https://example.com/api?token=abc123==&type=test';
const item = makeItem(rawUrl, { encodeUrl: true });

const result = generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false });
// %3D%3D = encoded '=='
expect(result).toContain('%3D%3D');
});

it('should preserve raw URL when settings are absent (encodeUrl defaults to false)', () => {
const rawUrl = 'https://example.com/auth?redirect=https://other.com/callback';
const item = makeItem(rawUrl);

const result = generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false });
expect(result).toContain('redirect=https://other.com/callback');
// %3A = encoded ':'
expect(result).not.toContain('%3A');
});

it('should be a no-op for URLs without query params', () => {
const rawUrl = 'https://example.com/api/users';
const item = makeItem(rawUrl, { encodeUrl: false });

const result = generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false });
expect(result).toBe(`curl -X GET '${rawUrl}'`);
});

it('should use draft settings when draft exists', () => {
const rawUrl = 'https://example.com/api?token=abc123==&type=test';
const item = makeItem(rawUrl, { encodeUrl: true }, { settings: { encodeUrl: false } });

const result = generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false });
expect(result).toContain('token=abc123==');
// %3D%3D = encoded '=='
expect(result).not.toContain('%3D%3D');
});
});
Loading