Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14767760
D10305.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
17 KB
Referenced Files
None
Subscribers
None
D10305.diff
View Options
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -247,6 +247,8 @@
'DifferentialDependsOnField' => 'applications/differential/customfield/DifferentialDependsOnField.php',
'DifferentialDiff' => 'applications/differential/storage/DifferentialDiff.php',
'DifferentialDiffCreateController' => 'applications/differential/controller/DifferentialDiffCreateController.php',
+ 'DifferentialDiffCreationRejectException' => 'applications/differential/exception/DifferentialDiffCreationRejectException.php',
+ 'DifferentialDiffEditor' => 'applications/differential/editor/DifferentialDiffEditor.php',
'DifferentialDiffPHIDType' => 'applications/differential/phid/DifferentialDiffPHIDType.php',
'DifferentialDiffProperty' => 'applications/differential/storage/DifferentialDiffProperty.php',
'DifferentialDiffQuery' => 'applications/differential/query/DifferentialDiffQuery.php',
@@ -748,6 +750,8 @@
'HeraldController' => 'applications/herald/controller/HeraldController.php',
'HeraldCustomAction' => 'applications/herald/extension/HeraldCustomAction.php',
'HeraldDAO' => 'applications/herald/storage/HeraldDAO.php',
+ 'HeraldDifferentialAdapter' => 'applications/herald/adapter/HeraldDifferentialAdapter.php',
+ 'HeraldDifferentialDiffAdapter' => 'applications/herald/adapter/HeraldDifferentialDiffAdapter.php',
'HeraldDifferentialRevisionAdapter' => 'applications/herald/adapter/HeraldDifferentialRevisionAdapter.php',
'HeraldDisableController' => 'applications/herald/controller/HeraldDisableController.php',
'HeraldEditLogQuery' => 'applications/herald/query/HeraldEditLogQuery.php',
@@ -2998,6 +3002,8 @@
'PhabricatorDestructibleInterface',
),
'DifferentialDiffCreateController' => 'DifferentialController',
+ 'DifferentialDiffCreationRejectException' => 'Exception',
+ 'DifferentialDiffEditor' => 'PhabricatorEditor',
'DifferentialDiffPHIDType' => 'PhabricatorPHIDType',
'DifferentialDiffProperty' => 'DifferentialDAO',
'DifferentialDiffQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
@@ -3529,7 +3535,9 @@
'HeraldCondition' => 'HeraldDAO',
'HeraldController' => 'PhabricatorController',
'HeraldDAO' => 'PhabricatorLiskDAO',
- 'HeraldDifferentialRevisionAdapter' => 'HeraldAdapter',
+ 'HeraldDifferentialAdapter' => 'HeraldAdapter',
+ 'HeraldDifferentialDiffAdapter' => 'HeraldDifferentialAdapter',
+ 'HeraldDifferentialRevisionAdapter' => 'HeraldDifferentialAdapter',
'HeraldDisableController' => 'HeraldController',
'HeraldEditLogQuery' => 'PhabricatorOffsetPagedQuery',
'HeraldInvalidActionException' => 'Exception',
diff --git a/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php
--- a/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php
@@ -162,7 +162,11 @@
break;
}
- $diff->save();
+ id(new DifferentialDiffEditor())
+ ->setActor($viewer)
+ ->setContentSource(
+ PhabricatorContentSource::newFromConduitRequest($request))
+ ->saveDiff($diff);
// If we didn't get an explicit `repositoryPHID` (which means the client is
// old, or couldn't figure out which repository the working copy belongs
diff --git a/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php
--- a/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php
@@ -59,7 +59,11 @@
$diff->setRepositoryPHID($repository->getPHID());
}
- $diff->save();
+ id(new DifferentialDiffEditor())
+ ->setActor($viewer)
+ ->setContentSource(
+ PhabricatorContentSource::newFromConduitRequest($request))
+ ->saveDiff($diff);
return $this->buildDiffInfoDictionary($diff);
}
diff --git a/src/applications/differential/editor/DifferentialDiffEditor.php b/src/applications/differential/editor/DifferentialDiffEditor.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/editor/DifferentialDiffEditor.php
@@ -0,0 +1,80 @@
+<?php
+
+final class DifferentialDiffEditor extends PhabricatorEditor {
+
+ private $contentSource;
+
+ public function setContentSource($content_source) {
+ $this->contentSource = $content_source;
+ return $this;
+ }
+
+ public function getContentSource() {
+ return $this->contentSource;
+ }
+
+ public function saveDiff(DifferentialDiff $diff) {
+ $actor = $this->requireActor();
+
+ // Generate a PHID first, so the transcript will point at the object if
+ // we deicde to preserve it.
+ $phid = $diff->generatePHID();
+ $diff->setPHID($phid);
+
+ $adapter = id(new HeraldDifferentialDiffAdapter())
+ ->setDiff($diff);
+
+ $adapter->setContentSource($this->getContentSource());
+ $adapter->setIsNewObject(true);
+
+ $engine = new HeraldEngine();
+
+ $rules = $engine->loadRulesForAdapter($adapter);
+ $rules = mpull($rules, null, 'getID');
+
+ $effects = $engine->applyRules($rules, $adapter);
+
+ $blocking_effect = null;
+ foreach ($effects as $effect) {
+ if ($effect->getAction() == HeraldAdapter::ACTION_BLOCK) {
+ $blocking_effect = $effect;
+ break;
+ }
+ }
+
+ if ($blocking_effect) {
+ $rule = idx($rules, $effect->getRuleID());
+ if ($rule && strlen($rule->getName())) {
+ $rule_name = $rule->getName();
+ } else {
+ $rule_name = pht('Unnamed Herald Rule');
+ }
+
+ $message = $effect->getTarget();
+ if (!strlen($message)) {
+ $message = pht('(None.)');
+ }
+
+ throw new DifferentialDiffCreationRejectException(
+ pht(
+ "Creation of this diff was rejected by Herald rule %s.\n".
+ " Rule: %s\n".
+ "Reason: %s",
+ 'H'.$effect->getRuleID(),
+ $rule_name,
+ $message));
+ }
+
+ $diff->save();
+
+ // NOTE: We only save the transcript if we didn't block the diff.
+ // Otherwise, we might save some of the diff's content in the transcript
+ // table, which would defeat the purpose of allowing rules to block
+ // storage of key material.
+
+ $engine->applyEffects($effects, $adapter, $rules);
+ $xscript = $engine->getTranscript();
+
+ }
+
+}
diff --git a/src/applications/differential/exception/DifferentialDiffCreationRejectException.php b/src/applications/differential/exception/DifferentialDiffCreationRejectException.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/exception/DifferentialDiffCreationRejectException.php
@@ -0,0 +1,5 @@
+<?php
+
+final class DifferentialDiffCreationRejectException extends Exception {
+
+}
diff --git a/src/applications/herald/adapter/HeraldDifferentialAdapter.php b/src/applications/herald/adapter/HeraldDifferentialAdapter.php
new file mode 100644
--- /dev/null
+++ b/src/applications/herald/adapter/HeraldDifferentialAdapter.php
@@ -0,0 +1,93 @@
+<?php
+
+abstract class HeraldDifferentialAdapter extends HeraldAdapter {
+
+ private $repository = false;
+ private $diff;
+
+ abstract protected function loadChangesets();
+ abstract protected function loadChangesetsWithHunks();
+
+ public function getDiff() {
+ return $this->diff;
+ }
+
+ public function setDiff(DifferentialDiff $diff) {
+ $this->diff = $diff;
+ return $this;
+ }
+
+ public function loadRepository() {
+ if ($this->repository === false) {
+ $repository_phid = $this->getObject()->getRepositoryPHID();
+ if ($repository_phid) {
+ $repository = id(new PhabricatorRepositoryQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withPHIDs(array($repository_phid))
+ ->needProjectPHIDs(true)
+ ->executeOne();
+ }
+ $this->repository = false;
+ }
+
+ return $this->repository;
+ }
+
+
+ protected function loadAffectedPaths() {
+ $changesets = $this->loadChangesets();
+
+ $paths = array();
+ foreach ($changesets as $changeset) {
+ $paths[] = $this->getAbsoluteRepositoryPathForChangeset($changeset);
+ }
+
+ return $paths;
+ }
+
+ protected function getAbsoluteRepositoryPathForChangeset(
+ DifferentialChangeset $changeset) {
+
+ $repository = $this->loadRepository();
+ if (!$repository) {
+ return '/'.ltrim($changeset->getFilename(), '/');
+ }
+
+ $diff = $this->getDiff();
+
+ return $changeset->getAbsoluteRepositoryPath($repository, $diff);
+ }
+
+ protected function loadContentDictionary() {
+ $add_lines = DifferentialHunk::FLAG_LINES_ADDED;
+ $rem_lines = DifferentialHunk::FLAG_LINES_REMOVED;
+ $mask = ($add_lines | $rem_lines);
+ return $this->loadContentWithMask($mask);
+ }
+
+ protected function loadAddedContentDictionary() {
+ return $this->loadContentWithMask(DifferentialHunk::FLAG_LINES_ADDED);
+ }
+
+ protected function loadRemovedContentDictionary() {
+ return $this->loadContentWithMask(DifferentialHunk::FLAG_LINES_REMOVED);
+ }
+
+ protected function loadContentWithMask($mask) {
+ $changesets = $this->loadChangesetsWithHunks();
+
+ $dict = array();
+ foreach ($changesets as $changeset) {
+ $content = array();
+ foreach ($changeset->getHunks() as $hunk) {
+ $content[] = $hunk->getContentWithMask($mask);
+ }
+
+ $path = $this->getAbsoluteRepositoryPathForChangeset($changeset);
+ $dict[$path] = implode("\n", $content);
+ }
+
+ return $dict;
+ }
+
+}
diff --git a/src/applications/herald/adapter/HeraldDifferentialDiffAdapter.php b/src/applications/herald/adapter/HeraldDifferentialDiffAdapter.php
new file mode 100644
--- /dev/null
+++ b/src/applications/herald/adapter/HeraldDifferentialDiffAdapter.php
@@ -0,0 +1,164 @@
+<?php
+
+final class HeraldDifferentialDiffAdapter extends HeraldDifferentialAdapter {
+
+ public function getAdapterApplicationClass() {
+ return 'PhabricatorDifferentialApplication';
+ }
+
+ protected function loadChangesets() {
+ return $this->loadChangesetsWithHunks();
+ }
+
+ protected function loadChangesetsWithHunks() {
+ return $this->getDiff()->getChangesets();
+ }
+
+ public function getObject() {
+ return $this->getDiff();
+ }
+
+ public function getAdapterContentType() {
+ return 'differential.diff';
+ }
+
+ public function getAdapterContentName() {
+ return pht('Differential Diffs');
+ }
+
+ public function getAdapterContentDescription() {
+ return pht(
+ "React to new diffs being uploaded, before writes occur.\n".
+ "These rules can reject diffs before they are written to permanent ".
+ "storage, to prevent users from accidentally uploading private keys or ".
+ "other sensitive information.");
+ }
+
+ public function supportsRuleType($rule_type) {
+ switch ($rule_type) {
+ case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
+ return true;
+ case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
+ case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
+ default:
+ return false;
+ }
+ }
+
+ public function getFields() {
+ return array_merge(
+ array(
+ self::FIELD_AUTHOR,
+ self::FIELD_AUTHOR_PROJECTS,
+ self::FIELD_REPOSITORY,
+ self::FIELD_REPOSITORY_PROJECTS,
+ self::FIELD_DIFF_FILE,
+ self::FIELD_DIFF_CONTENT,
+ self::FIELD_DIFF_ADDED_CONTENT,
+ self::FIELD_DIFF_REMOVED_CONTENT,
+ ),
+ parent::getFields());
+ }
+
+ public function getRepetitionOptions() {
+ return array(
+ HeraldRepetitionPolicyConfig::FIRST,
+ );
+ }
+
+ public function getPHID() {
+ return $this->getObject()->getPHID();
+ }
+
+ public function getHeraldName() {
+ return pht('New Diff');
+ }
+
+ public function getActionNameMap($rule_type) {
+ return array(
+ self::ACTION_BLOCK => pht('Block diff with message'),
+ );
+ }
+
+ public function getHeraldField($field) {
+ switch ($field) {
+ case self::FIELD_AUTHOR:
+ return $this->getObject()->getAuthorPHID();
+ break;
+ case self::FIELD_AUTHOR_PROJECTS:
+ $author_phid = $this->getHeraldField(self::FIELD_AUTHOR);
+ if (!$author_phid) {
+ return array();
+ }
+
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withMemberPHIDs(array($author_phid))
+ ->execute();
+
+ return mpull($projects, 'getPHID');
+ case self::FIELD_DIFF_FILE:
+ return $this->loadAffectedPaths();
+ case self::FIELD_REPOSITORY:
+ $repository = $this->loadRepository();
+ if (!$repository) {
+ return null;
+ }
+ return $repository->getPHID();
+ case self::FIELD_REPOSITORY_PROJECTS:
+ $repository = $this->loadRepository();
+ if (!$repository) {
+ return array();
+ }
+ return $repository->getProjectPHIDs();
+ case self::FIELD_DIFF_CONTENT:
+ return $this->loadContentDictionary();
+ case self::FIELD_DIFF_ADDED_CONTENT:
+ return $this->loadAddedContentDictionary();
+ case self::FIELD_DIFF_REMOVED_CONTENT:
+ return $this->loadRemovedContentDictionary();
+ }
+
+ return parent::getHeraldField($field);
+ }
+
+ public function getActions($rule_type) {
+ switch ($rule_type) {
+ case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
+ return array_merge(
+ array(
+ self::ACTION_BLOCK,
+ self::ACTION_NOTHING,
+ ),
+ parent::getActions($rule_type));
+ }
+ }
+
+ public function applyHeraldEffects(array $effects) {
+ assert_instances_of($effects, 'HeraldEffect');
+
+ $result = array();
+ foreach ($effects as $effect) {
+ $action = $effect->getAction();
+ switch ($action) {
+ case self::ACTION_NOTHING:
+ $result[] = new HeraldApplyTranscript(
+ $effect,
+ true,
+ pht('Did nothing.'));
+ break;
+ case self::ACTION_BLOCK:
+ $result[] = new HeraldApplyTranscript(
+ $effect,
+ true,
+ pht('Blocked diff.'));
+ break;
+ default:
+ throw new Exception(pht('No rules to handle action "%s"!', $action));
+ }
+ }
+
+ return $result;
+ }
+
+}
diff --git a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php
--- a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php
+++ b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php
@@ -1,9 +1,9 @@
<?php
-final class HeraldDifferentialRevisionAdapter extends HeraldAdapter {
+final class HeraldDifferentialRevisionAdapter
+ extends HeraldDifferentialAdapter {
protected $revision;
- protected $diff;
protected $explicitCCs;
protected $explicitReviewers;
@@ -17,7 +17,6 @@
protected $buildPlans = array();
protected $requiredSignatureDocumentPHIDs = array();
- protected $repository;
protected $affectedPackages;
protected $changesets;
private $haveHunks;
@@ -160,25 +159,6 @@
return $this->revision->getTitle();
}
- public function loadRepository() {
- if ($this->repository === null) {
- $this->repository = false;
- $repository_phid = $this->getObject()->getRepositoryPHID();
- if ($repository_phid) {
- $repository = id(new PhabricatorRepositoryQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withPHIDs(array($repository_phid))
- ->needProjectPHIDs(true)
- ->executeOne();
- if ($repository) {
- $this->repository = $repository;
- }
- }
- }
-
- return $this->repository;
- }
-
protected function loadChangesets() {
if ($this->changesets === null) {
$this->changesets = $this->diff->loadChangesets();
@@ -186,7 +166,7 @@
return $this->changesets;
}
- private function loadChangesetsWithHunks() {
+ protected function loadChangesetsWithHunks() {
$changesets = $this->loadChangesets();
if ($changesets && !$this->haveHunks) {
@@ -202,61 +182,6 @@
return $changesets;
}
- protected function loadAffectedPaths() {
- $changesets = $this->loadChangesets();
-
- $paths = array();
- foreach ($changesets as $changeset) {
- $paths[] = $this->getAbsoluteRepositoryPathForChangeset($changeset);
- }
- return $paths;
- }
-
- protected function getAbsoluteRepositoryPathForChangeset(
- DifferentialChangeset $changeset) {
-
- $repository = $this->loadRepository();
- if (!$repository) {
- return '/'.ltrim($changeset->getFilename(), '/');
- }
-
- $diff = $this->diff;
-
- return $changeset->getAbsoluteRepositoryPath($repository, $diff);
- }
-
- protected function loadContentDictionary() {
- $add_lines = DifferentialHunk::FLAG_LINES_ADDED;
- $rem_lines = DifferentialHunk::FLAG_LINES_REMOVED;
- $mask = ($add_lines | $rem_lines);
- return $this->loadContentWithMask($mask);
- }
-
- protected function loadAddedContentDictionary() {
- return $this->loadContentWithMask(DifferentialHunk::FLAG_LINES_ADDED);
- }
-
- protected function loadRemovedContentDictionary() {
- return $this->loadContentWithMask(DifferentialHunk::FLAG_LINES_REMOVED);
- }
-
- private function loadContentWithMask($mask) {
- $changesets = $this->loadChangesetsWithHunks();
-
- $dict = array();
- foreach ($changesets as $changeset) {
- $content = array();
- foreach ($changeset->getHunks() as $hunk) {
- $content[] = $hunk->getContentWithMask($mask);
- }
-
- $path = $this->getAbsoluteRepositoryPathForChangeset($changeset);
- $dict[$path] = implode("\n", $content);
- }
-
- return $dict;
- }
-
public function loadAffectedPackages() {
if ($this->affectedPackages === null) {
$this->affectedPackages = array();
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Fri, Jan 24, 6:15 PM (20 h, 47 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7042391
Default Alt Text
D10305.diff (17 KB)
Attached To
Mode
D10305: Allow Herald "diff" rules to reject content before it is written
Attached
Detach File
Event Timeline
Log In to Comment