@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
Remove PhabricatorFile::buildFromFileDataOrHash()
Summary:
Ref T12464. This is a very old method which can return an existing file instead of creating a new one, if there's some existing file with the same content.
In the best case this is a bad idea. This being somewhat reasonable predates policies, temporary files, etc. Modern methods like `newFromFileData()` do this right: they share underlying data in storage, but not the actual `File` records.
Specifically, this is the case where we get into trouble:
- I upload a private file with content "X".
- You somehow generate a file with the same content by, say, viewing a raw diff in Differential.
- If the diff had the same content, you get my file, but you don't have permission to see it or whatever so everything breaks and is terrible.
Just get rid of this.
Test Plan:
- Generated an SSH key.
- Viewed a raw diff in Differential.
- (Did not test Phragment.)
Reviewers: chad
Reviewed By: chad
Subscribers: hach-que
Maniphest Tasks: T12464
Differential Revision: https://secure.phabricator.com/D17617
+33
-67
+1
-1
src/applications/auth/controller/PhabricatorAuthSSHKeyGenerateController.php
+1
-1
src/applications/auth/controller/PhabricatorAuthSSHKeyGenerateController.php
+8
-8
src/applications/differential/controller/DifferentialRevisionViewController.php
+8
-8
src/applications/differential/controller/DifferentialRevisionViewController.php
···
889
889
}
890
890
$file_name .= 'diff';
891
891
892
-
$file = PhabricatorFile::buildFromFileDataOrHash(
893
-
$raw_diff,
894
-
array(
895
-
'name' => $file_name,
896
-
'ttl.relative' => phutil_units('24 hours in seconds'),
897
-
'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
898
-
));
899
-
900
892
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
893
+
$file = PhabricatorFile::newFromFileData(
894
+
$raw_diff,
895
+
array(
896
+
'name' => $file_name,
897
+
'ttl.relative' => phutil_units('24 hours in seconds'),
898
+
'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
899
+
));
900
+
901
901
$file->attachToObject($revision->getPHID());
902
902
unset($unguarded);
903
903
-38
src/applications/files/storage/PhabricatorFile.php
-38
src/applications/files/storage/PhabricatorFile.php
···
197
197
}
198
198
199
199
200
-
/**
201
-
* Given a block of data, try to load an existing file with the same content
202
-
* if one exists. If it does not, build a new file.
203
-
*
204
-
* This method is generally used when we have some piece of semi-trusted data
205
-
* like a diff or a file from a repository that we want to show to the user.
206
-
* We can't just dump it out because it may be dangerous for any number of
207
-
* reasons; instead, we need to serve it through the File abstraction so it
208
-
* ends up on the CDN domain if one is configured and so on. However, if we
209
-
* simply wrote a new file every time we'd potentially end up with a lot
210
-
* of redundant data in file storage.
211
-
*
212
-
* To solve these problems, we use file storage as a cache and reuse the
213
-
* same file again if we've previously written it.
214
-
*
215
-
* NOTE: This method unguards writes.
216
-
*
217
-
* @param string Raw file data.
218
-
* @param dict Dictionary of file information.
219
-
*/
220
-
public static function buildFromFileDataOrHash(
221
-
$data,
222
-
array $params = array()) {
223
-
224
-
$file = id(new PhabricatorFile())->loadOneWhere(
225
-
'name = %s AND contentHash = %s LIMIT 1',
226
-
idx($params, 'name'),
227
-
self::hashFileContent($data));
228
-
229
-
if (!$file) {
230
-
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
231
-
$file = self::newFromFileData($data, $params);
232
-
unset($unguarded);
233
-
}
234
-
235
-
return $file;
236
-
}
237
-
238
200
public static function newFileFromContentHash($hash, array $params) {
239
201
// Check to see if a file with same contentHash exist
240
202
$file = id(new PhabricatorFile())->loadOneWhere(
+9
-6
src/applications/phragment/conduit/PhragmentGetPatchConduitAPIMethod.php
+9
-6
src/applications/phragment/conduit/PhragmentGetPatchConduitAPIMethod.php
···
174
174
unset($patches[$key]['fileOld']);
175
175
unset($patches[$key]['fileNew']);
176
176
177
-
$file = PhabricatorFile::buildFromFileDataOrHash(
178
-
$data,
179
-
array(
180
-
'name' => 'patch.dmp',
181
-
'ttl.relative' => phutil_units('24 hours in seconds'),
182
-
));
177
+
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
178
+
$file = PhabricatorFile::newFromFileData(
179
+
$data,
180
+
array(
181
+
'name' => 'patch.dmp',
182
+
'ttl.relative' => phutil_units('24 hours in seconds'),
183
+
));
184
+
unset($unguarded);
185
+
183
186
$patches[$key]['patchURI'] = $file->getDownloadURI();
184
187
}
185
188
+8
-8
src/applications/phragment/controller/PhragmentPatchController.php
+8
-8
src/applications/phragment/controller/PhragmentPatchController.php
···
78
78
$return = $request->getStr('return');
79
79
}
80
80
81
-
$result = PhabricatorFile::buildFromFileDataOrHash(
82
-
$patch,
83
-
array(
84
-
'name' => $name,
85
-
'mime-type' => 'text/plain',
86
-
'ttl.relative' => phutil_units('24 hours in seconds'),
87
-
));
88
-
89
81
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
82
+
$result = PhabricatorFile::newFromFileData(
83
+
$patch,
84
+
array(
85
+
'name' => $name,
86
+
'mime-type' => 'text/plain',
87
+
'ttl.relative' => phutil_units('24 hours in seconds'),
88
+
));
89
+
90
90
$result->attachToObject($version_b->getFragmentPHID());
91
91
unset($unguarded);
92
92
+7
-6
src/applications/phragment/controller/PhragmentZIPController.php
+7
-6
src/applications/phragment/controller/PhragmentZIPController.php
···
100
100
}
101
101
102
102
$data = Filesystem::readFile((string)$temp);
103
-
$file = PhabricatorFile::buildFromFileDataOrHash(
104
-
$data,
105
-
array(
106
-
'name' => $zip_name,
107
-
'ttl.relative' => phutil_units('24 hours in seconds'),
108
-
));
109
103
110
104
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
105
+
$file = PhabricatorFile::newFromFileData(
106
+
$data,
107
+
array(
108
+
'name' => $zip_name,
109
+
'ttl.relative' => phutil_units('24 hours in seconds'),
110
+
));
111
+
111
112
$file->attachToObject($fragment->getPHID());
112
113
unset($unguarded);
113
114