Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion application/ui.extkeywidget.class.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ public function DisplaySelect(WebPage $oPage, $iMaxComboLength, $bAllowTargetCre
foreach ($aAdditionalField as $sAdditionalField) {
array_push($aArguments, $oObj->Get($sAdditionalField));
}
$aOption['additional_field'] = utils::HtmlEntities(vsprintf($sFormatAdditionalField, $aArguments));
$aOption['additional_field'] = utils::HtmlEntities(utils::VSprintf($sFormatAdditionalField, $aArguments));
}
if (!empty($sObjectImageAttCode)) {
// Try to retrieve image for contact
Expand Down
121 changes: 121 additions & 0 deletions application/utils.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -2075,6 +2075,127 @@ public static function EscapedHtmlDecode($sValue)
);
}

/**
* Format a string using vsprintf with safety checks to avoid ValueError
*
* This method fills missing arguments with their original format specifiers,
* then calls vsprintf with the complete array.
*
* @param string $sFormat The format string
* @param array $aArgs The arguments to format
* @param bool $bLogErrors Whether to log errors (defaults to true)
*
* @return string The formatted string
* @since 3.2.2
*/
public static function VSprintf(string $sFormat, array $aArgs, bool $bLogErrors = true): string
{
// Extract all format specifiers
$sPattern = '/%(?:(?:[1-9][0-9]*)\$)?[-+\'0# ]*(?:[0-9]*|\*)?(?:\.(?:[0-9]*|\*))?(?:[hlL])?[diouxXeEfFgGcrs%]/';
preg_match_all($sPattern, $sFormat, $aMatches, PREG_OFFSET_CAPTURE);

// Process matches, keeping track of their positions and excluding escaped percent signs (%%)
$aSpecifierMatches = [];
foreach ($aMatches[0] as $sMatch) {
if ($sMatch[0] !== '%%') {
$aSpecifierMatches[] = $sMatch;
}
}

// Check for positional specifiers and build position map
$bHasPositional = false;
$iMaxPosition = 0;
$aPositions = [];
$aUniquePositions = [];

foreach ($aSpecifierMatches as $index => $match) {
$sSpec = $match[0];
if (preg_match('/^%([1-9][0-9]*)\$/', $sSpec, $posMatch)) {
$bHasPositional = true;
$iPosition = (int)$posMatch[1] - 1; // Convert to 0-based
$aPositions[$index] = $iPosition;
$aUniquePositions[$iPosition] = true;
$iMaxPosition = max($iMaxPosition, $iPosition + 1);
} else {
$aPositions[$index] = $index;
$aUniquePositions[$index] = true;
$iMaxPosition = max($iMaxPosition, $index + 1);
}
}

// Count unique positions, this tells us how many arguments we actually need
$iExpectedCount = count($aUniquePositions);
$iActualCount = count($aArgs);

// If we have enough arguments, just use vsprintf
if ($iActualCount >= $iExpectedCount) {
return vsprintf($sFormat, $aArgs);
}
// else log the error if needed
if ($bLogErrors) {
IssueLog::Warning("Format string requires $iExpectedCount arguments, but only $iActualCount provided. Format: '$sFormat'" );
}

// Create a replacement map
if ($bHasPositional) {
// For positional, we need to handle the exact positions
$aReplacements = array_fill(0, $iMaxPosition, null);

// Fill in the real arguments first
foreach ($aArgs as $index => $sValue) {
if ($index < $iMaxPosition) {
$aReplacements[$index] = $sValue;
}
}

// For null values in the replacement map, use the original specifier
foreach ($aSpecifierMatches as $index => $sMatch) {
$iPosition = $aPositions[$index];
if ($aReplacements[$iPosition] === null) {
// Use the original format specifier when we don't have an argument
$aReplacements[$iPosition] = $sMatch[0];
}
}

// Remove any remaining nulls (for positions that weren't referenced)
$aReplacements = array_filter($aReplacements, static function($val) { return $val !== null; });
} else {
// For non-positional, we need to map each position
$aReplacements = [];
$iUsed = 0;

// Create a map of what values to use for each position
$aPositionValues = [];
for ($i = 0; $i < $iMaxPosition; $i++) {
if (isset($aUniquePositions[$i])) {
if ($iUsed < $iActualCount) {
// We have an actual argument for this position
$aPositionValues[$i] = $aArgs[$iUsed++];
} else {
// Mark this position to use the original specifier
$aPositionValues[$i] = null;
}
}
}

// Build the replacements array preserving the original order
foreach ($aSpecifierMatches as $index => $sMatch) {
$iPosition = $aPositions[$index];
if (isset($aPositionValues[$iPosition])) {
$aReplacements[] = $aPositionValues[$iPosition];
} else {
// Use the original format specifier when we don't have an argument
$aReplacements[] = $sMatch[0];
// Mark this position as used, so if it appears again, it gets the same replacement
$aPositionValues[$iPosition] = $sMatch[0];
}
}
}

// Process the format string with our filled-in arguments
return vsprintf($sFormat, $aReplacements);
}

