Skip to content

Commit

Permalink
Fix cross-platform incompatibility with Storage Emulator exports (#4411)
Browse files Browse the repository at this point in the history
* Fix cross-platform incompatibility with Storage Emulator exports

* Add CHANGELOG entries

* Check separator after bucket name instead; add comments

* Check separator using bucket naming guidelines

* Add test case for nested directory
  • Loading branch information
tohhsinpei committed Apr 8, 2022
1 parent 8e19a6f commit 3d43bae
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
- Fix URL with wrong host returned in storage resumable upload (#4374).
- Fixes Firestore emulator transaction expiration and reused bug.
- Fixes Firestore emulator deadlock bug. [#2452](https://github.com/firebase/firebase-tools/issues/2452)
- Fixes console error on large uploads to Storage Emulator (#4407).
- Fixes cross-platform incompatibility with Storage Emulator exports (#4411).
20 changes: 20 additions & 0 deletions scripts/storage-emulator-integration/import/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,26 @@ describe("Import Emulator Data", () => {
.expect(200);
});

it("retrieves file from importing emulator data previously exported on Windows", async function (this) {
this.timeout(TEST_SETUP_TIMEOUT);
await test.startEmulators([
"--only",
Emulators.STORAGE,
"--import",
path.join(__dirname, "windows-emulator-data"),
]);

await supertest(STORAGE_EMULATOR_HOST)
.get(`/v0/b/${BUCKET}/o/test_upload.jpg`)
.set({ Authorization: "Bearer owner" })
.expect(200);

await supertest(STORAGE_EMULATOR_HOST)
.get(`/v0/b/${BUCKET}/o/test-directory%2Ftest_nested_upload.jpg`)
.set({ Authorization: "Bearer owner" })
.expect(200);
});

afterEach(async function (this) {
this.timeout(EMULATORS_SHUTDOWN_DELAY_MS);
await test.stopEmulators();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"version": "10.4.2",
"storage": {
"version": "10.4.2",
"path": "storage_export"
}
}
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"buckets": [
{
"id": "fake-project-id.appspot.com"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"name": "test-directory\\test_nested_upload.jpg",
"bucket": "fake-project-id.appspot.com",
"contentType": "application/octet-stream",
"metageneration": 1,
"generation": 1648084940926,
"storageClass": "STANDARD",
"contentDisposition": "inline",
"cacheControl": "public, max-age=3600",
"contentEncoding": "identity",
"downloadTokens": [
"c3c71086-95a8-445d-96e7-f625972de4b0"
],
"etag": "PQJQBXRweACX9yRsBEInQjOJ/0s",
"timeCreated": "2022-03-24T01:22:20.926Z",
"updated": "2022-03-24T01:22:20.926Z",
"size": 0,
"md5Hash": "1B2M2Y8AsgTpgAmY7PhCfg==",
"crc32c": "0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"name": "test_upload.jpg",
"bucket": "fake-project-id.appspot.com",
"contentType": "application/octet-stream",
"metageneration": 1,
"generation": 1648084940926,
"storageClass": "STANDARD",
"contentDisposition": "inline",
"cacheControl": "public, max-age=3600",
"contentEncoding": "identity",
"downloadTokens": [
"c3c71086-95a8-445d-96e7-f625972de4b0"
],
"etag": "PQJQBXRweACX9yRsBEInQjOJ/0s",
"timeCreated": "2022-03-24T01:22:20.926Z",
"updated": "2022-03-24T01:22:20.926Z",
"size": 0,
"md5Hash": "1B2M2Y8AsgTpgAmY7PhCfg==",
"crc32c": "0"
}
18 changes: 17 additions & 1 deletion src/emulator/storage/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,8 @@ export class StorageLayer {
for (const b of await this.listBuckets()) {
bucketsList.buckets.push({ id: b.id });
}
// Resulting path is platform-specific, e.g. foo%5Cbar on Windows, foo%2Fbar on Linux
// after URI encoding. Similarly for metadata paths below.
const bucketsFilePath = path.join(storageExportPath, "buckets.json");
await fse.writeFile(bucketsFilePath, JSON.stringify(bucketsList, undefined, 2));

Expand Down Expand Up @@ -583,7 +585,13 @@ export class StorageLayer {
continue;
}

const decodedBlobPath = decodeURIComponent(blobPath);
let decodedBlobPath = decodeURIComponent(blobPath);
const decodedBlobPathSep = getPathSep(decodedBlobPath);
// Replace all file separators with that of current platform for compatibility
if (decodedBlobPathSep !== path.sep) {
decodedBlobPath = decodedBlobPath.split(decodedBlobPathSep).join(path.sep);
}

const blobDiskPath = this._persistence.getDiskPath(decodedBlobPath);

const file = new StoredFile(metadata, blobDiskPath);
Expand All @@ -605,3 +613,11 @@ export class StorageLayer {
}
}
}

/** Returns file separator used in given path, either '\\' or '/'. */
function getPathSep(decodedPath: string): string {
// Suffices to check first separator, which occurs immediately after bucket name.
// Bucket naming guidelines: https://cloud.google.com/storage/docs/naming-buckets
const firstSepIndex = decodedPath.search(/[^a-z0-9-_.]/g);
return decodedPath[firstSepIndex];
}

0 comments on commit 3d43bae

Please sign in to comment.