@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 most file reads policy-aware
Summary: Ref T603. Swaps out most `PhabricatorFile` loads for `PhabricatorFileQuery`.
Test Plan:
- Viewed Differential changesets.
- Used `file.info`.
- Used `file.download`.
- Viewed a file.
- Deleted a file.
- Used `/Fnnnn` to access a file.
- Uploaded an image, verified a thumbnail generated.
- Created and edited a macro.
- Added a meme.
- Did old-school attach-a-file-to-a-task.
- Viewed a paste.
- Viewed a mock.
- Embedded a mock.
- Profiled a page.
- Parsed a commit with image files linked to a revision with image files.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7178
+124
-67
+9
-3
src/applications/differential/controller/DifferentialChangesetViewController.php
+9
-3
src/applications/differential/controller/DifferentialChangesetViewController.php
···
296
296
DifferentialChangeset $changeset,
297
297
$is_new) {
298
298
299
+
$viewer = $this->getRequest()->getUser();
300
+
299
301
if ($is_new) {
300
302
$key = 'raw:new:phid';
301
303
} else {
···
307
309
$file = null;
308
310
$phid = idx($metadata, $key);
309
311
if ($phid) {
310
-
$file = id(new PhabricatorFile())->loadOneWhere(
311
-
'phid = %s',
312
-
$phid);
312
+
$file = id(new PhabricatorFileQuery())
313
+
->setViewer($viewer)
314
+
->withPHIDs(array($phid))
315
+
->execute();
316
+
if ($file) {
317
+
$file = head($file);
318
+
}
313
319
}
314
320
315
321
if (!$file) {
+1
src/applications/differential/controller/DifferentialRevisionViewController.php
+1
src/applications/differential/controller/DifferentialRevisionViewController.php
+1
src/applications/differential/mail/DifferentialReviewRequestMail.php
+1
src/applications/differential/mail/DifferentialReviewRequestMail.php
+1
src/applications/differential/parser/DifferentialChangesetParser.php
+1
src/applications/differential/parser/DifferentialChangesetParser.php
+4
-3
src/applications/files/conduit/ConduitAPI_file_download_Method.php
+4
-3
src/applications/files/conduit/ConduitAPI_file_download_Method.php
···
29
29
protected function execute(ConduitAPIRequest $request) {
30
30
$phid = $request->getValue('phid');
31
31
32
-
$file = id(new PhabricatorFile())->loadOneWhere(
33
-
'phid = %s',
34
-
$phid);
32
+
$file = id(new PhabricatorFileQuery())
33
+
->setViewer($request->getUser())
34
+
->withPHIDs(array($phid))
35
+
->executeOne();
35
36
if (!$file) {
36
37
throw new ConduitException('ERR-BAD-PHID');
37
38
}
+6
-4
src/applications/files/conduit/ConduitAPI_file_info_Method.php
+6
-4
src/applications/files/conduit/ConduitAPI_file_info_Method.php
···
30
30
$phid = $request->getValue('phid');
31
31
$id = $request->getValue('id');
32
32
33
+
$query = id(new PhabricatorFileQuery())
34
+
->setViewer($request->getUser());
33
35
if ($id) {
34
-
$file = id(new PhabricatorFile())->load($id);
36
+
$query->withIDs(array($id));
35
37
} else {
36
-
$file = id(new PhabricatorFile())->loadOneWhere(
37
-
'phid = %s',
38
-
$phid);
38
+
$query->withPHIDs(array($phid));
39
39
}
40
+
41
+
$file = $query->executeOne();
40
42
41
43
if (!$file) {
42
44
throw new ConduitException('ERR-NOT-FOUND');
+4
-3
src/applications/files/controller/PhabricatorFileDataController.php
+4
-3
src/applications/files/controller/PhabricatorFileDataController.php
···
25
25
->setURI($uri->setPath($request->getPath()));
26
26
}
27
27
28
-
$file = id(new PhabricatorFile())->loadOneWhere(
29
-
'phid = %s',
30
-
$this->phid);
28
+
$file = id(new PhabricatorFileQuery())
29
+
->setViewer($request->getUser())
30
+
->withPHIDs(array($this->phid))
31
+
->executeOne();
31
32
if (!$file) {
32
33
return new Aphront404Response();
33
34
}
+9
-4
src/applications/files/controller/PhabricatorFileDeleteController.php
+9
-4
src/applications/files/controller/PhabricatorFileDeleteController.php
···
9
9
}
10
10
11
11
public function processRequest() {
12
-
13
12
$request = $this->getRequest();
14
13
$user = $request->getUser();
15
14
16
-
$file = id(new PhabricatorFile())->loadOneWhere(
17
-
'id = %d',
18
-
$this->id);
15
+
$file = id(new PhabricatorFileQuery())
16
+
->setViewer($user)
17
+
->withIDs(array($this->id))
18
+
->requireCapabilities(
19
+
array(
20
+
PhabricatorPolicyCapability::CAN_VIEW,
21
+
PhabricatorPolicyCapability::CAN_EDIT,
22
+
))
23
+
->executeOne();
19
24
if (!$file) {
20
25
return new Aphront404Response();
21
26
}
+5
-1
src/applications/files/controller/PhabricatorFileShortcutController.php
+5
-1
src/applications/files/controller/PhabricatorFileShortcutController.php
···
10
10
}
11
11
12
12
public function processRequest() {
13
-
$file = id(new PhabricatorFile())->load($this->id);
13
+
$file = id(new PhabricatorFileQuery())
14
+
->setViewer($this->getRequest()->getUser())
15
+
->withIDs(array($this->id))
16
+
->executeOne();
14
17
if (!$file) {
15
18
return new Aphront404Response();
16
19
}
20
+
17
21
return id(new AphrontRedirectResponse())->setURI($file->getBestURI());
18
22
}
19
23
+12
-11
src/applications/files/controller/PhabricatorFileTransformController.php
+12
-11
src/applications/files/controller/PhabricatorFileTransformController.php
···
18
18
}
19
19
20
20
public function processRequest() {
21
+
$viewer = $this->getRequest()->getUser();
21
22
22
-
$file = id(new PhabricatorFile())->loadOneWhere('phid = %s', $this->phid);
23
+
$file = id(new PhabricatorFileQuery())
24
+
->setViewer($viewer)
25
+
->withPHIDs(array($this->phid))
26
+
->executeOne();
23
27
if (!$file) {
24
28
return new Aphront404Response();
25
29
}
···
125
129
private function buildTransformedFileResponse(
126
130
PhabricatorTransformedFile $xform) {
127
131
128
-
$file = id(new PhabricatorFile())->loadOneWhere(
129
-
'phid = %s',
130
-
$xform->getTransformedPHID());
131
-
if ($file) {
132
-
$uri = $file->getBestURI();
133
-
} else {
134
-
$bad_phid = $xform->getTransformedPHID();
135
-
throw new Exception(
136
-
"Unable to load file with phid {$bad_phid}."
137
-
);
132
+
$file = id(new PhabricatorFileQuery())
133
+
->setViewer($this->getRequest()->getUser())
134
+
->withPHIDs(array($xform->getTransformedPHID()))
135
+
->executeOne();
136
+
if (!$file) {
137
+
return new Aphront404Response();
138
138
}
139
139
140
140
// TODO: We could just delegate to the file view controller instead,
141
141
// which would save the client a roundtrip, but is slightly more complex.
142
+
$uri = $file->getBestURI();
142
143
return id(new AphrontRedirectResponse())->setURI($uri);
143
144
}
144
145
+2
src/applications/files/management/PhabricatorFilesManagementWorkflow.php
+2
src/applications/files/management/PhabricatorFilesManagementWorkflow.php
+2
src/applications/files/remarkup/PhabricatorRemarkupRuleEmbedFile.php
+2
src/applications/files/remarkup/PhabricatorRemarkupRuleEmbedFile.php
···
21
21
$file = null;
22
22
if ($matches[1]) {
23
23
// TODO: This is pretty inefficient if there are a bunch of files.
24
+
// TODO: (T603) This isn't policy-aware and should be extending
25
+
// PhabricatorRemarkupRuleObject.
24
26
$file = id(new PhabricatorFile())->load($matches[1]);
25
27
}
26
28
+1
src/applications/files/storage/PhabricatorFile.php
+1
src/applications/files/storage/PhabricatorFile.php
+4
-3
src/applications/macro/controller/PhabricatorMacroEditController.php
+4
-3
src/applications/macro/controller/PhabricatorMacroEditController.php
···
82
82
$errors[] = pht('Could not fetch URL: %s', $ex->getMessage());
83
83
}
84
84
} else if ($request->getStr('phid')) {
85
-
$file = id(new PhabricatorFile())->loadOneWhere(
86
-
'phid = %s',
87
-
$request->getStr('phid'));
85
+
$file = id(new PhabricatorFileQuery())
86
+
->setViewer($user)
87
+
->withPHIDs(array($request->getStr('phid')))
88
+
->executeOne();
88
89
}
89
90
90
91
if ($file) {
+8
-3
src/applications/macro/controller/PhabricatorMacroMemeController.php
+8
-3
src/applications/macro/controller/PhabricatorMacroMemeController.php
···
38
38
$file->getPHID(), $hash);
39
39
40
40
if ($xform) {
41
-
$memefile = id(new PhabricatorFile())->loadOneWhere(
42
-
'phid = %s', $xform->getTransformedPHID());
43
-
return $memefile->getBestURI();
41
+
$memefile = id(new PhabricatorFileQuery())
42
+
->setViewer($user)
43
+
->withPHIDs(array($xform->getTransformedPHID()))
44
+
->executeOne();
45
+
if ($memefile) {
46
+
return $memefile->getBestURI();
47
+
}
44
48
}
49
+
45
50
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
46
51
$transformers = (new PhabricatorImageTransformer());
47
52
$newfile = $transformers
+6
-3
src/applications/maniphest/controller/ManiphestTaskDetailController.php
+6
-3
src/applications/maniphest/controller/ManiphestTaskDetailController.php
···
611
611
if ($file_infos) {
612
612
$file_phids = array_keys($file_infos);
613
613
614
-
$files = id(new PhabricatorFile())->loadAllWhere(
615
-
'phid IN (%Ls)',
616
-
$file_phids);
614
+
// TODO: These should probably be handles or something; clean this up
615
+
// as we sort out file attachments.
616
+
$files = id(new PhabricatorFileQuery())
617
+
->setViewer($viewer)
618
+
->withPHIDs($file_phids)
619
+
->execute();
617
620
618
621
$file_view = new PhabricatorFileLinkListView();
619
622
$file_view->setFiles($files);
+4
-3
src/applications/maniphest/controller/ManiphestTaskEditController.php
+4
-3
src/applications/maniphest/controller/ManiphestTaskEditController.php
···
72
72
}
73
73
74
74
if ($file_phids) {
75
-
$files = id(new PhabricatorFile())->loadAllWhere(
76
-
'phid IN (%Ls)',
77
-
$file_phids);
75
+
$files = id(new PhabricatorFileQuery())
76
+
->setViewer($user)
77
+
->withPHIDs($file_phids)
78
+
->execute();
78
79
}
79
80
80
81
$template_id = $request->getInt('template');
+4
-3
src/applications/maniphest/controller/ManiphestTransactionSaveController.php
+4
-3
src/applications/maniphest/controller/ManiphestTransactionSaveController.php
···
33
33
// Look for drag-and-drop uploads first.
34
34
$file_phids = $request->getArr('files');
35
35
if ($file_phids) {
36
-
$files = id(new PhabricatorFile())->loadAllWhere(
37
-
'phid in (%Ls)',
38
-
$file_phids);
36
+
$files = id(new PhabricatorFileQuery())
37
+
->setViewer($user)
38
+
->withPHIDs(array($file_phids))
39
+
->execute();
39
40
}
40
41
41
42
// This means "attach a file" even though we store other types of data
+1
src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php
+1
src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php
+4
-3
src/applications/paste/controller/PhabricatorPasteViewController.php
+4
-3
src/applications/paste/controller/PhabricatorPasteViewController.php
···
44
44
return new Aphront404Response();
45
45
}
46
46
47
-
$file = id(new PhabricatorFile())->loadOneWhere(
48
-
'phid = %s',
49
-
$paste->getFilePHID());
47
+
$file = id(new PhabricatorFileQuery())
48
+
->setViewer($user)
49
+
->withPHIDs(array($paste->getFilePHID()))
50
+
->executeOne();
50
51
if (!$file) {
51
52
return new Aphront400Response();
52
53
}
+4
-3
src/applications/paste/query/PhabricatorPasteQuery.php
+4
-3
src/applications/paste/query/PhabricatorPasteQuery.php
···
162
162
163
163
private function loadRawContent(array $pastes) {
164
164
$file_phids = mpull($pastes, 'getFilePHID');
165
-
$files = id(new PhabricatorFile())->loadAllWhere(
166
-
'phid IN (%Ls)',
167
-
$file_phids);
165
+
$files = id(new PhabricatorFileQuery())
166
+
->setViewer($this->getViewer())
167
+
->withPHIDs($file_phids)
168
+
->execute();
168
169
$files = mpull($files, null, 'getPHID');
169
170
170
171
foreach ($pastes as $key => $paste) {
+2
src/applications/people/storage/PhabricatorUser.php
+2
src/applications/people/storage/PhabricatorUser.php
···
746
746
$src_phid = $this->getProfileImagePHID();
747
747
748
748
if ($src_phid) {
749
+
// TODO: (T603) Can we get rid of this entirely and move it to
750
+
// PeopleQuery with attach/attachable?
749
751
$file = id(new PhabricatorFile())->loadOneWhere('phid = %s', $src_phid);
750
752
if ($file) {
751
753
$this->profileImage = $file->getBestURI();
+8
-3
src/applications/pholio/controller/PholioInlineThumbController.php
+8
-3
src/applications/pholio/controller/PholioInlineThumbController.php
···
34
34
return new Aphront404Response();
35
35
}
36
36
37
-
$file = id(new PhabricatorFile())->loadOneWhere(
38
-
'phid = %s',
39
-
$image->getFilePHID());
37
+
$file = id(new PhabricatorFileQuery())
38
+
->setViewer($user)
39
+
->witHPHIDs(array($image->getFilePHID()))
40
+
->executeOne();
41
+
42
+
if (!$file) {
43
+
return new Aphront404Response();
44
+
}
40
45
41
46
return id(new AphrontRedirectResponse())->setURI($file->getThumb60x45URI());
42
47
}
+6
-3
src/applications/pholio/query/PholioImageQuery.php
+6
-3
src/applications/pholio/query/PholioImageQuery.php
···
104
104
assert_instances_of($images, 'PholioImage');
105
105
106
106
$file_phids = mpull($images, 'getFilePHID');
107
-
$all_files = mpull(id(new PhabricatorFile())->loadAllWhere(
108
-
'phid IN (%Ls)',
109
-
$file_phids), null, 'getPHID');
107
+
108
+
$all_files = id(new PhabricatorFileQuery())
109
+
->setViewer($this->getViewer())
110
+
->withPHIDs($file_phids)
111
+
->execute();
112
+
$all_files = mpull($all_files, null, 'getPHID');
110
113
111
114
if ($this->needInlineComments) {
112
115
$all_inline_comments = id(new PholioTransactionComment())
+6
-3
src/applications/pholio/query/PholioMockQuery.php
+6
-3
src/applications/pholio/query/PholioMockQuery.php
···
132
132
private function loadCoverFiles(array $mocks) {
133
133
assert_instances_of($mocks, 'PholioMock');
134
134
$cover_file_phids = mpull($mocks, 'getCoverPHID');
135
-
$cover_files = mpull(id(new PhabricatorFile())->loadAllWhere(
136
-
'phid IN (%Ls)',
137
-
$cover_file_phids), null, 'getPHID');
135
+
$cover_files = id(new PhabricatorFileQuery())
136
+
->setViewer($this->getViewer())
137
+
->withPHIDs($cover_file_phids)
138
+
->execute();
139
+
140
+
$cover_files = mpull($cover_files, null, 'getPHID');
138
141
139
142
foreach ($mocks as $mock) {
140
143
$file = idx($cover_files, $mock->getCoverPHID());
+1
src/applications/project/storage/PhabricatorProjectProfile.php
+1
src/applications/project/storage/PhabricatorProjectProfile.php
···
9
9
public function loadProfileImageURI() {
10
10
$src_phid = $this->getProfileImagePHID();
11
11
12
+
// TODO: (T603) Can we get rid of this and move it to a Query?
12
13
$file = id(new PhabricatorFile())->loadOneWhere('phid = %s', $src_phid);
13
14
if ($file) {
14
15
return $file->getBestURI();
+4
-3
src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+4
-3
src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
···
329
329
330
330
$files = array();
331
331
if ($file_phids) {
332
-
$files = id(new PhabricatorFile())->loadAllWhere(
333
-
'phid IN (%Ls)',
334
-
$file_phids);
332
+
$files = id(new PhabricatorFileQuery())
333
+
->setViewer(PhabricatorUser::getOmnipotentUser())
334
+
->withPHIDs($file_phids)
335
+
->execute();
335
336
$files = mpull($files, null, 'getPHID');
336
337
}
337
338
+5
-5
src/applications/xhprof/controller/PhabricatorXHProfProfileController.php
+5
-5
src/applications/xhprof/controller/PhabricatorXHProfProfileController.php
···
10
10
}
11
11
12
12
public function processRequest() {
13
+
$request = $this->getRequest();
13
14
14
-
$file = id(new PhabricatorFile())->loadOneWhere(
15
-
'phid = %s',
16
-
$this->phid);
17
-
15
+
$file = id(new PhabricatorFileQuery())
16
+
->setViewer($request->getUser())
17
+
->withPHIDs(array($this->phid))
18
+
->executeOne();
18
19
if (!$file) {
19
20
return new Aphront404Response();
20
21
}
···
25
26
throw new Exception("Failed to unserialize XHProf profile!");
26
27
}
27
28
28
-
$request = $this->getRequest();
29
29
$symbol = $request->getStr('symbol');
30
30
31
31
$is_framed = $request->getBool('frame');