fix: check for nonempty kv id and r2 bucket_name (#1667)

* fix: check for nonempty kv id and r2 bucket_name * chore: run prettier * chore: add changeset
cloudflare · Aug 22, 2022 · ba6451d · ba6451d
1 parent 46b73b5
commit ba6451d
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 10 deletions.
diff --git a/.changeset/gold-sheep-repeat.md b/.changeset/gold-sheep-repeat.md
@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+fix: check for nonempty kv id and r2 bucket_name
diff --git a/packages/wrangler/src/__tests__/configuration.test.ts b/packages/wrangler/src/__tests__/configuration.test.ts
@@ -1635,6 +1635,7 @@ describe("normalizeAndValidateConfig()", () => {
 								id: "KV_ID_2",
 								preview_id: 2222,
 							},
+							{ binding: "VALID", id: "" },
 						],
 					} as unknown as RawConfig,
 					undefined,
@@ -1654,7 +1655,8 @@ describe("normalizeAndValidateConfig()", () => {
 			            - \\"kv_namespaces[1]\\" bindings should have a string \\"id\\" field but got {\\"binding\\":\\"VALID\\"}.
 			            - \\"kv_namespaces[2]\\" bindings should have a string \\"binding\\" field but got {\\"binding\\":2000,\\"id\\":2111}.
 			            - \\"kv_namespaces[2]\\" bindings should have a string \\"id\\" field but got {\\"binding\\":2000,\\"id\\":2111}.
-			            - \\"kv_namespaces[3]\\" bindings should, optionally, have a string \\"preview_id\\" field but got {\\"binding\\":\\"KV_BINDING_2\\",\\"id\\":\\"KV_ID_2\\",\\"preview_id\\":2222}."
+			            - \\"kv_namespaces[3]\\" bindings should, optionally, have a string \\"preview_id\\" field but got {\\"binding\\":\\"KV_BINDING_2\\",\\"id\\":\\"KV_ID_2\\",\\"preview_id\\":2222}.
+			            - \\"kv_namespaces[4]\\" bindings should have a string \\"id\\" field but got {\\"binding\\":\\"VALID\\",\\"id\\":\\"\\"}."
 		        `);
 			});
 		});
@@ -1740,6 +1742,7 @@ describe("normalizeAndValidateConfig()", () => {
 								bucket_name: "R2_BUCKET_2",
 								preview_bucket_name: 2555,
 							},
+							{ binding: "R2_BINDING_1", bucket_name: "" },
 						],
 					} as unknown as RawConfig,
 					undefined,
@@ -1759,7 +1762,8 @@ describe("normalizeAndValidateConfig()", () => {
 			            - \\"r2_buckets[1]\\" bindings should have a string \\"bucket_name\\" field but got {\\"binding\\":\\"R2_BINDING_1\\"}.
 			            - \\"r2_buckets[2]\\" bindings should have a string \\"binding\\" field but got {\\"binding\\":2333,\\"bucket_name\\":2444}.
 			            - \\"r2_buckets[2]\\" bindings should have a string \\"bucket_name\\" field but got {\\"binding\\":2333,\\"bucket_name\\":2444}.
-			            - \\"r2_buckets[3]\\" bindings should, optionally, have a string \\"preview_bucket_name\\" field but got {\\"binding\\":\\"R2_BINDING_2\\",\\"bucket_name\\":\\"R2_BUCKET_2\\",\\"preview_bucket_name\\":2555}."
+			            - \\"r2_buckets[3]\\" bindings should, optionally, have a string \\"preview_bucket_name\\" field but got {\\"binding\\":\\"R2_BINDING_2\\",\\"bucket_name\\":\\"R2_BUCKET_2\\",\\"preview_bucket_name\\":2555}.
+			            - \\"r2_buckets[4]\\" bindings should have a string \\"bucket_name\\" field but got {\\"binding\\":\\"R2_BINDING_1\\",\\"bucket_name\\":\\"\\"}."
 		        `);
 			});
 		});

diff --git a/packages/wrangler/src/config/validation-helpers.ts b/packages/wrangler/src/config/validation-helpers.ts
@@ -383,7 +383,7 @@ export const validateRequiredProperty = (
 	container: string,
 	key: string,
 	value: unknown,
-	type: string,
+	type: TypeofType,
 	choices?: unknown[]
 ): boolean => {
 	if (container) {
@@ -417,7 +417,7 @@ export const validateOptionalProperty = (
 	container: string,
 	key: string,
 	value: unknown,
-	type: string,
+	type: TypeofType,
 	choices?: unknown[]
 ): boolean => {
 	if (value !== undefined) {
@@ -440,7 +440,7 @@ export const validateTypedArray = (
 	diagnostics: Diagnostics,
 	container: string,
 	value: unknown,
-	type: string
+	type: TypeofType
 ): boolean => {
 	let isValid = true;
 	if (!Array.isArray(value)) {
@@ -472,7 +472,7 @@ export const validateOptionalTypedArray = (
 	diagnostics: Diagnostics,
 	container: string,
 	value: unknown,
-	type: string
+	type: TypeofType
 ) => {
 	if (value !== undefined) {
 		return validateTypedArray(diagnostics, container, value, type);
@@ -486,7 +486,7 @@ export const validateOptionalTypedArray = (
 export const isRequiredProperty = <T extends object>(
 	obj: object,
 	prop: keyof T,
-	type: string,
+	type: TypeofType,
 	choices?: unknown[]
 ): obj is T =>
 	hasProperty<T>(obj, prop) &&
@@ -499,7 +499,7 @@ export const isRequiredProperty = <T extends object>(
 export const isOptionalProperty = <T extends object>(
 	obj: object,
 	prop: keyof T,
-	type: string
+	type: TypeofType
 ): obj is T => !hasProperty<T>(obj, prop) || typeof obj[prop] === type;
 
 /**
@@ -582,3 +582,16 @@ const isRecord = (
 	value: unknown
 ): value is Record<string | number | symbol, unknown> =>
 	typeof value === "object" && value !== null && !Array.isArray(value);
+
+/**
+ * JavaScript `typeof` operator return values.
+ */
+type TypeofType =
+	| "string"
+	| "number"
+	| "bigint"
+	| "boolean"
+	| "symbol"
+	| "undefined"
+	| "object"
+	| "function";
diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts
@@ -1630,7 +1630,10 @@ const validateKVBinding: ValidatorFn = (diagnostics, field, value) => {
 		);
 		isValid = false;
 	}
-	if (!isRequiredProperty(value, "id", "string")) {
+	if (
+		!isRequiredProperty(value, "id", "string") ||
+		(value as { id: string }).id.length === 0
+	) {
 		diagnostics.errors.push(
 			`"${field}" bindings should have a string "id" field but got ${JSON.stringify(
 				value
@@ -1668,7 +1671,10 @@ const validateR2Binding: ValidatorFn = (diagnostics, field, value) => {
 		);
 		isValid = false;
 	}
-	if (!isRequiredProperty(value, "bucket_name", "string")) {
+	if (
+		!isRequiredProperty(value, "bucket_name", "string") ||
+		(value as { bucket_name: string }).bucket_name.length === 0
+	) {
 		diagnostics.errors.push(
 			`"${field}" bindings should have a string "bucket_name" field but got ${JSON.stringify(
 				value