Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix storage admin sdk content type #5229

Merged
merged 6 commits into from Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -123,6 +123,13 @@ describe("GCS Javascript SDK conformance tests", () => {
expect(fileMetadata).to.deep.include(metadata);
});

it("should upload with proper content type", async () => {
const jpgFile = createRandomFile("small_file.jpg", SMALL_FILE_SIZE, tmpDir);
const [, fileMetadata] = await testBucket.upload(jpgFile);

expect(fileMetadata.contentType).to.equal("image/jpeg");
});

it("should handle firebaseStorageDownloadTokens", async () => {
const testFileName = "public/file";
await testBucket.upload(smallFilePath, {
Expand Down
Expand Up @@ -251,6 +251,26 @@ describe("GCS endpoint conformance tests", () => {
expect(returnedMetadata.contentType).to.equal(customMetadata.contentType);
expect(returnedMetadata.contentDisposition).to.equal(customMetadata.contentDisposition);
});

it("should upload content type properly from x-upload-content-type headers", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a test for the media upload protocol above as well?

const uploadURL = await supertest(storageHost)
.post(
`/upload/storage/v1/b/${storageBucket}/o?name=${TEST_FILE_NAME}&uploadType=resumable`
)
.set(authHeader)
.set({
"x-upload-content-type": "image/png",
})
.expect(200)
.then((res) => new URL(res.header["location"]));

const returnedMetadata = await supertest(storageHost)
.put(uploadURL.pathname + uploadURL.search)
.expect(200)
.then((res) => res.body);

expect(returnedMetadata.contentType).to.equal("image/png");
});
});

describe("multipart upload", () => {
Expand Down Expand Up @@ -292,6 +312,23 @@ describe("GCS endpoint conformance tests", () => {

expect(res.text).to.include("Bad content type.");
});

it("should upload content type properly from x-upload headers", async () => {
const returnedMetadata = await supertest(storageHost)
.post(`/upload/storage/v1/b/${storageBucket}/o?uploadType=multipart`)
.set(authHeader)
.set({
"content-type": "multipart/related; boundary=b1d5b2e3-1845-4338-9400-6ac07ce53c1e",
})
.set({
"x-upload-content-type": "text/plain",
})
.send(MULTIPART_REQUEST_BODY)
.expect(200)
.then((res) => res.body);

expect(returnedMetadata.contentType).to.equal("text/plain");
});
});
});

Expand Down
4 changes: 2 additions & 2 deletions src/emulator/storage/apis/firebase.ts
Expand Up @@ -232,7 +232,7 @@ export function createFirebaseEndpoints(emulator: StorageEmulator): Router {
const upload = uploadService.startResumableUpload({
bucketId,
objectId,
metadataRaw: JSON.stringify(req.body),
metadata: req.body,
// Store auth header for use in the finalize request
authorization: req.header("authorization"),
});
Expand Down Expand Up @@ -355,7 +355,7 @@ export function createFirebaseEndpoints(emulator: StorageEmulator): Router {
const upload = uploadService.multipartUpload({
bucketId,
objectId,
metadataRaw,
metadata: JSON.parse(metadataRaw),
dataRaw: dataRaw,
authorization: req.header("authorization"),
});
Expand Down
7 changes: 4 additions & 3 deletions src/emulator/storage/apis/gcloud.ts
Expand Up @@ -261,10 +261,11 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router {
res.sendStatus(400);
return;
}
const contentType = req.header("x-upload-content-type");
const upload = uploadService.startResumableUpload({
bucketId: req.params.bucketId,
objectId: name,
metadataRaw: JSON.stringify(req.body),
metadata: { contentType, ...req.body },
authorization: req.header("authorization"),
});

Expand Down Expand Up @@ -292,6 +293,7 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router {
// Multipart upload protocol.
if (uploadType === "multipart") {
const contentTypeHeader = req.header("content-type") || req.header("x-upload-content-type");
const contentType = req.header("x-upload-content-type");
if (!contentTypeHeader) {
return res.sendStatus(400);
}
Expand Down Expand Up @@ -319,11 +321,10 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router {
res.sendStatus(400);
return;
}

const upload = uploadService.multipartUpload({
bucketId: req.params.bucketId,
objectId: name,
metadataRaw: metadataRaw,
metadata: { contentType, ...JSON.parse(metadataRaw) },
dataRaw: dataRaw,
authorization: req.header("authorization"),
});
Expand Down
8 changes: 4 additions & 4 deletions src/emulator/storage/upload.ts
Expand Up @@ -44,7 +44,7 @@ export type MediaUploadRequest = {
export type MultipartUploadRequest = {
bucketId: string;
objectId: string;
metadataRaw: string;
metadata: object;
dataRaw: Buffer;
authorization?: string;
};
Expand All @@ -53,7 +53,7 @@ export type MultipartUploadRequest = {
export type StartResumableUploadRequest = {
bucketId: string;
objectId: string;
metadataRaw: string;
metadata: object;
authorization?: string;
};

Expand Down Expand Up @@ -117,7 +117,7 @@ export class UploadService {
objectId: request.objectId,
uploadType: UploadType.MULTIPART,
dataRaw: request.dataRaw,
metadata: JSON.parse(request.metadataRaw),
metadata: request.metadata,
authorization: request.authorization,
});
this._persistence.deleteFile(upload.path, /* failSilently = */ true);
Expand Down Expand Up @@ -155,7 +155,7 @@ export class UploadService {
type: UploadType.RESUMABLE,
path: this.getStagingFileName(id, request.bucketId, request.objectId),
status: UploadStatus.ACTIVE,
metadata: JSON.parse(request.metadataRaw),
metadata: request.metadata,
size: 0,
authorization: request.authorization,
};
Expand Down
6 changes: 3 additions & 3 deletions src/test/emulators/storage/files.spec.ts
Expand Up @@ -83,7 +83,7 @@ describe("files", () => {
bucketId,
objectId: encodeURIComponent(objectId),
dataRaw: Buffer.from(opts?.data ?? "hello world"),
metadataRaw: JSON.stringify(opts?.metadata ?? {}),
metadata: opts?.metadata ?? {},
});
await storageLayer.uploadObject(upload);
}
Expand All @@ -99,7 +99,7 @@ describe("files", () => {
const upload = _uploadService.startResumableUpload({
bucketId: "bucket",
objectId: "dir%2Fobject",
metadataRaw: "{}",
metadata: {},
});

expect(storageLayer.uploadObject(upload)).to.be.rejectedWith("Unexpected upload status");
Expand All @@ -110,7 +110,7 @@ describe("files", () => {
const uploadId = _uploadService.startResumableUpload({
bucketId: "bucket",
objectId: "dir%2Fobject",
metadataRaw: "{}",
metadata: {},
}).id;
_uploadService.continueResumableUpload(uploadId, Buffer.from("hello world"));
const upload = _uploadService.finalizeResumableUpload(uploadId);
Expand Down