From 0aa564d5e2ba24a9bb7312483be81ac258b5f9e3 Mon Sep 17 00:00:00 2001 From: GitLab Duo Date: Wed, 1 Oct 2025 12:04:14 +0000 Subject: [PATCH] Duo Workflow: Resolve issue #373236 --- app/assets/javascripts/locale/sprintf.js | 72 +++++-- spec/frontend/locale/sprintf_spec.js | 255 +++++++++++++++++++++++ 2 files changed, 315 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/locale/sprintf.js b/app/assets/javascripts/locale/sprintf.js index 7faf9390684651..833c71158bf4be 100644 --- a/app/assets/javascripts/locale/sprintf.js +++ b/app/assets/javascripts/locale/sprintf.js @@ -2,28 +2,76 @@ import { escape } from 'lodash'; /** * Very limited implementation of sprintf supporting only named parameters. + * Uses character-by-character parsing to properly handle %% escaping before parameter processing. + * + * This implementation correctly handles the %% escape sequence according to standard sprintf behavior: + * - %% is converted to a single % character + * - %% escaping is processed BEFORE parameter replacement + * - This ensures %%{param}%% becomes %{param}% (not the parameter value) + * + * Examples: + * - sprintf('Hello %{name}', {name: 'World'}) → 'Hello World' + * - sprintf('%%{name}%%', {name: 'test'}) → '%{name}%' + * - sprintf('Progress: %%50', {}) → 'Progress: %50' + * - sprintf('%%%%', {}) → '%%' + * * @param {string} input - (translated) text with parameters (e.g. '%{num_users} users use us') * @param {Object.} [parameters] - object mapping parameter names to values (e.g. { num_users: 5 }) * @param {boolean} [escapeParameters=true] - whether parameter values should be escaped (see https://lodash.com/docs/4.17.15#escape) * @returns {string} the text with parameters replaces (e.g. '5 users use us') * @see https://ruby-doc.org/core-2.3.3/Kernel.html#method-i-sprintf * @see https://gitlab.com/gitlab-org/gitlab-foss/issues/37992 + * @see https://gitlab.com/gitlab-org/gitlab/-/issues/373236 */ export default function sprintf(input, parameters, escapeParameters = true) { - let output = input; + let output = ''; + const length = input.length; + const parameterRegex = /^{(\w+)}/; + const mappedParameters = parameters ? new Map(Object.entries(parameters)) : new Map(); - output = output.replace(/%+/g, '%'); + // Character-by-character parsing to handle %% escaping correctly + // This approach ensures %% is processed before parameter replacement, + // which is critical for correct sprintf behavior + for (let i = 0; i < length; i++) { + const char = input[i]; + const nextChar = input[i + 1]; - if (parameters) { - const mappedParameters = new Map(Object.entries(parameters)); - - mappedParameters.forEach((key, parameterName) => { - const parameterValue = mappedParameters.get(parameterName); - const escapedParameterValue = escapeParameters ? escape(parameterValue) : parameterValue; - // Pass the param value as a function to ignore special replacement patterns like $` and $'. - // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace#syntax - output = output.replace(new RegExp(`%{${parameterName}}`, 'g'), () => escapedParameterValue); - }); + if (char === '%') { + if (nextChar === '%') { + // Handle %% escape sequence - output single % and skip next character + // This must be processed before parameter replacement to match sprintf behavior + output += '%'; + i++; // Skip the next % + } else if (nextChar === '{') { + // Look for parameter pattern starting after the % + const remainingInput = input.substring(i + 1); + const match = parameterRegex.exec(remainingInput); + + if (match) { + const parameterName = match[1]; + const parameterValue = mappedParameters.get(parameterName); + + if (parameterValue !== undefined) { + const escapedParameterValue = escapeParameters ? escape(parameterValue) : parameterValue; + output += escapedParameterValue; + // Skip past the entire parameter pattern %{parameterName} + i += match[0].length + 1; // +1 for the initial % + } else { + // Parameter not found, output the % character as-is + output += char; + } + } else { + // No valid parameter pattern, output the % character as-is + output += char; + } + } else { + // Single % not followed by % or {, output as-is + output += char; + } + } else { + // Regular character, output as-is + output += char; + } } return output; diff --git a/spec/frontend/locale/sprintf_spec.js b/spec/frontend/locale/sprintf_spec.js index a7e245e2b7864f..b2cb53dbefdb3e 100644 --- a/spec/frontend/locale/sprintf_spec.js +++ b/spec/frontend/locale/sprintf_spec.js @@ -83,6 +83,72 @@ describe('locale', () => { expect(output).toBe('contains duplicated 15%'); }); + + it('handles %%{param}%% pattern correctly', () => { + const input = '%%{foo}%%'; + const parameters = { + foo: 'bar', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('%{foo}%'); + }); + + it('handles multiple consecutive %% signs', () => { + const input = 'test %%%% more %%%%%% end'; + + const output = sprintf(input, {}, false); + + expect(output).toBe('test %% more %%% end'); + }); + + it('handles %% at the beginning and end', () => { + const input = '%%start middle end%%'; + + const output = sprintf(input, {}, false); + + expect(output).toBe('%start middle end%'); + }); + + it('handles %% with parameters mixed', () => { + const input = '%%{name}%% has %%{count}%% items'; + const parameters = { + name: 'user', + count: 5, + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('%{name}% has %{count}% items'); + }); + + it('handles single % followed by non-special characters', () => { + const input = 'progress: %50 complete'; + + const output = sprintf(input, {}, false); + + expect(output).toBe('progress: %50 complete'); + }); + + it('handles %% followed by regular parameters', () => { + const input = '%% and %{param}'; + const parameters = { + param: 'value', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('% and value'); + }); + + it('handles empty parameters with %% patterns', () => { + const input = '%%{missing}%% and %%{also_missing}%%'; + + const output = sprintf(input, {}, false); + + expect(output).toBe('%{missing}% and %{also_missing}%'); + }); }); describe('ignores special replacements in the input', () => { @@ -95,5 +161,194 @@ describe('locale', () => { expect(output).toBe(`My odd ${replacement} is preserved`); }); }); + + describe('backward compatibility', () => { + it('maintains existing behavior for normal parameters', () => { + const input = 'Hello %{name}, you have %{count} messages'; + const parameters = { + name: 'John', + count: 3, + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('Hello John, you have 3 messages'); + }); + + it('maintains existing behavior for missing parameters', () => { + const input = 'Hello %{name}, you have %{missing} messages'; + const parameters = { + name: 'John', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('Hello John, you have %{missing} messages'); + }); + + it('maintains existing behavior for parameter escaping', () => { + const input = 'User: %{userInput}'; + const parameters = { + userInput: '', + }; + + const output = sprintf(input, parameters, true); + + expect(output).toBe('User: <script>alert("xss")</script>'); + }); + + it('maintains existing behavior for no escaping', () => { + const input = 'Safe content: %{content}'; + const parameters = { + content: 'bold', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('Safe content: bold'); + }); + + it('maintains existing behavior for multiple occurrences', () => { + const input = '%{verb} or not to %{verb}, that is the %{noun}'; + const parameters = { + verb: 'be', + noun: 'question', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('be or not to be, that is the question'); + }); + + it('maintains existing behavior for complex parameter names', () => { + const input = 'Value: %{param_name} and %{param123}'; + const parameters = { + param_name: 'test', + param123: 456, + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('Value: test and 456'); + }); + }); + + describe('edge cases and comprehensive testing', () => { + it('handles malformed parameter patterns', () => { + const input = 'Test %{incomplete and %{valid}'; + const parameters = { + valid: 'works', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('Test %{incomplete and works'); + }); + + it('handles empty parameter names', () => { + const input = 'Test %{} and %{valid}'; + const parameters = { + valid: 'works', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('Test %{} and works'); + }); + + it('handles parameters with special characters in values', () => { + const input = 'Special: %{special}'; + const parameters = { + special: '$&$`$\'$$', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('Special: $&$`$\'$$'); + }); + + it('handles nested %% patterns with parameters', () => { + const input = '%%%{param}%%%'; + const parameters = { + param: 'test', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('%test%'); + }); + + it('handles string ending with %', () => { + const input = 'Progress: 50%'; + + const output = sprintf(input, {}, false); + + expect(output).toBe('Progress: 50%'); + }); + + it('handles string ending with %%', () => { + const input = 'Progress: 50%%'; + + const output = sprintf(input, {}, false); + + expect(output).toBe('Progress: 50%'); + }); + + it('handles undefined parameter values', () => { + const input = 'Value: %{undefined_param}'; + const parameters = { + undefined_param: undefined, + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('Value: %{undefined_param}'); + }); + + it('handles null parameter values', () => { + const input = 'Value: %{null_param}'; + const parameters = { + null_param: null, + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('Value: null'); + }); + + it('handles zero parameter values', () => { + const input = 'Count: %{count}'; + const parameters = { + count: 0, + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('Count: 0'); + }); + + it('handles empty string parameter values', () => { + const input = 'Value: "%{empty}"'; + const parameters = { + empty: '', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('Value: ""'); + }); + + it('handles very long parameter names', () => { + const longParamName = 'very_long_parameter_name_that_exceeds_normal_length'; + const input = `Value: %{${longParamName}}`; + const parameters = { + [longParamName]: 'success', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('Value: success'); + }); + }); }); }); -- GitLab