Index: src/applications/diffusion/conduit/ConduitAPI_diffusion_rawdiffquery_Method.php =================================================================== --- src/applications/diffusion/conduit/ConduitAPI_diffusion_rawdiffquery_Method.php +++ src/applications/diffusion/conduit/ConduitAPI_diffusion_rawdiffquery_Method.php @@ -21,6 +21,7 @@ 'commit' => 'required string', 'path' => 'optional string', 'timeout' => 'optional int', + 'byteLimit' => 'optional int', 'linesOfContext' => 'optional int', 'againstCommit' => 'optional string', ); @@ -28,20 +29,29 @@ protected function getResult(ConduitAPIRequest $request) { $drequest = $this->getDiffusionRequest(); - $timeout = $request->getValue('timeout'); - $lines_of_context = $request->getValue('linesOfContext'); - $against_commit = $request->getValue('againstCommit'); $raw_query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest); + + $timeout = $request->getValue('timeout'); if ($timeout !== null) { $raw_query->setTimeout($timeout); } + + $lines_of_context = $request->getValue('linesOfContext'); if ($lines_of_context !== null) { $raw_query->setLinesOfContext($lines_of_context); } + + $against_commit = $request->getValue('againstCommit'); if ($against_commit !== null) { $raw_query->setAgainstCommit($against_commit); } + + $byte_limit = $request->getValue('byteLimit'); + if ($byte_limit !== null) { + $raw_query->setByteLimit($byte_limit); + } + return $raw_query->loadRawDiff(); } } Index: src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php =================================================================== --- src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php +++ src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php @@ -34,9 +34,7 @@ $commit, $path); - if ($this->getTimeout()) { - $future->setTimeout($this->getTimeout()); - } + $this->configureFuture($future); try { list($raw_diff) = $future->resolvex(); @@ -61,9 +59,7 @@ $commit, $drequest->getPath()); - if ($this->getTimeout()) { - $future->setTimeout($this->getTimeout()); - } + $this->configureFuture($future); list($raw_diff) = $future->resolvex(); } Index: src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php =================================================================== --- src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php +++ src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php @@ -6,7 +6,6 @@ return $this->executeRawDiffCommand(); } - protected function executeRawDiffCommand() { $drequest = $this->getRequest(); $repository = $drequest->getRepository(); @@ -31,9 +30,7 @@ $commit, $path); - if ($this->getTimeout()) { - $future->setTimeout($this->getTimeout()); - } + $this->configureFuture($future); list($raw_diff) = $future->resolvex(); Index: src/applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php =================================================================== --- src/applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php +++ src/applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php @@ -6,6 +6,7 @@ private $timeout; private $linesOfContext = 65535; private $againstCommit; + private $byteLimit; final public static function newFromDiffusionRequest( DiffusionRequest $request) { @@ -25,6 +26,15 @@ return $this->timeout; } + public function setByteLimit($byte_limit) { + $this->byteLimit = $byte_limit; + return $this; + } + + public function getByteLimit() { + return $this->byteLimit; + } + final public function setLinesOfContext($lines_of_context) { $this->linesOfContext = $lines_of_context; return $this; @@ -43,4 +53,15 @@ return $this->againstCommit; } + protected function configureFuture(ExecFuture $future) { + if ($this->getTimeout()) { + $future->setTimeout($this->getTimeout()); + } + + if ($this->getByteLimit()) { + $future->setStdoutSizeLimit($this->getByteLimit()); + $future->setStderrSizeLimit($this->getByteLimit()); + } + } + } Index: src/applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php =================================================================== --- src/applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php +++ src/applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php @@ -22,9 +22,7 @@ $commit, $repository->getSubversionPathURI($drequest->getPath())); - if ($this->getTimeout()) { - $future->setTimeout($this->getTimeout()); - } + $this->configureFuture($future); list($raw_diff) = $future->resolvex(); return $raw_diff; Index: src/applications/herald/adapter/HeraldAdapter.php =================================================================== --- src/applications/herald/adapter/HeraldAdapter.php +++ src/applications/herald/adapter/HeraldAdapter.php @@ -18,6 +18,7 @@ const FIELD_DIFF_CONTENT = 'diff-content'; const FIELD_DIFF_ADDED_CONTENT = 'diff-added-content'; const FIELD_DIFF_REMOVED_CONTENT = 'diff-removed-content'; + const FIELD_DIFF_ENORMOUS = 'diff-enormous'; const FIELD_REPOSITORY = 'repository'; const FIELD_REPOSITORY_PROJECTS = 'repository-projects'; const FIELD_RULE = 'rule'; @@ -193,6 +194,7 @@ self::FIELD_DIFF_CONTENT => pht('Any changed file content'), self::FIELD_DIFF_ADDED_CONTENT => pht('Any added file content'), self::FIELD_DIFF_REMOVED_CONTENT => pht('Any removed file content'), + self::FIELD_DIFF_ENORMOUS => pht('Change is enormous'), self::FIELD_REPOSITORY => pht('Repository'), self::FIELD_REPOSITORY_PROJECTS => pht('Repository\'s projects'), self::FIELD_RULE => pht('Another Herald rule'), @@ -342,6 +344,7 @@ self::CONDITION_NOT_EXISTS, ); case self::FIELD_IS_MERGE_COMMIT: + case self::FIELD_DIFF_ENORMOUS: return array( self::CONDITION_IS_TRUE, self::CONDITION_IS_FALSE, Index: src/applications/herald/adapter/HeraldCommitAdapter.php =================================================================== --- src/applications/herald/adapter/HeraldCommitAdapter.php +++ src/applications/herald/adapter/HeraldCommitAdapter.php @@ -99,6 +99,7 @@ self::FIELD_DIFF_CONTENT, self::FIELD_DIFF_ADDED_CONTENT, self::FIELD_DIFF_REMOVED_CONTENT, + self::FIELD_DIFF_ENORMOUS, self::FIELD_RULE, self::FIELD_AFFECTED_PACKAGE, self::FIELD_AFFECTED_PACKAGE_OWNER, @@ -277,14 +278,26 @@ 'commit' => $this->commit->getCommitIdentifier(), )); + $byte_limit = (1024 * 1024 * 1024); // 1GB + $raw = DiffusionQuery::callConduitWithDiffusionRequest( PhabricatorUser::getOmnipotentUser(), $drequest, 'diffusion.rawdiffquery', array( 'commit' => $this->commit->getCommitIdentifier(), - 'timeout' => 60 * 60 * 15, - 'linesOfContext' => 0)); + 'timeout' => (60 * 15), // 15 minutes + 'byteLimit' => $byte_limit, + 'linesOfContext' => 0, + )); + + if (strlen($raw) >= $byte_limit) { + throw new Exception( + pht( + 'The raw text of this change is enormous (larger than %d bytes). '. + 'Herald can not process it.', + $byte_limit)); + } $parser = new ArcanistDiffParser(); $changes = $parser->parseDiff($raw); @@ -360,6 +373,9 @@ return $this->getDiffContent('+'); case self::FIELD_DIFF_REMOVED_CONTENT: return $this->getDiffContent('-'); + case self::FIELD_DIFF_ENORMOUS: + $this->getDiffContent('*'); + return ($this->commitDiff instanceof Exception); case self::FIELD_AFFECTED_PACKAGE: $packages = $this->loadAffectedPackages(); return mpull($packages, 'getPHID'); Index: src/applications/herald/storage/HeraldRule.php =================================================================== --- src/applications/herald/storage/HeraldRule.php +++ src/applications/herald/storage/HeraldRule.php @@ -17,7 +17,7 @@ protected $isDisabled = 0; protected $triggerObjectPHID; - protected $configVersion = 24; + protected $configVersion = 25; // phids for which this rule has been applied private $ruleApplied = self::ATTACHABLE;