From c90bf7773271ba2d9b66c6f34a7f9e5d7caefe95 Mon Sep 17 00:00:00 2001 From: Chris Rybicki Date: Wed, 18 Sep 2024 12:34:32 -0400 Subject: [PATCH] fix(sdk): local bucket files can't be accessed in simulator (#7143) --- .../sdk/src/shared-aws/bucket.inflight.ts | 11 --- .../sdk/src/shared-azure/bucket.inflight.ts | 5 -- .../sdk/src/shared-gcp/bucket.inflight.ts | 11 --- .../sdk/src/target-sim/bucket.inflight.ts | 38 +++----- .../test/shared-aws/bucket.inflight.test.ts | 56 ------------ .../test/shared-azure/bucket.inflight.test.ts | 25 ------ .../sdk/test/target-sim/bucket.test.ts | 86 +++++++------------ tests/sdk_tests/bucket/public_url.test.w | 6 +- tests/sdk_tests/bucket/signed_url.test.w | 19 ---- .../bucket/signed_url.test.w_test_sim.md | 3 +- 10 files changed, 43 insertions(+), 217 deletions(-) diff --git a/packages/@winglang/sdk/src/shared-aws/bucket.inflight.ts b/packages/@winglang/sdk/src/shared-aws/bucket.inflight.ts index baa8fb30057..307d1cdbc4f 100644 --- a/packages/@winglang/sdk/src/shared-aws/bucket.inflight.ts +++ b/packages/@winglang/sdk/src/shared-aws/bucket.inflight.ts @@ -369,12 +369,6 @@ export class BucketClient implements IAwsBucketClient { throw new Error("Cannot provide public url for a non-public bucket"); } - if (!(await this.exists(key))) { - throw new Error( - `Cannot provide public url for a non-existent key (key=${key})` - ); - } - const region = await this.getLocation(); return encodeURI( @@ -401,11 +395,6 @@ export class BucketClient implements IAwsBucketClient { // Set the S3 command switch (action) { case BucketSignedUrlAction.DOWNLOAD: - if (!(await this.exists(key))) { - throw new Error( - `Cannot provide signed url for a non-existent key (key=${key})` - ); - } s3Command = new GetObjectCommand({ Bucket: this.bucketName, Key: key, diff --git a/packages/@winglang/sdk/src/shared-azure/bucket.inflight.ts b/packages/@winglang/sdk/src/shared-azure/bucket.inflight.ts index a5247862689..e7a47a81703 100644 --- a/packages/@winglang/sdk/src/shared-azure/bucket.inflight.ts +++ b/packages/@winglang/sdk/src/shared-azure/bucket.inflight.ts @@ -247,11 +247,6 @@ export class BucketClient implements IBucketClient { if (!accessPolicy?.blobPublicAccess) { throw new Error("Cannot provide public url for a non-public bucket"); } - if (!(await this.exists(key))) { - throw new Error( - `Cannot provide public url for a non-existent key (key=${key})` - ); - } return encodeURI( `https://${this.storageAccount}.blob.core.windows.net/${this.bucketName}/${key}` diff --git a/packages/@winglang/sdk/src/shared-gcp/bucket.inflight.ts b/packages/@winglang/sdk/src/shared-gcp/bucket.inflight.ts index cc8dad81922..8ec0226652f 100644 --- a/packages/@winglang/sdk/src/shared-gcp/bucket.inflight.ts +++ b/packages/@winglang/sdk/src/shared-gcp/bucket.inflight.ts @@ -228,12 +228,6 @@ export class BucketClient implements IBucketClient { throw new Error("Cannot provide public url for a non-public bucket"); } - if (!(await this.exists(key))) { - throw new Error( - `Cannot provide public url for a non-existent key (key=${key})` - ); - } - return `https://storage.googleapis.com/${this.bucketName}/${key}`; } @@ -257,11 +251,6 @@ export class BucketClient implements IBucketClient { // Set the GCS action switch (action) { case BucketSignedUrlAction.DOWNLOAD: - if (!(await this.exists(key))) { - throw new Error( - `Cannot provide signed url for a non-existent key (key=${key})` - ); - } gcsAction = "read"; break; case BucketSignedUrlAction.UPLOAD: diff --git a/packages/@winglang/sdk/src/target-sim/bucket.inflight.ts b/packages/@winglang/sdk/src/target-sim/bucket.inflight.ts index 35fa67ccd5c..17d4c67eb2d 100644 --- a/packages/@winglang/sdk/src/target-sim/bucket.inflight.ts +++ b/packages/@winglang/sdk/src/target-sim/bucket.inflight.ts @@ -2,7 +2,6 @@ import * as crypto from "crypto"; import * as fs from "fs"; import { Server } from "http"; import { dirname, join } from "path"; -import { pathToFileURL } from "url"; import express from "express"; import mime from "mime-types"; import { BucketAttributes, BucketSchema } from "./schema-resources"; @@ -141,22 +140,24 @@ export class Bucket implements IBucketClient, ISimulatorResourceInstance { return; }); - // Handle signed URL downloads. + // Handle file accesses and signed URL downloads. this.app.get("*", (req, res) => { const action = req.query.action; - if (action !== BucketSignedUrlAction.DOWNLOAD) { - return res.status(403).send("Operation not allowed"); - } + if (!this._public) { + if (action !== BucketSignedUrlAction.DOWNLOAD) { + return res.status(403).send("Operation not allowed"); + } - const validUntil = req.query.validUntil?.toString(); - if (!validUntil || Date.now() > parseInt(validUntil)) { - return res.status(403).send("Signed URL has expired"); + const validUntil = req.query.validUntil?.toString(); + if (!validUntil || Date.now() > parseInt(validUntil)) { + return res.status(403).send("Signed URL has expired"); + } } const key = req.path.slice(1); // remove leading slash const hash = this.hashKey(key); const filename = join(this._fileDir, hash); - return res.download(filename); + return res.sendFile(filename); }); } @@ -458,15 +459,7 @@ export class Bucket implements IBucketClient, ISimulatorResourceInstance { return this.context.withTrace({ message: `Public URL (key=${key}).`, activity: async () => { - const filePath = join(this._fileDir, key); - - if (!this._metadata.has(key)) { - throw new Error( - `Cannot provide public url for an non-existent key (key=${key})` - ); - } - - return pathToFileURL(filePath).href; + return new URL(key, this.url).toString(); }, }); } @@ -480,15 +473,6 @@ export class Bucket implements IBucketClient, ISimulatorResourceInstance { // a POJO with seconds, but TypeScript thinks otherwise. const duration = options?.duration?.seconds ?? 900; - if ( - action === BucketSignedUrlAction.DOWNLOAD && - !(await this.exists(key)) - ) { - throw new Error( - `Cannot provide signed url for a non-existent key (key=${key})` - ); - } - const url = new URL(key, this.url); url.searchParams.set("action", action); url.searchParams.set( diff --git a/packages/@winglang/sdk/test/shared-aws/bucket.inflight.test.ts b/packages/@winglang/sdk/test/shared-aws/bucket.inflight.test.ts index 5ecc4b375ce..acbe7d68ac7 100644 --- a/packages/@winglang/sdk/test/shared-aws/bucket.inflight.test.ts +++ b/packages/@winglang/sdk/test/shared-aws/bucket.inflight.test.ts @@ -254,38 +254,6 @@ test("Given a non public bucket when reaching to a key public url it should thro ); }); -test("Given a public bucket when reaching to a non-existent key, public url it should throw an error", async () => { - // GIVEN - let error; - const BUCKET_NAME = "BUCKET_NAME"; - const KEY = "KEY"; - - s3Mock.on(GetPublicAccessBlockCommand, { Bucket: BUCKET_NAME }).resolves({ - PublicAccessBlockConfiguration: { - BlockPublicAcls: false, - BlockPublicPolicy: false, - RestrictPublicBuckets: false, - IgnorePublicAcls: false, - }, - }); - s3Mock - .on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }) - .rejects(new NotFound({ message: "Object not found", $metadata: {} })); - - // WHEN - const client = new BucketClient({ $bucketName: BUCKET_NAME }); - try { - await client.publicUrl(KEY); - } catch (err) { - error = err; - } - - // THEN - expect(error?.message).toBe( - "Cannot provide public url for a non-existent key (key=KEY)" - ); -}); - test("Given a public bucket, when giving one of its keys, we should get its public url", async () => { // GIVEN const BUCKET_NAME = "BUCKET_NAME"; @@ -557,30 +525,6 @@ test("tryDelete a non-existent object from the bucket", async () => { expect(objectTryDelete).toEqual(false); }); -test("Given a bucket when reaching to a non-existent key, signed url it should throw an error", async () => { - // GIVEN - let error; - const BUCKET_NAME = "BUCKET_NAME"; - const KEY = "KEY"; - - s3Mock - .on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }) - .rejects(new NotFound({ message: "Object not found", $metadata: {} })); - - // WHEN - const client = new BucketClient({ $bucketName: BUCKET_NAME }); - try { - await client.signedUrl(KEY); - } catch (err) { - error = err; - } - - // THEN - expect(error?.message).toBe( - `Cannot provide signed url for a non-existent key (key=${KEY})` - ); -}); - // Skipped due to issue with mocking getSignedUrl: // https://github.com/m-radzikowski/aws-sdk-client-mock/issues/62 test.skip("Given a bucket, when giving one of its keys, we should get its signed url", async () => { diff --git a/packages/@winglang/sdk/test/shared-azure/bucket.inflight.test.ts b/packages/@winglang/sdk/test/shared-azure/bucket.inflight.test.ts index 09e117b1438..6b14916c0cc 100644 --- a/packages/@winglang/sdk/test/shared-azure/bucket.inflight.test.ts +++ b/packages/@winglang/sdk/test/shared-azure/bucket.inflight.test.ts @@ -409,31 +409,6 @@ test("Given a non public bucket when reaching to a key public url it should thro ); }); -test("Given a public bucket when reaching to a non existent key, public url it should throw an error", async () => { - // GIVEN - const BUCKET_NAME = "BUCKET_NAME"; - const STORAGE_NAME = "STORAGE_NAME"; - const KEY = "non-existent-key"; - - // WHEN - const client = new BucketClient({ - $bucketName: BUCKET_NAME, - $storageAccount: STORAGE_NAME, - $blobServiceClient: mockBlobServiceClient, - }); - TEST_PATH = "sad"; - // @ts-expect-error - accessing private property - client.containerClient.getAccessPolicy = vi - .fn() - .mockResolvedValue({ blobPublicAccess: "container" }); - client.exists = vi.fn().mockResolvedValue(false); - - // THEN - await expect(() => client.publicUrl(KEY)).rejects.toThrowError( - `Cannot provide public url for a non-existent key (key=${KEY})` - ); -}); - test("Given a public bucket, when giving one of its keys, we should get its public url", async () => { // GIVEN const BUCKET_NAME = "BUCKET_NAME"; diff --git a/packages/@winglang/sdk/test/target-sim/bucket.test.ts b/packages/@winglang/sdk/test/target-sim/bucket.test.ts index 01085cd25f0..eadde50a276 100644 --- a/packages/@winglang/sdk/test/target-sim/bucket.test.ts +++ b/packages/@winglang/sdk/test/target-sim/bucket.test.ts @@ -574,7 +574,7 @@ test("Given a non public bucket when reaching to a key public url it should thro await s.stop(); }); -test("Given a public bucket when reaching to a non existent key, public url it should throw an error", async () => { +test("Given a public bucket, when giving one of its keys, we should get its public url", async () => { // GIVEN const app = new SimApp(); new cloud.Bucket(app, "my_bucket", { public: true }); @@ -583,15 +583,18 @@ test("Given a public bucket when reaching to a non existent key, public url it s const client = s.getResource("/my_bucket") as cloud.IBucketClient; const KEY = "KEY"; + const VALUE = "VALUE"; + + // WHEN + await client.put(KEY, VALUE); + const response = await client.publicUrl(KEY); // THEN - await expect(() => client.publicUrl(KEY)).rejects.toThrowError( - /Cannot provide public url for an non-existent key/ - ); await s.stop(); + expect(response).toMatch(/https?:\/\/(localhost|127\.0\.0\.1):\d+\/KEY/); }); -test("Given a public bucket, when giving one of its keys, we should get its public url", async () => { +test("accessing the publicUrl of a valid key should return the file contents", async () => { // GIVEN const app = new SimApp(); new cloud.Bucket(app, "my_bucket", { public: true }); @@ -604,12 +607,31 @@ test("Given a public bucket, when giving one of its keys, we should get its publ // WHEN await client.put(KEY, VALUE); - const response = await client.publicUrl(KEY); + const publicUrl = await client.publicUrl(KEY); // THEN + const response = await fetch(publicUrl); + const text = await response.text(); + expect(text).toBe(VALUE); +}); + +test("accessing the publicUrl of a non existent key should throw an error", async () => { + // GIVEN + const app = new SimApp(); + new cloud.Bucket(app, "my_bucket", { public: true }); + + const s = await app.startSimulator(); + const client = s.getResource("/my_bucket") as cloud.IBucketClient; + + const KEY = "KEY"; + + // WHEN + const publicUrl = await client.publicUrl(KEY); + const response = await fetch(publicUrl); + + // THEN + expect(response.status).toBe(404); await s.stop(); - // file paths are different on windows and linux - expect(response.endsWith("KEY")).toBe(true); }); test("check if an object exists in the bucket", async () => { @@ -994,51 +1016,3 @@ test("signedUrl doesn't accept multipart uploads yet", async () => { await s.stop(); }); - -// Deceided to seperate this feature in a different release,(see https://github.com/winglang/wing/issues/4143) - -// test("Given a bucket when reaching to a non existent key, signed url it should throw an error", async () => { -// //GIVEN -// let error; -// const app = new SimApp(); -// new cloud.Bucket(app, "my_bucket", { public: true }); - -// const s = await app.startSimulator(); -// const client = s.getResource("/my_bucket") as cloud.IBucketClient; - -// const KEY = "KEY"; - -// // WHEN -// try { -// await client.signedUrl(KEY); -// } catch (err) { -// error = err; -// } - -// expect(error?.message).toBe( -// "Cannot provide signed url for an non-existent key (key=${KEY})" -// ); -// // THEN -// await s.stop(); -// }); - -// test("Given a bucket, when giving one of its keys, we should get it's signed url", async () => { -// // GIVEN -// const app = new SimApp(); -// new cloud.Bucket(app, "my_bucket", { public: true }); - -// const s = await app.startSimulator(); -// const client = s.getResource("/my_bucket") as cloud.IBucketClient; - -// const KEY = "KEY"; -// const VALUE = "VALUE"; - -// // WHEN -// await client.put(KEY, VALUE); -// const response = await client.signedUrl(KEY); - -// // THEN -// await s.stop(); -// const filePath = `${client.fileDir}/${KEY}`; -// expect(response).toEqual(url.pathToFileURL(filePath).searchParams.append("Expires","86400")); -// }); diff --git a/tests/sdk_tests/bucket/public_url.test.w b/tests/sdk_tests/bucket/public_url.test.w index e5ecb634b99..5fb99a28ba1 100644 --- a/tests/sdk_tests/bucket/public_url.test.w +++ b/tests/sdk_tests/bucket/public_url.test.w @@ -25,11 +25,7 @@ test "publicUrl" { let publicUrl = publicBucket.publicUrl("file1.txt"); assert(publicUrl != ""); - - // TODO: works in aws, doesn't work in sim since publicUrl is returning a path to the file, remove condition when #2833 is resolved. - if (util.env("WING_TARGET") != "sim") { - assert(http.get(publicUrl).body == "Foo"); - } + assert(http.get(publicUrl).body == "Foo"); assertThrows(BUCKET_NOT_PUBLIC_ERROR, () => { privateBucket.publicUrl("file2.txt"); diff --git a/tests/sdk_tests/bucket/signed_url.test.w b/tests/sdk_tests/bucket/signed_url.test.w index 3b36b3eb0c7..8c31d91acfa 100644 --- a/tests/sdk_tests/bucket/signed_url.test.w +++ b/tests/sdk_tests/bucket/signed_url.test.w @@ -33,25 +33,6 @@ test "signedUrl GET (explicit)" { expect.equal(output, VALUE); } -test "signedUrl GET with non-existent key" { - let assertThrows = (expected: str, block: (): void) => { - let var error = false; - try { - block(); - } catch actual { - expect.equal(actual, expected); - error = true; - } - expect.equal(error, true); - }; - let UNEXISTING_KEY = "no-such-file.txt"; - let OBJECT_DOES_NOT_EXIST_ERROR = "Cannot provide signed url for a non-existent key (key={UNEXISTING_KEY})"; - - assertThrows(OBJECT_DOES_NOT_EXIST_ERROR, () => { - bucket.signedUrl(UNEXISTING_KEY); - }); -} - test "signedUrl PUT" { let KEY = "tempfile.txt"; let VALUE = "Hello, Wing!"; diff --git a/tools/hangar/__snapshots__/test_corpus/sdk_tests/bucket/signed_url.test.w_test_sim.md b/tools/hangar/__snapshots__/test_corpus/sdk_tests/bucket/signed_url.test.w_test_sim.md index 612e92db068..967e3d909a5 100644 --- a/tools/hangar/__snapshots__/test_corpus/sdk_tests/bucket/signed_url.test.w_test_sim.md +++ b/tools/hangar/__snapshots__/test_corpus/sdk_tests/bucket/signed_url.test.w_test_sim.md @@ -5,10 +5,9 @@ pass ─ signed_url.test.wsim » root/Default/test:signedUrl duration option is respected pass ─ signed_url.test.wsim » root/Default/test:signedUrl GET (explicit) pass ─ signed_url.test.wsim » root/Default/test:signedUrl GET (implicit) -pass ─ signed_url.test.wsim » root/Default/test:signedUrl GET with non-existent key pass ─ signed_url.test.wsim » root/Default/test:signedUrl PUT -Tests 5 passed (5) +Tests 4 passed (4) Snapshots 1 skipped Test Files 1 passed (1) Duration