From 4f53dade87f5a0b9e3400e0db81850e14f0f62cd Mon Sep 17 00:00:00 2001 From: Zefir Kirilov Date: Thu, 26 Sep 2024 15:39:45 +0300 Subject: [PATCH 1/7] validate boundary when constructing Multipart Based RFC 2046, Section 5.1.1. --- src/Multipart.ts | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/Multipart.ts b/src/Multipart.ts index 2884618..9629712 100644 --- a/src/Multipart.ts +++ b/src/Multipart.ts @@ -59,13 +59,58 @@ export class Multipart implements Part { * @param parts The parts to include in the multipart * @param [boundary] The multipart boundary used to separate the parts. Randomly generated if not provided * @param [mediaType] The media type of the multipart. Defaults to "multipart/mixed" + * + * @throws {RangeError} If the boundary is invalid. A valid boundary is 1 to 70 characters long, does not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space */ public constructor(public readonly parts: Part[], boundary: Uint8Array | string = crypto.randomUUID(), mediaType: string = "multipart/mixed") { this.#boundary = typeof boundary === "string" ? new TextEncoder().encode(boundary) : boundary; + if (!Multipart.isValidBoundary(this.#boundary)) + throw new RangeError("Boundary must be 1 to 70 characters long, not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space"); this.#mediaType = mediaType; this.setHeaders(); } + /** + * Check if the boundary is valid + * A valid boundary is 1 to 70 characters long, does not end with space, and may only contain: + * A-Z a-z 0-9 '()+_,-./:=? and space + * + * ```bnf + * boundary := 0*69 bcharsnospace + * + * bchars := bcharsnospace / " " + * + * bcharsnospace := DIGIT / ALPHA / "'" / "(" / ")" / + * "+" / "_" / "," / "-" / "." / + * "/" / ":" / "=" / "?" + * ``` + * + * From: RFC 2046, Section 5.1.1. Common Syntax + * + * @internal + */ + private static isValidBoundary(boundary: Uint8Array): boolean { + if (boundary.length < 1 || boundary.length > 70 || boundary[boundary.length - 1] === Multipart.SP) + return false; + + for (const char of boundary) { + if (char >= 0x30 && char <= 0x39) continue; + if ((char >= 0x41 && char <= 0x5a) || (char >= 0x61 && char <= 0x7a)) continue; + if ( + char === Multipart.SP || + (char >= 0x27 && char <= 0x29) || + (char >= 0x2b && char <= 0x2f) || + char === 0x3a || + char === 0x3d || + char === 0x3f || + char === 0x5f + ) continue; + return false; + } + + return true; + } + /** * The boundary bytes used to separate the parts */ From 728c5e7be5087f602960cc185c63fa497afc5baa Mon Sep 17 00:00:00 2001 From: Zefir Kirilov Date: Thu, 26 Sep 2024 15:45:20 +0300 Subject: [PATCH 2/7] also validate multipart boundary when changed --- src/Multipart.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Multipart.ts b/src/Multipart.ts index 9629712..96fce94 100644 --- a/src/Multipart.ts +++ b/src/Multipart.ts @@ -120,6 +120,8 @@ export class Multipart implements Part { public set boundary(boundary: Uint8Array | string) { this.#boundary = typeof boundary === "string" ? new TextEncoder().encode(boundary) : boundary; + if (!Multipart.isValidBoundary(this.#boundary)) + throw new RangeError("Boundary must be 1 to 70 characters long, not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space"); this.setHeaders(); } From 26251127a8673fac6f957c4e246d0590dc028a5b Mon Sep 17 00:00:00 2001 From: Zefir Kirilov Date: Thu, 26 Sep 2024 15:50:51 +0300 Subject: [PATCH 3/7] update invalid boundary error message --- src/Multipart.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Multipart.ts b/src/Multipart.ts index 96fce94..256d93f 100644 --- a/src/Multipart.ts +++ b/src/Multipart.ts @@ -65,7 +65,7 @@ export class Multipart implements Part { public constructor(public readonly parts: Part[], boundary: Uint8Array | string = crypto.randomUUID(), mediaType: string = "multipart/mixed") { this.#boundary = typeof boundary === "string" ? new TextEncoder().encode(boundary) : boundary; if (!Multipart.isValidBoundary(this.#boundary)) - throw new RangeError("Boundary must be 1 to 70 characters long, not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space"); + throw new RangeError("Invalid boundary: must be 1 to 70 characters long, not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space"); this.#mediaType = mediaType; this.setHeaders(); } @@ -121,7 +121,7 @@ export class Multipart implements Part { public set boundary(boundary: Uint8Array | string) { this.#boundary = typeof boundary === "string" ? new TextEncoder().encode(boundary) : boundary; if (!Multipart.isValidBoundary(this.#boundary)) - throw new RangeError("Boundary must be 1 to 70 characters long, not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space"); + throw new RangeError("Invalid boundary: must be 1 to 70 characters long, not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space"); this.setHeaders(); } From e1ecdb5a2511d6a364b93eba7029140905f5ab66 Mon Sep 17 00:00:00 2001 From: Zefir Kirilov Date: Thu, 26 Sep 2024 16:02:34 +0300 Subject: [PATCH 4/7] multipart boundary validation tests --- test/Multipart.test.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/Multipart.test.js b/test/Multipart.test.js index e95572b..9f0128d 100644 --- a/test/Multipart.test.js +++ b/test/Multipart.test.js @@ -27,6 +27,30 @@ describe("Multipart", function () { expect(new TextDecoder().decode(multipart.boundary)).to.equal("empty-boundary"); expect(multipart.mediaType).to.equal("multipart/mixed"); }); + + it("should accept only valid boundaries", function () { + expect(() => new Multipart([], "")).to.throw(RangeError, "Invalid boundary"); + expect(() => new Multipart([], " ")).to.throw(RangeError, "Invalid boundary"); + expect(() => new Multipart([], "a ")).to.throw(RangeError, "Invalid boundary"); + expect(() => new Multipart([], "0123456789".repeat(7) + "0")).to.throw(RangeError, "Invalid boundary"); + expect(() => new Multipart([], "foo!bar")).to.throw(RangeError, "Invalid boundary"); + + expect(() => new Multipart([], "a")).to.not.throw(); + expect(() => new Multipart([], "0123456789".repeat(7))).to.not.throw(); + expect(() => new Multipart([], "foo bar")).to.not.throw(); + expect(() => new Multipart([], "foo'bar")).to.not.throw(); + expect(() => new Multipart([], "foo(bar")).to.not.throw(); + expect(() => new Multipart([], "foo)bar")).to.not.throw(); + expect(() => new Multipart([], "foo+bar")).to.not.throw(); + expect(() => new Multipart([], "foo_bar")).to.not.throw(); + expect(() => new Multipart([], "foo,bar")).to.not.throw(); + expect(() => new Multipart([], "foo-bar")).to.not.throw(); + expect(() => new Multipart([], "foo.bar")).to.not.throw(); + expect(() => new Multipart([], "foo/bar")).to.not.throw(); + expect(() => new Multipart([], "foo:bar")).to.not.throw(); + expect(() => new Multipart([], "foo=bar")).to.not.throw(); + expect(() => new Multipart([], "foo?bar")).to.not.throw(); + }); }); describe("parse", function () { From 919496dde07140a12f3fafe438dab8b8edb6e831 Mon Sep 17 00:00:00 2001 From: Zefir Kirilov Date: Thu, 26 Sep 2024 16:10:06 +0300 Subject: [PATCH 5/7] wrap doc comment --- src/Multipart.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Multipart.ts b/src/Multipart.ts index 256d93f..762c064 100644 --- a/src/Multipart.ts +++ b/src/Multipart.ts @@ -60,7 +60,8 @@ export class Multipart implements Part { * @param [boundary] The multipart boundary used to separate the parts. Randomly generated if not provided * @param [mediaType] The media type of the multipart. Defaults to "multipart/mixed" * - * @throws {RangeError} If the boundary is invalid. A valid boundary is 1 to 70 characters long, does not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space + * @throws {RangeError} If the boundary is invalid. A valid boundary is 1 to 70 characters long, + * does not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space */ public constructor(public readonly parts: Part[], boundary: Uint8Array | string = crypto.randomUUID(), mediaType: string = "multipart/mixed") { this.#boundary = typeof boundary === "string" ? new TextEncoder().encode(boundary) : boundary; From 6e19299a3b6e3420ac6d28ee7e5a80d18b39a26f Mon Sep 17 00:00:00 2001 From: Zefir Kirilov Date: Thu, 26 Sep 2024 16:10:28 +0300 Subject: [PATCH 6/7] define throws in multipart boundary setter doc comment --- src/Multipart.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Multipart.ts b/src/Multipart.ts index 762c064..65ccce3 100644 --- a/src/Multipart.ts +++ b/src/Multipart.ts @@ -113,12 +113,17 @@ export class Multipart implements Part { } /** - * The boundary bytes used to separate the parts + * Get the boundary bytes used to separate the parts */ public get boundary(): Uint8Array { return this.#boundary; } + /** + * Set the boundary bytes used to separate the parts + * @throws {RangeError} If the boundary is invalid. A valid boundary is 1 to 70 characters long, + * does not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space + */ public set boundary(boundary: Uint8Array | string) { this.#boundary = typeof boundary === "string" ? new TextEncoder().encode(boundary) : boundary; if (!Multipart.isValidBoundary(this.#boundary)) From e60c907b0702dabd4b5a259fbd35137228b21380 Mon Sep 17 00:00:00 2001 From: Zefir Kirilov Date: Thu, 26 Sep 2024 21:12:34 +0300 Subject: [PATCH 7/7] more relaxed boundary validation only throw error when building multipart body bytes show warning when parsing --- src/Multipart.ts | 21 ++++++++++-------- test/Multipart.test.js | 48 +++++++++++++++++++++--------------------- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/src/Multipart.ts b/src/Multipart.ts index 65ccce3..6828f9a 100644 --- a/src/Multipart.ts +++ b/src/Multipart.ts @@ -59,14 +59,9 @@ export class Multipart implements Part { * @param parts The parts to include in the multipart * @param [boundary] The multipart boundary used to separate the parts. Randomly generated if not provided * @param [mediaType] The media type of the multipart. Defaults to "multipart/mixed" - * - * @throws {RangeError} If the boundary is invalid. A valid boundary is 1 to 70 characters long, - * does not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space */ public constructor(public readonly parts: Part[], boundary: Uint8Array | string = crypto.randomUUID(), mediaType: string = "multipart/mixed") { this.#boundary = typeof boundary === "string" ? new TextEncoder().encode(boundary) : boundary; - if (!Multipart.isValidBoundary(this.#boundary)) - throw new RangeError("Invalid boundary: must be 1 to 70 characters long, not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space"); this.#mediaType = mediaType; this.setHeaders(); } @@ -121,13 +116,9 @@ export class Multipart implements Part { /** * Set the boundary bytes used to separate the parts - * @throws {RangeError} If the boundary is invalid. A valid boundary is 1 to 70 characters long, - * does not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space */ public set boundary(boundary: Uint8Array | string) { this.#boundary = typeof boundary === "string" ? new TextEncoder().encode(boundary) : boundary; - if (!Multipart.isValidBoundary(this.#boundary)) - throw new RangeError("Invalid boundary: must be 1 to 70 characters long, not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space"); this.setHeaders(); } @@ -149,8 +140,14 @@ export class Multipart implements Part { /** * Get the bytes of the body of this multipart. Includes all parts separated by the boundary. * Does not include the headers. + * + * @throws {RangeError} If the multipart boundary is invalid. A valid boundary is 1 to 70 characters long, + * does not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space */ public get body(): Uint8Array { + if (!Multipart.isValidBoundary(this.#boundary)) + throw new RangeError("Invalid boundary: must be 1 to 70 characters long, not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space"); + const result: ArrayLike[] = []; for (const part of this.parts) result.push(Multipart.DOUBLE_DASH, this.boundary, Multipart.CRLF, part.bytes(), Multipart.CRLF); result.push(Multipart.DOUBLE_DASH, this.boundary, Multipart.DOUBLE_DASH, Multipart.CRLF); @@ -180,6 +177,9 @@ export class Multipart implements Part { * @param [mediaType] Multipart media type to pass to the constructor */ public static parseBody(data: Uint8Array, boundary: Uint8Array, mediaType?: string): Multipart { + if (!Multipart.isValidBoundary(boundary)) + console.warn("Invalid boundary:", new TextDecoder().decode(boundary), "\nMust be 1 to 70 characters long, not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space"); + const parts: Uint8Array[] = []; const fullBoundarySequence = new Uint8Array(Multipart.combineArrays([Multipart.DOUBLE_DASH, boundary, Multipart.CRLF])); const endBoundarySequence = new Uint8Array(Multipart.combineArrays([Multipart.DOUBLE_DASH, boundary, Multipart.DOUBLE_DASH, Multipart.CRLF])); @@ -393,6 +393,9 @@ export class Multipart implements Part { /** * Get the bytes of the headers and {@link body} of this multipart. + * + * @throws {RangeError} If the multipart boundary is invalid. A valid boundary is 1 to 70 characters long, + * does not end with space, and may only contain: A-Z a-z 0-9 '()+_,-./:=? and space */ public bytes(): Uint8Array { const result: ArrayLike[] = []; diff --git a/test/Multipart.test.js b/test/Multipart.test.js index 9f0128d..7587eb6 100644 --- a/test/Multipart.test.js +++ b/test/Multipart.test.js @@ -27,30 +27,6 @@ describe("Multipart", function () { expect(new TextDecoder().decode(multipart.boundary)).to.equal("empty-boundary"); expect(multipart.mediaType).to.equal("multipart/mixed"); }); - - it("should accept only valid boundaries", function () { - expect(() => new Multipart([], "")).to.throw(RangeError, "Invalid boundary"); - expect(() => new Multipart([], " ")).to.throw(RangeError, "Invalid boundary"); - expect(() => new Multipart([], "a ")).to.throw(RangeError, "Invalid boundary"); - expect(() => new Multipart([], "0123456789".repeat(7) + "0")).to.throw(RangeError, "Invalid boundary"); - expect(() => new Multipart([], "foo!bar")).to.throw(RangeError, "Invalid boundary"); - - expect(() => new Multipart([], "a")).to.not.throw(); - expect(() => new Multipart([], "0123456789".repeat(7))).to.not.throw(); - expect(() => new Multipart([], "foo bar")).to.not.throw(); - expect(() => new Multipart([], "foo'bar")).to.not.throw(); - expect(() => new Multipart([], "foo(bar")).to.not.throw(); - expect(() => new Multipart([], "foo)bar")).to.not.throw(); - expect(() => new Multipart([], "foo+bar")).to.not.throw(); - expect(() => new Multipart([], "foo_bar")).to.not.throw(); - expect(() => new Multipart([], "foo,bar")).to.not.throw(); - expect(() => new Multipart([], "foo-bar")).to.not.throw(); - expect(() => new Multipart([], "foo.bar")).to.not.throw(); - expect(() => new Multipart([], "foo/bar")).to.not.throw(); - expect(() => new Multipart([], "foo:bar")).to.not.throw(); - expect(() => new Multipart([], "foo=bar")).to.not.throw(); - expect(() => new Multipart([], "foo?bar")).to.not.throw(); - }); }); describe("parse", function () { @@ -276,5 +252,29 @@ describe("Multipart", function () { expect(new TextDecoder().decode(bytes)).to.equal(new TextDecoder().decode(expectedBytes)); }); + + it("should accept only valid boundaries", function () { + expect(() => new Multipart([], "").bytes()).to.throw(RangeError, "Invalid boundary"); + expect(() => new Multipart([], " ").bytes()).to.throw(RangeError, "Invalid boundary"); + expect(() => new Multipart([], "a ").bytes()).to.throw(RangeError, "Invalid boundary"); + expect(() => new Multipart([], "0123456789".repeat(7) + "0").bytes()).to.throw(RangeError, "Invalid boundary"); + expect(() => new Multipart([], "foo!bar").bytes()).to.throw(RangeError, "Invalid boundary"); + + expect(() => new Multipart([], "a").bytes()).to.not.throw(); + expect(() => new Multipart([], "0123456789".repeat(7)).bytes()).to.not.throw(); + expect(() => new Multipart([], "foo bar").bytes()).to.not.throw(); + expect(() => new Multipart([], "foo'bar").bytes()).to.not.throw(); + expect(() => new Multipart([], "foo(bar").bytes()).to.not.throw(); + expect(() => new Multipart([], "foo)bar").bytes()).to.not.throw(); + expect(() => new Multipart([], "foo+bar").bytes()).to.not.throw(); + expect(() => new Multipart([], "foo_bar").bytes()).to.not.throw(); + expect(() => new Multipart([], "foo,bar").bytes()).to.not.throw(); + expect(() => new Multipart([], "foo-bar").bytes()).to.not.throw(); + expect(() => new Multipart([], "foo.bar").bytes()).to.not.throw(); + expect(() => new Multipart([], "foo/bar").bytes()).to.not.throw(); + expect(() => new Multipart([], "foo:bar").bytes()).to.not.throw(); + expect(() => new Multipart([], "foo=bar").bytes()).to.not.throw(); + expect(() => new Multipart([], "foo?bar").bytes()).to.not.throw(); + }); }); });