/**
* Convert a string containing some (valid) HTML markup to plain text
*
Expand Down
2 changes: 1 addition & 1 deletion core/dict.class.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ public static function Format($sFormatCode /*, ... arguments ... */)
}

try{
return vsprintf($sLocalizedFormat, $aArguments);
return utils::VSprintf($sLocalizedFormat, $aArguments);
} catch(\Throwable $e){
\IssueLog::Error("Cannot format dict key", null, ["sFormatCode" => $sFormatCode, "sLangCode" => $sLangCode, 'exception_msg' => $e->getMessage() ]);
return $sFormatCode.' - '.implode(', ', $aArguments);
Expand Down
2 changes: 1 addition & 1 deletion core/modelreflection.class.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public function DictFormat($sFormatCode /*, ... arguments ....*/)
return $sFormatCode.' - '.implode(', ', $aArguments);
}

return vsprintf($sLocalizedFormat, $aArguments);
return utils::VSprintf($sLocalizedFormat, $aArguments);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion core/valuesetdef.class.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ protected function LoadValuesForAutocomplete($aArgs, $sContains = '', $sOperatio
foreach ($aAdditionalField as $sAdditionalField) {
array_push($aArguments, $oObject->Get($sAdditionalField));
}
$aData['additional_field'] = vsprintf($sFormatAdditionalField, $aArguments);
$aData['additional_field'] = utils::VSprintf($sFormatAdditionalField, $aArguments);
} else {
$aData['additional_field'] = '';
}
Expand Down
2 changes: 1 addition & 1 deletion sources/Service/Base/ObjectRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public static function ComputeOthersData(DBObject $oDbObject, string $sClass, ar
foreach ($aComplementAttributeSpec[1] as $sAdditionalField) {
$aArguments[] = $oDbObject->Get($sAdditionalField);
}
$aData['additional_field'] = vsprintf($aComplementAttributeSpec[0], $aArguments);
$aData['additional_field'] = utils::VSprintf($aComplementAttributeSpec[0], $aArguments);
$sAdditionalFieldForHtml = utils::EscapeHtml($aData['additional_field']);
$aData['full_description'] = "{$sFriendlyNameForHtml}<br><i><small>{$sAdditionalFieldForHtml}</small></i>";
} else {
Expand Down
90 changes: 90 additions & 0 deletions tests/php-unit-tests/unitary-tests/application/utilsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -907,4 +907,94 @@ public function testFileGetContentsAndMIMETypeOnRemoteURL()
$this->assertFalse(utils::GetDocumentFromSelfURL($sURL));
}

/**
* @dataProvider VSprintfProvider
*/
public function testVSprintf($sFormat, $aArgs, $sExpected)
{
$sTested = utils::VSprintf($sFormat, $aArgs, false);
$this->assertEquals($sExpected, $sTested);
}

public function VSprintfProvider()
{
return [
// Basic positional specifier tests
'Basic positional with enough args' => [
'Format: %1$s, %2$d, %3$s',
['Hello', 42, 'World'],
'Format: Hello, 42, World'
],
'Basic positional with args in different order' => [
'Format: %2$s, %1$d, %3$s',
[42, 'Hello', 'World'],
'Format: Hello, 42, World'
],
'Positional with reused specifiers' => [
'Format: %1$s, %2$d, %1$s again',
['Hello', 42],
'Format: Hello, 42, Hello again'
],

// Missing arguments tests
'Missing one positional arg' => [
'Format: %1$s, %2$d, %3$s',
['Hello', 42],
'Format: Hello, 42, %3$s'
],
'Missing multiple positional args' => [
'Format: %1$s, %2$s, %3$s, %4$s',
['Hello'],
'Format: Hello, %2$s, %3$s, %4$s'
],
'Missing first positional arg' => [
'Format: %1$s, %2$s, %3$s',
[],
'Format: %1$s, %2$s, %3$s'
],

// Edge cases
'Positional with larger numbers' => [
'Format: %2$s, %1$d, %3$s, %2$s again',
[123456, 'Hello', 'World'],
'Format: Hello, 123456, World, Hello again'
],
'Positional specifiers with non-sequential indexes' => [
'Format: %3$s then %1$s and %5$d',
['first', 'second', 'third', 'fourth', 42],
'Format: third then first and 42'
],

// More complex format specifiers
'Positional with format modifiers' => [
'Format: %1$\'*10s, %2$04d',
['Hello', 42],
'Format: *****Hello, 0042'
],
'Positional with various types' => [
'Format: String: %1$s, Integer: %2$d, Char: %3$c',
['Hello', 42, 65],
'Format: String: Hello, Integer: 42, Char: A'
],

// Testing with non-Latin characters
'Positional with UTF-8 characters' => [
'Format: %1$s %2$s %3$s',
['こんにちは', 'Здравствуйте', '你好'],
'Format: こんにちは Здравствуйте 你好'
],

// Mixed formats
'Mixed positional with complex specifiers' => [
'Format: %1$-10s | %2$+d',
['Hello', 42],
'Format: Hello | +42'
],
'Reused positional indexes with some missing' => [
'Format: %1$s %2$d %1$s %3$s %2$d',
['Hello', 42],
'Format: Hello 42 Hello %3$s 42'
]
];
}
}