Page MenuHomePhabricator

D7885.id17838.diff
No OneTemporary

D7885.id17838.diff

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_RULE = 'rule';
const FIELD_AFFECTED_PACKAGE = 'affected-package';
@@ -192,6 +193,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_RULE => pht('Another Herald rule'),
self::FIELD_AFFECTED_PACKAGE => pht('Any affected package'),
@@ -339,6 +341,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 = 23;
+ protected $configVersion = 25;
// phids for which this rule has been applied
private $ruleApplied = self::ATTACHABLE;

File Metadata

Mime Type
text/plain
Expires
Tue, Oct 22, 11:54 PM (20 h, 41 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6743102
Default Alt Text
D7885.id17838.diff (8 KB)

Event Timeline