Page MenuHomePhabricator
Files F15401202

D19839.id47392.diff
No OneTemporary
Actions

D19839.id47392.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
@@ -4203,6 +4203,7 @@
'PhabricatorRepositorySyncEventPHIDType' => 'applications/repository/phid/PhabricatorRepositorySyncEventPHIDType.php',
'PhabricatorRepositorySyncEventQuery' => 'applications/repository/query/PhabricatorRepositorySyncEventQuery.php',
'PhabricatorRepositoryTestCase' => 'applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php',
+ 'PhabricatorRepositoryTouchLimitTransaction' => 'applications/repository/xaction/PhabricatorRepositoryTouchLimitTransaction.php',
'PhabricatorRepositoryTrackOnlyTransaction' => 'applications/repository/xaction/PhabricatorRepositoryTrackOnlyTransaction.php',
'PhabricatorRepositoryTransaction' => 'applications/repository/storage/PhabricatorRepositoryTransaction.php',
'PhabricatorRepositoryTransactionQuery' => 'applications/repository/query/PhabricatorRepositoryTransactionQuery.php',
@@ -10198,6 +10199,7 @@
'PhabricatorRepositorySyncEventPHIDType' => 'PhabricatorPHIDType',
'PhabricatorRepositorySyncEventQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorRepositoryTestCase' => 'PhabricatorTestCase',
+ 'PhabricatorRepositoryTouchLimitTransaction' => 'PhabricatorRepositoryTransactionType',
'PhabricatorRepositoryTrackOnlyTransaction' => 'PhabricatorRepositoryTransactionType',
'PhabricatorRepositoryTransaction' => 'PhabricatorModularTransaction',
'PhabricatorRepositoryTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
diff --git a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php
--- a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php
+++ b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php
@@ -491,6 +491,15 @@
->setConduitDescription(pht('Change the copy time limit.'))
->setConduitTypeDescription(pht('New repository copy time limit.'))
->setValue($object->getCopyTimeLimit()),
+ id(new PhabricatorTextEditField())
+ ->setKey('touchLimit')
+ ->setLabel(pht('Touched Paths Limit'))
+ ->setTransactionType(
+ PhabricatorRepositoryTouchLimitTransaction::TRANSACTIONTYPE)
+ ->setDescription(pht('Maximum permitted paths touched per commit.'))
+ ->setConduitDescription(pht('Change the touch limit.'))
+ ->setConduitTypeDescription(pht('New repository touch limit.'))
+ ->setValue($object->getTouchLimit()),
);
}
diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
--- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
+++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
@@ -39,6 +39,7 @@
private $emailPHIDs = array();
private $changesets = array();
private $changesetsSize = 0;
+ private $filesizeCache = array();
/* -( Config )------------------------------------------------------------- */
@@ -174,6 +175,15 @@
throw $ex;
}
+ try {
+ if (!$is_initial_import) {
+ $this->rejectCommitsAffectingTooManyPaths($content_updates);
+ }
+ } catch (DiffusionCommitHookRejectException $ex) {
+ $this->rejectCode = PhabricatorRepositoryPushLog::REJECT_TOUCHES;
+ throw $ex;
+ }
+
try {
if (!$is_initial_import) {
$this->rejectEnormousChanges($content_updates);
@@ -1276,7 +1286,8 @@
foreach ($content_updates as $update) {
$identifier = $update->getRefNew();
- $sizes = $this->loadFileSizesForCommit($identifier);
+ $sizes = $this->getFileSizesForCommit($identifier);
+
foreach ($sizes as $path => $size) {
if ($size <= $limit) {
continue;
@@ -1301,7 +1312,47 @@
}
}
- public function loadFileSizesForCommit($identifier) {
+ private function rejectCommitsAffectingTooManyPaths(array $content_updates) {
+ $repository = $this->getRepository();
+
+ $limit = $repository->getTouchLimit();
+ if (!$limit) {
+ return;
+ }
+
+ foreach ($content_updates as $update) {
+ $identifier = $update->getRefNew();
+
+ $sizes = $this->getFileSizesForCommit($identifier);
+ if (count($sizes) > $limit) {
+ $message = pht(
+ 'COMMIT AFFECTS TOO MANY PATHS'.
+ "\n".
+ 'This repository ("%s") is configured with a touched files limit '.
+ 'that caps the maximum number of paths any single commit may '.
+ 'affect. You are pushing a change ("%s") which exceeds this '.
+ 'limit: it affects %s paths, but the largest number of paths any '.
+ 'commit may affect is %s paths.',
+ $repository->getDisplayName(),
+ $identifier,
+ phutil_count($sizes),
+ new PhutilNumber($limit));
+
+ throw new DiffusionCommitHookRejectException($message);
+ }
+ }
+ }
+
+ public function getFileSizesForCommit($identifier) {
+ if (!isset($this->filesizeCache[$identifier])) {
+ $file_sizes = $this->loadFileSizesForCommit($identifier);
+ $this->filesizeCache[$identifier] = $file_sizes;
+ }
+
+ return $this->filesizeCache[$identifier];
+ }
+
+ private function loadFileSizesForCommit($identifier) {
$repository = $this->getRepository();
return id(new DiffusionLowLevelFilesizeQuery())
diff --git a/src/applications/diffusion/management/DiffusionRepositoryLimitsManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositoryLimitsManagementPanel.php
--- a/src/applications/diffusion/management/DiffusionRepositoryLimitsManagementPanel.php
+++ b/src/applications/diffusion/management/DiffusionRepositoryLimitsManagementPanel.php
@@ -38,6 +38,7 @@
return array(
'filesizeLimit',
'copyTimeLimit',
+ 'touchLimit',
);
}
@@ -95,6 +96,16 @@
$view->addProperty(pht('Clone/Fetch Timeout'), $copy_display);
+ $touch_limit = $repository->getTouchLimit();
+ if ($touch_limit) {
+ $touch_display = pht('%s Paths', new PhutilNumber($touch_limit));
+ } else {
+ $touch_display = pht('Unlimited');
+ $touch_display = phutil_tag('em', array(), $touch_display);
+ }
+
+ $view->addProperty(pht('Touched Paths Limit'), $touch_display);
+
return $this->newBox(pht('Limits'), $view);
}
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -1926,6 +1926,14 @@
return $this->setDetail('limit.filesize', $limit);
}
+ public function getTouchLimit() {
+ return $this->getDetail('limit.touch');
+ }
+
+ public function setTouchLimit($limit) {
+ return $this->setDetail('limit.touch', $limit);
+ }
+
/**
* Retrieve the service URI for the device hosting this repository.
*
diff --git a/src/applications/repository/storage/PhabricatorRepositoryPushLog.php b/src/applications/repository/storage/PhabricatorRepositoryPushLog.php
--- a/src/applications/repository/storage/PhabricatorRepositoryPushLog.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryPushLog.php
@@ -25,6 +25,7 @@
const CHANGEFLAG_DANGEROUS = 16;
const CHANGEFLAG_ENORMOUS = 32;
const CHANGEFLAG_OVERSIZED = 64;
+ const CHANGEFLAG_TOUCHES = 128;
const REJECT_ACCEPT = 0;
const REJECT_DANGEROUS = 1;
@@ -33,6 +34,7 @@
const REJECT_BROKEN = 4;
const REJECT_ENORMOUS = 5;
const REJECT_OVERSIZED = 6;
+ const REJECT_TOUCHES = 7;
protected $repositoryPHID;
protected $epoch;
@@ -66,6 +68,7 @@
self::CHANGEFLAG_DANGEROUS => pht('Dangerous'),
self::CHANGEFLAG_ENORMOUS => pht('Enormous'),
self::CHANGEFLAG_OVERSIZED => pht('Oversized'),
+ self::CHANGEFLAG_TOUCHES => pht('Touches Too Many Paths'),
);
}
@@ -78,6 +81,7 @@
self::REJECT_BROKEN => pht('Rejected: Broken'),
self::REJECT_ENORMOUS => pht('Rejected: Enormous'),
self::REJECT_OVERSIZED => pht('Rejected: Oversized File'),
+ self::REJECT_TOUCHES => pht('Rejected: Touches Too Many Paths'),
);
}
diff --git a/src/applications/repository/xaction/PhabricatorRepositoryTouchLimitTransaction.php b/src/applications/repository/xaction/PhabricatorRepositoryTouchLimitTransaction.php
new file mode 100644
--- /dev/null
+++ b/src/applications/repository/xaction/PhabricatorRepositoryTouchLimitTransaction.php
@@ -0,0 +1,76 @@
+<?php
+
+final class PhabricatorRepositoryTouchLimitTransaction
+ extends PhabricatorRepositoryTransactionType {
+
+ const TRANSACTIONTYPE = 'limit.touch';
+
+ public function generateOldValue($object) {
+ return $object->getTouchLimit();
+ }
+
+ public function generateNewValue($object, $value) {
+ if (!strlen($value)) {
+ return null;
+ }
+
+ $value = (int)$value;
+ if (!$value) {
+ return null;
+ }
+
+ return $value;
+ }
+
+ public function applyInternalEffects($object, $value) {
+ $object->setTouchLimit($value);
+ }
+
+ public function getTitle() {
+ $old = $this->getOldValue();
+ $new = $this->getNewValue();
+
+ if ($old && $new) {
+ return pht(
+ '%s changed the touch limit for this repository from %s paths to '.
+ '%s paths.',
+ $this->renderAuthor(),
+ $this->renderOldValue(),
+ $this->renderNewValue());
+ } else if ($new) {
+ return pht(
+ '%s set the touch limit for this repository to %s paths.',
+ $this->renderAuthor(),
+ $this->renderNewValue());
+ } else {
+ return pht(
+ '%s removed the touch limit (%s paths) for this repository.',
+ $this->renderAuthor(),
+ $this->renderOldValue());
+ }
+ }
+
+ public function validateTransactions($object, array $xactions) {
+ $errors = array();
+
+ foreach ($xactions as $xaction) {
+ $new = $xaction->getNewValue();
+
+ if (!strlen($new)) {
+ continue;
+ }
+
+ if (!preg_match('/^\d+\z/', $new)) {
+ $errors[] = $this->newInvalidError(
+ pht(
+ 'Unable to parse touch limit, specify a positive number of '.
+ 'paths.'),
+ $xaction);
+ continue;
+ }
+ }
+
+ return $errors;
+ }
+
+}
diff --git a/src/docs/user/userguide/diffusion_managing.diviner b/src/docs/user/userguide/diffusion_managing.diviner
--- a/src/docs/user/userguide/diffusion_managing.diviner
+++ b/src/docs/user/userguide/diffusion_managing.diviner
@@ -246,11 +246,38 @@
it becomes too large) it will be rejected. This option only applies to hosted
repositories.
+This limit is primarily intended to make it more difficult to accidentally push
+very large files that shouldn't be version controlled (like logs, binaries,
+machine learning data, or media assets). Pushing huge datafiles by mistake can
+make the repository unwieldy by dramatically increasing how much data must be
+transferred over the network to clone it, and simply reverting the changes
+doesn't reduce the impact of this kind of mistake.
+
**Clone/Fetch Timeout**: Configure the internal timeout for creating copies
of this repository during operations like intracluster synchronization and
Drydock working copy construction. This timeout does not affect external
users.
+**Touch Limit**: Apply a limit to the maximum number of paths that any commit
+may touch. If a commit affects more paths than this limit, it will be rejected.
+This option only applies to hosted repositories. Users may work around this
+limit by breaking the commit into several smaller commits which each affect
+fewer paths.
+
+This limit is intended to offer a guard rail against users making silly
+mistakes that create obviously mistaken changes, like copying an entire
+repository into itself and pushing the result. This kind of change can take
+some effort to clean up if it becomes part of repository history.
+
+Note that if you move a file, both the old and new locations count as touched
+paths. You should generally configure this limit to be more than twice the
+number of files you anticipate any user ever legitimately wanting to move in
+a single commit. For example, a limit of `20000` will let users move up to
+10,000 files in a single commit, but will reject users mistakenly trying to
+push a copy of another repository or a directory with a million logfiles or
+whatever other kind of creative nonsense they manage to dream up.
+
+
Branches
========

File Metadata

Mime Type
text/plain
Expires
Tue, Mar 18, 4:14 PM (3 d, 16 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7582991
Default Alt Text
D19839.id47392.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility