Skip to content

Commit f1f082c

Browse files
authored
Create a working copy of the uploaded blob before processing it. (#8725)
1 parent 3d614cf commit f1f082c

File tree

1 file changed

+37
-13
lines changed

1 file changed

+37
-13
lines changed

app/lib/package/backend.dart

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ PackageBackend get packageBackend =>
7979
class PackageBackend {
8080
final DatastoreDB db;
8181

82+
final Storage _storage;
83+
8284
/// The Cloud Storage bucket to use for incoming package archives.
8385
/// The following files are present:
8486
/// - `tmp/$guid` (incoming package archive that was uploaded, but not yet processed)
@@ -92,12 +94,12 @@ class PackageBackend {
9294

9395
PackageBackend(
9496
this.db,
95-
Storage storage,
97+
this._storage,
9698
this._incomingBucket,
9799
Bucket canonicalBucket,
98100
Bucket publicBucket,
99101
) : tarballStorage =
100-
TarballStorage(db, storage, canonicalBucket, publicBucket);
102+
TarballStorage(db, _storage, canonicalBucket, publicBucket);
101103

102104
/// Whether the package exists and is not blocked or deleted.
103105
Future<bool> isPackageVisible(String package) async {
@@ -891,28 +893,49 @@ class PackageBackend {
891893

892894
/// Finishes the upload of a package and returns the list of messages
893895
/// related to the publishing.
894-
Future<List<String>> publishUploadedBlob(String guid) async {
896+
Future<List<String>> publishUploadedBlob(String uploadGuid) async {
895897
final restriction = await getUploadRestrictionStatus();
896898
if (restriction == UploadRestrictionStatus.noUploads) {
897899
throw PackageRejectedException.uploadRestricted();
898900
}
899901
final agent = await requireAuthenticatedClient();
900-
_logger.info('Finishing async upload (uuid: $guid)');
902+
_logger.info('Finishing async upload (uuid: $uploadGuid)');
901903
_logger.info('Reading tarball from cloud storage.');
902904

903905
return await withTempDirectory((Directory dir) async {
904-
final filename = '${dir.absolute.path}/tarball.tar.gz';
905-
final info = await _incomingBucket.tryInfo(tmpObjectName(guid));
906+
// Check the existence of the uploaded file
907+
final uploadObjectName = tmpObjectName(uploadGuid);
908+
final info = await _incomingBucket.tryInfo(uploadObjectName);
906909
if (info?.length == null) {
907910
throw PackageRejectedException.archiveEmpty();
908911
}
912+
913+
// Create a temporary copy that we will continue working with.
914+
// This will protect us against the unlikely scenario where the
915+
// uploaded blob changes during this processing.
916+
final workGuid = createUuid();
917+
final workObjectName = '${tmpObjectName(workGuid)}-$uploadGuid';
918+
try {
919+
await _storage.copyObjectWithRetry(
920+
_incomingBucket.absoluteObjectName(uploadObjectName),
921+
_incomingBucket.absoluteObjectName(workObjectName),
922+
);
923+
} catch (e, st) {
924+
_logger.warning('Failed to copy uploaded file to work object.', e, st);
925+
throw InvalidInputException(
926+
'Failed to copy uploaded file (uuid:$uploadGuid).');
927+
}
928+
929+
// Check the file size is within limits.
909930
if (info!.length > UploadSignerService.maxUploadSize) {
910931
throw PackageRejectedException.archiveTooLarge(
911932
UploadSignerService.maxUploadSize);
912933
}
934+
935+
final filename = '${dir.absolute.path}/tarball.tar.gz';
913936
await _incomingBucket.readWithRetry(
914-
tmpObjectName(guid), (input) => _saveTarballToFS(input, filename));
915-
_logger.info('Examining tarball content ($guid).');
937+
workObjectName, (input) => _saveTarballToFS(input, filename));
938+
_logger.info('Examining tarball content ($uploadGuid).');
916939
final sw = Stopwatch()..start();
917940
final file = File(filename);
918941
final fileLength = await file.length();
@@ -1009,15 +1032,16 @@ class PackageBackend {
10091032
entities: entities,
10101033
agent: agent,
10111034
archive: archive,
1012-
guid: guid,
1035+
objectName: workObjectName,
10131036
hasCanonicalArchiveObject:
10141037
canonicalContentMatch == ContentMatchStatus.same,
10151038
);
10161039
_logger.info('Tarball uploaded in ${sw.elapsed}.');
1017-
_logger.info('Removing temporary object $guid.');
1040+
_logger.info('Removing temporary object $uploadGuid.');
10181041

10191042
sw.reset();
1020-
await _incomingBucket.deleteWithRetry(tmpObjectName(guid));
1043+
await _incomingBucket.deleteWithRetry(uploadObjectName);
1044+
await _incomingBucket.deleteWithRetry(workObjectName);
10211045
_logger.info('Temporary object removed in ${sw.elapsed}.');
10221046
return [
10231047
'Successfully uploaded '
@@ -1087,7 +1111,7 @@ class PackageBackend {
10871111
required _UploadEntities entities,
10881112
required AuthenticatedAgent agent,
10891113
required PackageSummary archive,
1090-
required String guid,
1114+
required String objectName,
10911115
required bool hasCanonicalArchiveObject,
10921116
}) async {
10931117
final sw = Stopwatch()..start();
@@ -1218,7 +1242,7 @@ class PackageBackend {
12181242
// Copy archive to canonical bucket.
12191243
await tarballStorage.copyFromTempToCanonicalBucket(
12201244
sourceAbsoluteObjectName:
1221-
_incomingBucket.absoluteObjectName(tmpObjectName(guid)),
1245+
_incomingBucket.absoluteObjectName(objectName),
12221246
package: newVersion.package,
12231247
version: newVersion.version!,
12241248
);

0 commit comments

Comments
 (0)