@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Make the Files "TTL" API more structured
Summary:
Ref T11357. When creating a file, callers can currently specify a `ttl`. However, it isn't unambiguous what you're supposed to pass, and some callers get it wrong.
For example, to mean "this file expires in 60 minutes", you might pass either of these:
- `time() + phutil_units('60 minutes in seconds')`
- `phutil_units('60 minutes in seconds')`
The former means "60 minutes from now". The latter means "1 AM, January 1, 1970". In practice, because the GC normally runs only once every four hours (at least, until recently), and all the bad TTLs are cases where files are normally accessed immediately, these 1970 TTLs didn't cause any real problems.
Split `ttl` into `ttl.relative` and `ttl.absolute`, and make sure the values are sane. Then correct all callers, and simplify out the `time()` calls where possible to make switching to `PhabricatorTime` easier.
Test Plan:
- Generated an SSH keypair.
- Viewed a changeset.
- Viewed a raw diff.
- Viewed a commit's file data.
- Viewed a temporary file's details, saw expiration date and relative time.
- Ran unit tests.
- (Didn't really test Phragment.)
Reviewers: chad
Reviewed By: chad
Subscribers: hach-que
Maniphest Tasks: T11357
Differential Revision: https://secure.phabricator.com/D17616
+91
-22
+1
-1
src/applications/auth/controller/PhabricatorAuthSSHKeyGenerateController.php
+1
-1
src/applications/auth/controller/PhabricatorAuthSSHKeyGenerateController.php
+1
-1
src/applications/differential/controller/DifferentialChangesetViewController.php
+1
-1
src/applications/differential/controller/DifferentialChangesetViewController.php
+1
-1
src/applications/differential/controller/DifferentialRevisionViewController.php
+1
-1
src/applications/differential/controller/DifferentialRevisionViewController.php
+2
-2
src/applications/diffusion/query/DiffusionFileFutureQuery.php
+2
-2
src/applications/diffusion/query/DiffusionFileFutureQuery.php
···
93
93
$drequest = $this->getRequest();
94
94
95
95
$name = basename($drequest->getPath());
96
-
$ttl = PhabricatorTime::getNow() + phutil_units('48 hours in seconds');
96
+
$relative_ttl = phutil_units('48 hours in seconds');
97
97
98
98
try {
99
99
$threshold = PhabricatorFileStorageEngine::getChunkThreshold();
···
101
101
102
102
$source = id(new PhabricatorExecFutureFileUploadSource())
103
103
->setName($name)
104
-
->setTTL($ttl)
104
+
->setRelativeTTL($relative_ttl)
105
105
->setViewPolicy(PhabricatorPolicies::POLICY_NOONE)
106
106
->setExecFuture($future);
107
107
+1
-1
src/applications/files/PhabricatorImageTransformer.php
+1
-1
src/applications/files/PhabricatorImageTransformer.php
+1
-1
src/applications/files/conduit/FileAllocateConduitAPIMethod.php
+1
-1
src/applications/files/conduit/FileAllocateConduitAPIMethod.php
+12
src/applications/files/controller/PhabricatorFileInfoController.php
+12
src/applications/files/controller/PhabricatorFileInfoController.php
···
212
212
pht('Mime Type'),
213
213
$file->getMimeType());
214
214
215
+
$ttl = $file->getTtl();
216
+
if ($ttl) {
217
+
$delta = $ttl - PhabricatorTime::getNow();
218
+
219
+
$finfo->addProperty(
220
+
pht('Expires'),
221
+
pht(
222
+
'%s (%s)',
223
+
phabricator_datetime($ttl, $viewer),
224
+
phutil_format_relative_time_detailed($delta)));
225
+
}
226
+
215
227
$width = $file->getImageWidth();
216
228
if ($width) {
217
229
$finfo->addProperty(
+61
-4
src/applications/files/storage/PhabricatorFile.php
+61
-4
src/applications/files/storage/PhabricatorFile.php
···
9
9
*
10
10
* | name | Human readable filename.
11
11
* | authorPHID | User PHID of uploader.
12
-
* | ttl | Temporary file lifetime, in seconds.
12
+
* | ttl.absolute | Temporary file lifetime as an epoch timestamp.
13
+
* | ttl.relative | Temporary file lifetime, relative to now, in seconds.
13
14
* | viewPolicy | File visibility policy.
14
15
* | isExplicitUpload | Used to show users files they explicitly uploaded.
15
16
* | canCDN | Allows the file to be cached and delivered over a CDN.
17
+
* | profile | Marks the file as a profile image.
18
+
* | format | Internal encoding format.
16
19
* | mime-type | Optional, explicit file MIME type.
17
20
* | builtin | Optional filename, identifies this as a builtin.
18
21
*
···
1110
1113
1111
1114
$params = array(
1112
1115
'name' => $builtin->getBuiltinDisplayName(),
1113
-
'ttl' => time() + (60 * 60 * 24 * 7),
1116
+
'ttl.relative' => phutil_units('7 days in seconds'),
1114
1117
'canCDN' => true,
1115
1118
'builtin' => $key,
1116
1119
);
···
1280
1283
* @return this
1281
1284
*/
1282
1285
private function readPropertiesFromParameters(array $params) {
1286
+
PhutilTypeSpec::checkMap(
1287
+
$params,
1288
+
array(
1289
+
'name' => 'optional string',
1290
+
'authorPHID' => 'optional string',
1291
+
'ttl.relative' => 'optional int',
1292
+
'ttl.absolute' => 'optional int',
1293
+
'viewPolicy' => 'optional string',
1294
+
'isExplicitUpload' => 'optional bool',
1295
+
'canCDN' => 'optional bool',
1296
+
'profile' => 'optional bool',
1297
+
'format' => 'optional string|PhabricatorFileStorageFormat',
1298
+
'mime-type' => 'optional string',
1299
+
'builtin' => 'optional string',
1300
+
'storageEngines' => 'optional list<PhabricatorFileStorageEngine>',
1301
+
));
1302
+
1283
1303
$file_name = idx($params, 'name');
1284
1304
$this->setName($file_name);
1285
1305
1286
1306
$author_phid = idx($params, 'authorPHID');
1287
1307
$this->setAuthorPHID($author_phid);
1288
1308
1289
-
$file_ttl = idx($params, 'ttl');
1290
-
$this->setTtl($file_ttl);
1309
+
$absolute_ttl = idx($params, 'ttl.absolute');
1310
+
$relative_ttl = idx($params, 'ttl.relative');
1311
+
if ($absolute_ttl !== null && $relative_ttl !== null) {
1312
+
throw new Exception(
1313
+
pht(
1314
+
'Specify an absolute TTL or a relative TTL, but not both.'));
1315
+
} else if ($absolute_ttl !== null) {
1316
+
if ($absolute_ttl < PhabricatorTime::getNow()) {
1317
+
throw new Exception(
1318
+
pht(
1319
+
'Absolute TTL must be in the present or future, but TTL "%s" '.
1320
+
'is in the past.',
1321
+
$absolute_ttl));
1322
+
}
1323
+
1324
+
$this->setTtl($absolute_ttl);
1325
+
} else if ($relative_ttl !== null) {
1326
+
if ($relative_ttl < 0) {
1327
+
throw new Exception(
1328
+
pht(
1329
+
'Relative TTL must be zero or more seconds, but "%s" is '.
1330
+
'negative.',
1331
+
$relative_ttl));
1332
+
}
1333
+
1334
+
$max_relative = phutil_units('365 days in seconds');
1335
+
if ($relative_ttl > $max_relative) {
1336
+
throw new Exception(
1337
+
pht(
1338
+
'Relative TTL must not be more than "%s" seconds, but TTL '.
1339
+
'"%s" was specified.',
1340
+
$max_relative,
1341
+
$relative_ttl));
1342
+
}
1343
+
1344
+
$absolute_ttl = PhabricatorTime::getNow() + $relative_ttl;
1345
+
1346
+
$this->setTtl($absolute_ttl);
1347
+
}
1291
1348
1292
1349
$view_policy = idx($params, 'viewPolicy');
1293
1350
if ($view_policy) {
+2
-2
src/applications/files/storage/__tests__/PhabricatorFileTestCase.php
+2
-2
src/applications/files/storage/__tests__/PhabricatorFileTestCase.php
···
370
370
371
371
$data = Filesystem::readRandomCharacters(64);
372
372
373
-
$ttl = (time() + 60 * 60 * 24);
373
+
$ttl = (PhabricatorTime::getNow() + phutil_units('24 hours in seconds'));
374
374
375
375
$params = array(
376
376
'name' => 'test.dat',
377
-
'ttl' => ($ttl),
377
+
'ttl.absolute' => $ttl,
378
378
'storageEngines' => array(
379
379
$engine,
380
380
),
+6
-6
src/applications/files/uploadsource/PhabricatorFileUploadSource.php
+6
-6
src/applications/files/uploadsource/PhabricatorFileUploadSource.php
···
4
4
extends Phobject {
5
5
6
6
private $name;
7
-
private $ttl;
7
+
private $relativeTTL;
8
8
private $viewPolicy;
9
9
10
10
private $rope;
···
22
22
return $this->name;
23
23
}
24
24
25
-
public function setTTL($ttl) {
26
-
$this->ttl = $ttl;
25
+
public function setRelativeTTL($relative_ttl) {
26
+
$this->relativeTTL = $relative_ttl;
27
27
return $this;
28
28
}
29
29
30
-
public function getTTL() {
31
-
return $this->ttl;
30
+
public function getRelativeTTL() {
31
+
return $this->relativeTTL;
32
32
}
33
33
34
34
public function setViewPolicy($view_policy) {
···
214
214
private function getNewFileParameters() {
215
215
return array(
216
216
'name' => $this->getName(),
217
-
'ttl' => $this->getTTL(),
217
+
'ttl.relative' => $this->getRelativeTTL(),
218
218
'viewPolicy' => $this->getViewPolicy(),
219
219
);
220
220
}
+1
-1
src/applications/phragment/conduit/PhragmentGetPatchConduitAPIMethod.php
+1
-1
src/applications/phragment/conduit/PhragmentGetPatchConduitAPIMethod.php
+1
-1
src/applications/phragment/controller/PhragmentPatchController.php
+1
-1
src/applications/phragment/controller/PhragmentPatchController.php
+1
-1
src/applications/phragment/controller/PhragmentZIPController.php
+1
-1
src/applications/phragment/controller/PhragmentZIPController.php