From e43a7df4b6738d6dd3168c22a56ad727b3dbc6a5 Mon Sep 17 00:00:00 2001 From: Doctor <44320105+BlackAsLight@users.noreply.github.com> Date: Mon, 30 Sep 2024 13:31:41 +1000 Subject: [PATCH] fix(tar): ignore non-tar file portion of a stream in `UntarStream` (#6064) --- tar/tar_stream_test.ts | 5 +- tar/untar_stream.ts | 64 +++++++++-------- tar/untar_stream_test.ts | 145 +++++++++++++++++++-------------------- 3 files changed, 108 insertions(+), 106 deletions(-) diff --git a/tar/tar_stream_test.ts b/tar/tar_stream_test.ts index adf872956..acd2ff8a3 100644 --- a/tar/tar_stream_test.ts +++ b/tar/tar_stream_test.ts @@ -1,12 +1,13 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +import { assertEquals, assertRejects, assertThrows } from "@std/assert"; +import { concat } from "@std/bytes"; import { assertValidTarStreamOptions, TarStream, type TarStreamInput, } from "./tar_stream.ts"; -import { assertEquals, assertRejects, assertThrows } from "../assert/mod.ts"; import { UntarStream } from "./untar_stream.ts"; -import { concat } from "../bytes/mod.ts"; Deno.test("TarStream() with default stream", async () => { const text = new TextEncoder().encode("Hello World!"); diff --git a/tar/untar_stream.ts b/tar/untar_stream.ts index 197436622..bdfdde31b 100644 --- a/tar/untar_stream.ts +++ b/tar/untar_stream.ts @@ -176,7 +176,8 @@ export class UntarStream implements TransformStream { #readable: ReadableStream; #writable: WritableStream; - #gen: AsyncGenerator; + #reader: ReadableStreamDefaultReader; + #buffer: Uint8Array[] = []; #lock = false; constructor() { const { readable, writable } = new TransformStream< @@ -185,43 +186,50 @@ export class UntarStream >(); this.#readable = ReadableStream.from(this.#untar()); this.#writable = writable; + this.#reader = readable.pipeThrough(new FixedChunkStream(512)).getReader(); + } - this.#gen = async function* () { - const buffer: Uint8Array[] = []; - for await ( - const chunk of readable.pipeThrough(new FixedChunkStream(512)) - ) { - if (chunk.length !== 512) { - throw new RangeError( - `Cannot extract the tar archive: The tarball chunk has an unexpected number of bytes (${chunk.length})`, - ); - } + async #read(): Promise { + const { done, value } = await this.#reader.read(); + if (done) return undefined; + if (value.length !== 512) { + throw new RangeError( + `Cannot extract the tar archive: The tarball chunk has an unexpected number of bytes (${value.length})`, + ); + } + this.#buffer.push(value); + return this.#buffer.shift(); + } - buffer.push(chunk); - if (buffer.length > 2) yield buffer.shift()!; - } - if (buffer.length < 2) { + async *#untar(): AsyncGenerator { + for (let i = 0; i < 2; ++i) { + const { done, value } = await this.#reader.read(); + if (done || value.length !== 512) { throw new RangeError( "Cannot extract the tar archive: The tarball is too small to be valid", ); } - if (!buffer.every((value) => value.every((x) => x === 0))) { - throw new TypeError( - "Cannot extract the tar archive: The tarball has invalid ending", - ); - } - }(); - } - - async *#untar(): AsyncGenerator { + this.#buffer.push(value); + } const decoder = new TextDecoder(); while (true) { while (this.#lock) { await new Promise((resolve) => setTimeout(resolve, 0)); } - const { done, value } = await this.#gen.next(); - if (done) break; + // Check for premature ending + if (this.#buffer.every((value) => value.every((x) => x === 0))) { + await this.#reader.cancel("Tar stream finished prematurely"); + return; + } + + const value = await this.#read(); + if (value == undefined) { + if (this.#buffer.every((value) => value.every((x) => x === 0))) break; + throw new TypeError( + "Cannot extract the tar archive: The tarball has invalid ending", + ); + } // Validate Checksum const checksum = parseInt( @@ -286,8 +294,8 @@ export class UntarStream async *#genFile(size: number): AsyncGenerator { for (let i = Math.ceil(size / 512); i > 0; --i) { - const { done, value } = await this.#gen.next(); - if (done) { + const value = await this.#read(); + if (value == undefined) { throw new SyntaxError( "Cannot extract the tar archive: Unexpected end of Tarball", ); diff --git a/tar/untar_stream_test.ts b/tar/untar_stream_test.ts index a89f04c4a..7c6c39d05 100644 --- a/tar/untar_stream_test.ts +++ b/tar/untar_stream_test.ts @@ -1,12 +1,13 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -import { concat } from "../bytes/mod.ts"; + +import { assertEquals, assertRejects } from "@std/assert"; +import { toBytes } from "@std/streams/unstable-to-bytes"; import { TarStream, type TarStreamInput } from "./tar_stream.ts"; import { type OldStyleFormat, type PosixUstarFormat, UntarStream, } from "./untar_stream.ts"; -import { assertEquals, assertRejects } from "../assert/mod.ts"; Deno.test("expandTarArchiveCheckingHeaders", async () => { const text = new TextEncoder().encode("Hello World!"); @@ -39,9 +40,9 @@ Deno.test("expandTarArchiveCheckingHeaders", async () => { .pipeThrough(new UntarStream()); const headers: (OldStyleFormat | PosixUstarFormat)[] = []; - for await (const item of readable) { - headers.push(item.header); - await item.readable?.cancel(); + for await (const entry of readable) { + headers.push(entry.header); + await entry.readable?.cancel(); } assertEquals(headers, [{ name: "./potato", @@ -98,9 +99,7 @@ Deno.test("expandTarArchiveCheckingBodies", async () => { let buffer = new Uint8Array(); for await (const item of readable) { - if (item.readable) { - buffer = concat(await Array.fromAsync(item.readable)); - } + if (item.readable) buffer = await toBytes(item.readable); } assertEquals(buffer, text); }); @@ -125,59 +124,47 @@ Deno.test("UntarStream() with size equals to multiple of 512", async () => { let buffer = new Uint8Array(); for await (const entry of readable) { - if (entry.readable) { - buffer = concat(await Array.fromAsync(entry.readable)); - } + if (entry.readable) buffer = await toBytes(entry.readable); } assertEquals(buffer, data); }); Deno.test("UntarStream() with invalid size", async () => { - const readable = ReadableStream.from([ - { - type: "file", - path: "newFile.txt", - size: 512, - readable: ReadableStream.from([new Uint8Array(512).fill(97)]), - }, - ]) - .pipeThrough(new TarStream()) - .pipeThrough( - new TransformStream({ - flush(controller) { - controller.enqueue(new Uint8Array(100)); - }, - }), - ) + const bytes = (await toBytes( + ReadableStream.from([ + { + type: "file", + path: "newFile.txt", + size: 512, + readable: ReadableStream.from([new Uint8Array(512).fill(97)]), + }, + ]) + .pipeThrough(new TarStream()), + )).slice(0, -100); + + const readable = ReadableStream.from([bytes]) .pipeThrough(new UntarStream()); await assertRejects( async () => { - for await (const entry of readable) { - if (entry.readable) { - // deno-lint-ignore no-empty - for await (const _ of entry.readable) {} - } - } + for await (const entry of readable) await entry.readable?.cancel(); }, RangeError, - "Cannot extract the tar archive: The tarball chunk has an unexpected number of bytes (100)", + "Cannot extract the tar archive: The tarball chunk has an unexpected number of bytes (412)", ); }); Deno.test("UntarStream() with invalid ending", async () => { - const tarBytes = concat( - await Array.fromAsync( - ReadableStream.from([ - { - type: "file", - path: "newFile.txt", - size: 512, - readable: ReadableStream.from([new Uint8Array(512).fill(97)]), - }, - ]) - .pipeThrough(new TarStream()), - ), + const tarBytes = await toBytes( + ReadableStream.from([ + { + type: "file", + path: "newFile.txt", + size: 512, + readable: ReadableStream.from([new Uint8Array(512).fill(97)]), + }, + ]) + .pipeThrough(new TarStream()), ); tarBytes[tarBytes.length - 1] = 1; @@ -186,12 +173,7 @@ Deno.test("UntarStream() with invalid ending", async () => { await assertRejects( async () => { - for await (const entry of readable) { - if (entry.readable) { - // deno-lint-ignore no-empty - for await (const _ of entry.readable) {} - } - } + for await (const entry of readable) await entry.readable?.cancel(); }, TypeError, "Cannot extract the tar archive: The tarball has invalid ending", @@ -204,12 +186,7 @@ Deno.test("UntarStream() with too small size", async () => { await assertRejects( async () => { - for await (const entry of readable) { - if (entry.readable) { - // deno-lint-ignore no-empty - for await (const _ of entry.readable) {} - } - } + for await (const entry of readable) await entry.readable?.cancel(); }, RangeError, "Cannot extract the tar archive: The tarball is too small to be valid", @@ -217,18 +194,16 @@ Deno.test("UntarStream() with too small size", async () => { }); Deno.test("UntarStream() with invalid checksum", async () => { - const tarBytes = concat( - await Array.fromAsync( - ReadableStream.from([ - { - type: "file", - path: "newFile.txt", - size: 512, - readable: ReadableStream.from([new Uint8Array(512).fill(97)]), - }, - ]) - .pipeThrough(new TarStream()), - ), + const tarBytes = await toBytes( + ReadableStream.from([ + { + type: "file", + path: "newFile.txt", + size: 512, + readable: ReadableStream.from([new Uint8Array(512).fill(97)]), + }, + ]) + .pipeThrough(new TarStream()), ); tarBytes[148] = 97; @@ -237,14 +212,32 @@ Deno.test("UntarStream() with invalid checksum", async () => { await assertRejects( async () => { - for await (const entry of readable) { - if (entry.readable) { - // deno-lint-ignore no-empty - for await (const _ of entry.readable) {} - } - } + for await (const entry of readable) await entry.readable?.cancel(); }, Error, "Cannot extract the tar archive: An archive entry has invalid header checksum", ); }); + +Deno.test("UntarStream() with extra bytes", async () => { + const readable = ReadableStream.from([ + { + type: "directory", + path: "a", + }, + ]) + .pipeThrough(new TarStream()) + .pipeThrough( + new TransformStream({ + flush(controller) { + controller.enqueue(new Uint8Array(512 * 2).fill(1)); + }, + }), + ) + .pipeThrough(new UntarStream()); + + for await (const entry of readable) { + assertEquals(entry.path, "a"); + entry.readable?.cancel(); + } +});