Page MenuHomePhabricator
Files F15440318

D19839.id47367.diff
No OneTemporary
Actions

D19839.id47367.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
@@ -4200,6 +4200,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',
@@ -10190,6 +10191,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 )------------------------------------------------------------- */
@@ -1268,40 +1269,73 @@
private function rejectOversizedFiles(array $content_updates) {
$repository = $this->getRepository();
- $limit = $repository->getFilesizeLimit();
- if (!$limit) {
+ $filesize_limit = $repository->getFilesizeLimit();
+ $touch_limit = $repository->getTouchLimit();
+
+ if (!$filesize_limit && !$touch_limit) {
return;
}
foreach ($content_updates as $update) {
$identifier = $update->getRefNew();
- $sizes = $this->loadFileSizesForCommit($identifier);
- foreach ($sizes as $path => $size) {
- if ($size <= $limit) {
- continue;
- }
+ $sizes = $this->getFileSizesForCommit($identifier);
- $message = pht(
- 'OVERSIZED FILE'.
- "\n".
- 'This repository ("%s") is configured with a maximum individual '.
- 'file size limit, but you are pushing a change ("%s") which causes '.
- 'the size of a file ("%s") to exceed the limit. The commit makes '.
- 'the file %s bytes long, but the limit for this repository is '.
- '%s bytes.',
- $repository->getDisplayName(),
- $identifier,
- $path,
- new PhutilNumber($size),
- new PhutilNumber($limit));
+ if ($filesize_limit) {
+ foreach ($sizes as $path => $size) {
+ if ($size <= $filesize_limit) {
+ continue;
+ }
- throw new DiffusionCommitHookRejectException($message);
+ $message = pht(
+ 'OVERSIZED FILE'.
+ "\n".
+ 'This repository ("%s") is configured with a maximum individual '.
+ 'file size limit, but you are pushing a change ("%s") which '.
+ 'causes the size of a file ("%s") to exceed the limit. The '.
+ 'commit makes the file %s bytes long, but the limit for this '.
+ 'repository is %s bytes.',
+ $repository->getDisplayName(),
+ $identifier,
+ $path,
+ new PhutilNumber($size),
+ new PhutilNumber($filesize_limit));
+
+ throw new DiffusionCommitHookRejectException($message);
+ }
+ }
+
+ if ($touch_limit) {
+ if (count($sizes) > $touch_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($touch_limit));
+
+ throw new DiffusionCommitHookRejectException($message);
+ }
}
}
}
- public function loadFileSizesForCommit($identifier) {
+ 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/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
Thu, Mar 27, 12:04 PM (4 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7724533
Default Alt Text
D19839.id47367.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility