revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-25 00:02:40 +01:00
Code Issues Releases Wiki Activity

Fix a false negative in lint for "xsprintf()"-family functions

Browse source 
Summary:
Ref T13577. This lint rule correctly detects the error in `pht('x %s y')` but the narrow test for `n_STRING_SCALAR` prevents it from detecting the error in `pht('x %s y'.'z')`.

Make the test broader.

Test Plan:
  - Ran `arc lint` on `HTTPSFuture.php`, got a detection of the issue in T13577.
  - Added a failing test and made it pass.

Maniphest Tasks: T13577

Differential Revision: https://secure.phabricator.com/D21453
This commit is contained in:
epriestley 2020-09-04 17:01:13 -07:00
parent ceb082ef6b
commit 73847a4b19
2 changed files with 11 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
src/lint/linter/xhpast/rules/ArcanistFormattedStringXHPASTLinterRule.php
View file

@ -82,7 +82,12 @@ final class ArcanistFormattedStringXHPASTLinterRule
}
$format = $parameters->getChildByIndex($start);
if ($format->getTypeName() != 'n_STRING_SCALAR') {
if (!$format->isConstantString()) {
// TODO: When this parameter is not a constant string, the call may
// be unsafe. We should make some attempt to warn about this for
// "qsprintf()" and other security-sensitive functions.
continue;
}

5
src/lint/linter/xhpast/rules/__tests__/formatted-string/formatted-string.lint-test
View file

@ -11,12 +11,17 @@ fprintf(null, 'x');
queryfx(null, 'x', 'y');
foobar(null, null, '%s');
pht('x %s y');
pht('x %s y'.'z');
~~~~~~~~~~
error:3:1:XHP54:Formatted String
error:7:1:XHP54:Formatted String
error:8:1:XHP54:Formatted String
error:11:1:XHP54:Formatted String
error:13:1:XHP54:Formatted String
error:15:1:XHP54:Formatted String
error:16:1:XHP54:Formatted String
~~~~~~~~~~
~~~~~~~~~~
{