From e8a0ebaeffaa0a7d5e8f34c7ce014f0092f36a6e Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 25 Sep 2015 11:16:04 -0700 Subject: [PATCH] Improve validation of 'name' and 'code' parameters for lint messages Summary: Ref T9145. Fixes T9316. We now require "name" and "code" has a maximum length (currently, this is 32, but the next diff will raise it to 128). Test Plan: - Installed PHPCS. - Hit both the "name" and "code" issues. - Applied this patch. - Got better errors sooner. Reviewers: chad Reviewed By: chad Subscribers: aik099 Maniphest Tasks: T9145, T9316 Differential Revision: https://secure.phabricator.com/D14165 --- src/lint/ArcanistLintMessage.php | 17 +++++++++++++++++ src/lint/engine/ArcanistLintEngine.php | 15 +++++++++++++++ src/lint/linter/ArcanistPhpcsLinter.php | 18 ++++++++++-------- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/lint/ArcanistLintMessage.php b/src/lint/ArcanistLintMessage.php index d4de43b8..29f9ab92 100644 --- a/src/lint/ArcanistLintMessage.php +++ b/src/lint/ArcanistLintMessage.php @@ -90,6 +90,23 @@ final class ArcanistLintMessage extends Phobject { } public function setCode($code) { + $code = (string)$code; + + $maximum_bytes = 128; + $actual_bytes = strlen($code); + + if ($actual_bytes > $maximum_bytes) { + throw new Exception( + pht( + 'Parameter ("%s") passed to "%s" when constructing a lint message '. + 'must be a scalar with a maximum string length of %s bytes, but is '. + '%s bytes in length.', + $code, + 'setCode()', + new PhutilNumber($maximum_bytes), + new PhutilNumber($actual_bytes))); + } + $this->code = $code; return $this; } diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php index 4a092fdb..0bef3e0a 100644 --- a/src/lint/engine/ArcanistLintEngine.php +++ b/src/lint/engine/ArcanistLintEngine.php @@ -217,6 +217,8 @@ abstract class ArcanistLintEngine extends Phobject { foreach ($runnable as $linter) { foreach ($linter->getLintMessages() as $message) { + $this->validateLintMessage($linter, $message); + if (!$this->isSeverityEnabled($message->getSeverity())) { continue; } @@ -598,5 +600,18 @@ abstract class ArcanistLintEngine extends Phobject { $this->endLintServiceCall($call_id); } + private function validateLintMessage( + ArcanistLinter $linter, + ArcanistLintMessage $message) { + + $name = $message->getName(); + if (!strlen($name)) { + throw new Exception( + pht( + 'Linter "%s" generated a lint message that is invalid because it '. + 'does not have a name. Lint messages must have a name.', + get_class($linter))); + } + } } diff --git a/src/lint/linter/ArcanistPhpcsLinter.php b/src/lint/linter/ArcanistPhpcsLinter.php index da3d118c..9556ea0a 100644 --- a/src/lint/linter/ArcanistPhpcsLinter.php +++ b/src/lint/linter/ArcanistPhpcsLinter.php @@ -109,15 +109,17 @@ final class ArcanistPhpcsLinter extends ArcanistExternalLinter { $prefix = 'W'; } - $code = 'PHPCS.'.$prefix.'.'.$child->getAttribute('source'); + $source = $child->getAttribute('source'); + $code = 'PHPCS.'.$prefix.'.'.$source; - $message = new ArcanistLintMessage(); - $message->setPath($path); - $message->setLine($child->getAttribute('line')); - $message->setChar($child->getAttribute('column')); - $message->setCode($code); - $message->setDescription($child->nodeValue); - $message->setSeverity($this->getLintMessageSeverity($code)); + $message = id(new ArcanistLintMessage()) + ->setPath($path) + ->setName($source) + ->setLine($child->getAttribute('line')) + ->setChar($child->getAttribute('column')) + ->setCode($code) + ->setDescription($child->nodeValue) + ->setSeverity($this->getLintMessageSeverity($code)); $messages[] = $message; }