From c6effd4739ceef50b71d7d5049d55d6f4f7b8bb8 Mon Sep 17 00:00:00 2001 From: KnorpelSenf Date: Sat, 6 Nov 2021 17:04:18 +0100 Subject: [PATCH 1/9] Add protection against accidental stream reuse --- src/platform.deno.ts | 15 +++++++++++++-- src/platform.node.ts | 15 +++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/platform.deno.ts b/src/platform.deno.ts index 8590daea..1738ad3a 100644 --- a/src/platform.deno.ts +++ b/src/platform.deno.ts @@ -46,7 +46,8 @@ export const inputFileData = Symbol("InputFile data"); * Reference](https://core.telegram.org/bots/api#inputfile). */ export class InputFile { - public readonly [inputFileData]: ConstructorParameters[0]; + private consumed = false; + private readonly fileData: ConstructorParameters[0]; /** * Optional name of the constructed `InputFile` instance. * @@ -69,12 +70,22 @@ export class InputFile { | AsyncIterable, filename?: string, ) { - this[inputFileData] = file; + this.fileData = file; if (filename === undefined && typeof file === "string") { filename = basename(file); } this.filename = filename; } + get [inputFileData]() { + if (this.consumed) { + throw new Error("Cannot reuse InputFile data source!"); + } + const data = this.fileData; + if (typeof data !== "string" && (!(data instanceof Uint8Array))) { + this.consumed = false; + } + return data; + } } // === Export InputFile types diff --git a/src/platform.node.ts b/src/platform.node.ts index 8372e914..ce6fe62d 100644 --- a/src/platform.node.ts +++ b/src/platform.node.ts @@ -36,7 +36,8 @@ export const inputFileData = Symbol("InputFile data"); * Reference](https://core.telegram.org/bots/api#inputfile). */ export class InputFile { - public readonly [inputFileData]: ConstructorParameters[0]; + private consumed = false; + private readonly fileData: ConstructorParameters[0]; /** * Optional name of the constructed `InputFile` instance. * @@ -55,12 +56,22 @@ export class InputFile { file: string | Uint8Array | ReadStream | AsyncIterable, filename?: string, ) { - this[inputFileData] = file; + this.fileData = file; if (filename === undefined && typeof file === "string") { filename = basename(file); } this.filename = filename; } + get [inputFileData]() { + if (this.consumed) { + throw new Error("Cannot reuse InputFile data source!"); + } + const data = this.fileData; + if (typeof data !== "string" && (!(data instanceof Uint8Array))) { + this.consumed = false; + } + return data; + } } // === Export InputFile types From 23dcf2affda5934e0d93bf2e055308a259612ea3 Mon Sep 17 00:00:00 2001 From: KnorpelSenf Date: Sat, 6 Nov 2021 22:23:49 +0100 Subject: [PATCH 2/9] Fix wrong consumable state flag --- src/platform.deno.ts | 2 +- src/platform.node.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform.deno.ts b/src/platform.deno.ts index 1738ad3a..7f835160 100644 --- a/src/platform.deno.ts +++ b/src/platform.deno.ts @@ -82,7 +82,7 @@ export class InputFile { } const data = this.fileData; if (typeof data !== "string" && (!(data instanceof Uint8Array))) { - this.consumed = false; + this.consumed = true; } return data; } diff --git a/src/platform.node.ts b/src/platform.node.ts index ce6fe62d..d15b5476 100644 --- a/src/platform.node.ts +++ b/src/platform.node.ts @@ -68,7 +68,7 @@ export class InputFile { } const data = this.fileData; if (typeof data !== "string" && (!(data instanceof Uint8Array))) { - this.consumed = false; + this.consumed = true; } return data; } From 7d778b1d7e3808e236af971911dbaae9e7f8df4d Mon Sep 17 00:00:00 2001 From: KnorpelSenf Date: Tue, 9 Nov 2021 12:15:45 +0100 Subject: [PATCH 3/9] Improve SoC for InputFile data sources --- src/core/payload.ts | 15 ++++----------- src/platform.deno.ts | 36 +++++++++++++++++++----------------- src/platform.node.ts | 25 +++++++++++-------------- 3 files changed, 34 insertions(+), 42 deletions(-) diff --git a/src/core/payload.ts b/src/core/payload.ts index e70033d5..17296dd2 100644 --- a/src/core/payload.ts +++ b/src/core/payload.ts @@ -1,9 +1,4 @@ -import { - InputFile, - inputFileData, - itrToStream, - streamFile, -} from "../platform.deno.ts"; +import { InputFile, itrToStream, toRaw } from "../platform.deno.ts"; // === Payload types (JSON vs. form data) /** @@ -184,11 +179,9 @@ ${filename} yield enc.encode( `content-disposition:form-data;name="${id}";filename=${filename}\r\n\r\n`, ); - const fileData = input[inputFileData]; - // handle buffers, file paths, and streams: - if (fileData instanceof Uint8Array) yield fileData; - else if (typeof fileData === "string") yield* await streamFile(fileData); - else yield* fileData; + const data = await input[toRaw](); + if (data instanceof Uint8Array) yield data; + else yield* data; } /** Returns the default file extension for an API property name */ function getExt(key: string) { diff --git a/src/platform.deno.ts b/src/platform.deno.ts index 9b274a85..ef74f09c 100644 --- a/src/platform.deno.ts +++ b/src/platform.deno.ts @@ -1,3 +1,4 @@ +/** Are we running on Deno or in a web browser? */ const isDeno = typeof Deno !== "undefined"; // === Needed imports @@ -24,12 +25,6 @@ if (isDeno) { // === Export system-specific operations // Turn an AsyncIterable into a stream export { readableStreamFromIterable as itrToStream } from "https://deno.land/std@0.113.0/streams/mod.ts"; -// Turn a file path into an AsyncIterable -export const streamFile = isDeno - ? (path: string) => Deno.open(path).then(iterateReader) - : () => { - throw new Error("Reading files by path requires a Deno environment"); - }; // === Base configuration for `fetch` calls export const baseFetchConfig = {}; @@ -45,7 +40,7 @@ interface URLLike { // === InputFile handling and File augmenting // Accessor for file data in `InputFile` instances -export const inputFileData = Symbol("InputFile data"); +export const toRaw = Symbol("InputFile data"); /** * An `InputFile` wraps a number of different sources for [sending @@ -87,20 +82,27 @@ export class InputFile { } this.filename = filename; } - get [inputFileData]() { + async [toRaw](): Promise> { if (this.consumed) { throw new Error("Cannot reuse InputFile data source!"); } - let data = this.fileData; - if ( - typeof data === "object" && ("url" in data || data instanceof URL) - ) { - data = fetchFile(data instanceof URL ? data : data.url); - } else if ( - typeof data !== "string" && (!(data instanceof Uint8Array)) - ) { - this.consumed = true; + const data = this.fileData; + // Handle file paths + if (typeof data === "string") { + if (!isDeno) { + throw new Error( + "Reading files by path requires a Deno environment", + ); + } + const file = await Deno.open(data); + return iterateReader(file); } + // Handle URL and URLLike + if (data instanceof URL) return fetchFile(data); + if ("url" in data) return fetchFile(data.url); + // Mark streams and iterators as consumed + if (!(data instanceof Uint8Array)) this.consumed = true; + // Return buffers and byte streams as-is return data; } } diff --git a/src/platform.node.ts b/src/platform.node.ts index 9e8dad1a..99086b70 100644 --- a/src/platform.node.ts +++ b/src/platform.node.ts @@ -5,6 +5,7 @@ import { basename } from "path"; import { Readable } from "stream"; import type { ReadStream } from "fs"; import { URL } from "url"; +import { createReadStream } from "fs"; // === Export all API types export * from "@grammyjs/types"; @@ -16,8 +17,6 @@ export { debug } from "debug"; // Turn an AsyncIterable into a stream export const itrToStream = (itr: AsyncIterable) => Readable.from(itr, { objectMode: false }); -// Turn a file path into an AsyncIterable -export { createReadStream as streamFile } from "fs"; // === Base configuration for `fetch` calls export const baseFetchConfig = { @@ -35,7 +34,7 @@ interface URLLike { } // === InputFile handling and File augmenting // Accessor for file data in `InputFile` instances -export const inputFileData = Symbol("InputFile data"); +export const toRaw = Symbol("InputFile data"); /** * An `InputFile` wraps a number of different sources for [sending @@ -77,20 +76,18 @@ export class InputFile { } this.filename = filename; } - get [inputFileData]() { + async [toRaw](): Promise> { if (this.consumed) { throw new Error("Cannot reuse InputFile data source!"); } - let data = this.fileData; - if ( - typeof data === "object" && ("url" in data || data instanceof URL) - ) { - data = fetchFile(data instanceof URL ? data : data.url); - } else if ( - typeof data !== "string" && (!(data instanceof Uint8Array)) - ) { - this.consumed = false; - } + const data = this.fileData; + // Handle file paths, URLs, and URLLike objects + if (typeof data === "string") return createReadStream(data); + if (data instanceof URL) return fetchFile(data); + if ("url" in data) return fetchFile(data.url); + // Mark streams and iterators as consumed + if (!(data instanceof Uint8Array)) this.consumed = true; + // Return buffers and byte streams as-is return data; } } From 4bb5650d20ef9c9ff950cc73027896d090079105 Mon Sep 17 00:00:00 2001 From: KnorpelSenf Date: Thu, 11 Nov 2021 12:02:18 +0100 Subject: [PATCH 4/9] Support file uploads via Deno.open() (#87) --- src/platform.deno.ts | 17 +++++++++++++---- src/platform.node.ts | 8 +++++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/platform.deno.ts b/src/platform.deno.ts index ef74f09c..13e1f7f1 100644 --- a/src/platform.deno.ts +++ b/src/platform.deno.ts @@ -41,6 +41,7 @@ interface URLLike { // === InputFile handling and File augmenting // Accessor for file data in `InputFile` instances export const toRaw = Symbol("InputFile data"); +type MaybePromise = T | Promise; /** * An `InputFile` wraps a number of different sources for [sending @@ -67,13 +68,16 @@ export class InputFile { * @param filename Optional name of the file */ constructor( - file: + file: MaybePromise< | string + | Blob + | Deno.File | URL | URLLike | Uint8Array | ReadableStream - | AsyncIterable, + | AsyncIterable + >, filename?: string, ) { this.fileData = file; @@ -86,8 +90,8 @@ export class InputFile { if (this.consumed) { throw new Error("Cannot reuse InputFile data source!"); } - const data = this.fileData; - // Handle file paths + const data = await this.fileData; + // Handle local files if (typeof data === "string") { if (!isDeno) { throw new Error( @@ -97,6 +101,8 @@ export class InputFile { const file = await Deno.open(data); return iterateReader(file); } + if (data instanceof Blob) return data.stream(); + if (isDenoFile(data)) return iterateReader(data); // Handle URL and URLLike if (data instanceof URL) return fetchFile(data); if ("url" in data) return fetchFile(data.url); @@ -116,6 +122,9 @@ async function* fetchFile(url: string | URL): AsyncIterable { } yield* body; } +function isDenoFile(data: unknown): data is Deno.File { + return isDeno && data instanceof Deno.File; +} // === Export InputFile types type GrammyTypes = InputFileProxy; diff --git a/src/platform.node.ts b/src/platform.node.ts index 99086b70..32c9d09d 100644 --- a/src/platform.node.ts +++ b/src/platform.node.ts @@ -35,6 +35,7 @@ interface URLLike { // === InputFile handling and File augmenting // Accessor for file data in `InputFile` instances export const toRaw = Symbol("InputFile data"); +type MaybePromise = T | Promise; /** * An `InputFile` wraps a number of different sources for [sending @@ -61,13 +62,14 @@ export class InputFile { * @param filename Optional name of the file */ constructor( - file: + file: MaybePromise< | string | URL | URLLike | Uint8Array | ReadStream - | AsyncIterable, + | AsyncIterable + >, filename?: string, ) { this.fileData = file; @@ -80,7 +82,7 @@ export class InputFile { if (this.consumed) { throw new Error("Cannot reuse InputFile data source!"); } - const data = this.fileData; + const data = await this.fileData; // Handle file paths, URLs, and URLLike objects if (typeof data === "string") return createReadStream(data); if (data instanceof URL) return fetchFile(data); From 2c6ecdac71e344b1b5836e0d5121b60edf93afce Mon Sep 17 00:00:00 2001 From: KnorpelSenf Date: Sat, 20 Nov 2021 23:08:44 +0100 Subject: [PATCH 5/9] Do not support promises for InputFile instances --- src/platform.deno.ts | 6 ++---- src/platform.node.ts | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/platform.deno.ts b/src/platform.deno.ts index 13e1f7f1..cab8c75b 100644 --- a/src/platform.deno.ts +++ b/src/platform.deno.ts @@ -41,7 +41,6 @@ interface URLLike { // === InputFile handling and File augmenting // Accessor for file data in `InputFile` instances export const toRaw = Symbol("InputFile data"); -type MaybePromise = T | Promise; /** * An `InputFile` wraps a number of different sources for [sending @@ -68,7 +67,7 @@ export class InputFile { * @param filename Optional name of the file */ constructor( - file: MaybePromise< + file: | string | Blob | Deno.File @@ -76,8 +75,7 @@ export class InputFile { | URLLike | Uint8Array | ReadableStream - | AsyncIterable - >, + | AsyncIterable, filename?: string, ) { this.fileData = file; diff --git a/src/platform.node.ts b/src/platform.node.ts index 32c9d09d..1feb0101 100644 --- a/src/platform.node.ts +++ b/src/platform.node.ts @@ -35,7 +35,6 @@ interface URLLike { // === InputFile handling and File augmenting // Accessor for file data in `InputFile` instances export const toRaw = Symbol("InputFile data"); -type MaybePromise = T | Promise; /** * An `InputFile` wraps a number of different sources for [sending @@ -62,14 +61,13 @@ export class InputFile { * @param filename Optional name of the file */ constructor( - file: MaybePromise< + file: | string | URL | URLLike | Uint8Array | ReadStream - | AsyncIterable - >, + | AsyncIterable, filename?: string, ) { this.fileData = file; From c7d1335f6bf6c6eeee57112a27f05acff3369cd4 Mon Sep 17 00:00:00 2001 From: KnorpelSenf Date: Sat, 20 Nov 2021 23:11:01 +0100 Subject: [PATCH 6/9] Remove superfluous `await`s --- src/platform.deno.ts | 2 +- src/platform.node.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform.deno.ts b/src/platform.deno.ts index cab8c75b..462bb0df 100644 --- a/src/platform.deno.ts +++ b/src/platform.deno.ts @@ -88,7 +88,7 @@ export class InputFile { if (this.consumed) { throw new Error("Cannot reuse InputFile data source!"); } - const data = await this.fileData; + const data = this.fileData; // Handle local files if (typeof data === "string") { if (!isDeno) { diff --git a/src/platform.node.ts b/src/platform.node.ts index 1feb0101..99086b70 100644 --- a/src/platform.node.ts +++ b/src/platform.node.ts @@ -80,7 +80,7 @@ export class InputFile { if (this.consumed) { throw new Error("Cannot reuse InputFile data source!"); } - const data = await this.fileData; + const data = this.fileData; // Handle file paths, URLs, and URLLike objects if (typeof data === "string") return createReadStream(data); if (data instanceof URL) return fetchFile(data); From ed17eef09202782f3d0abf6af581e173687ab7b3 Mon Sep 17 00:00:00 2001 From: KnorpelSenf Date: Sat, 20 Nov 2021 23:15:56 +0100 Subject: [PATCH 7/9] Fix CI and update test deps --- src/platform.node.ts | 2 +- test/bot.test.ts | 2 +- test/filter.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/platform.node.ts b/src/platform.node.ts index 99086b70..bff850a3 100644 --- a/src/platform.node.ts +++ b/src/platform.node.ts @@ -76,7 +76,7 @@ export class InputFile { } this.filename = filename; } - async [toRaw](): Promise> { + [toRaw](): Uint8Array | AsyncIterable { if (this.consumed) { throw new Error("Cannot reuse InputFile data source!"); } diff --git a/test/bot.test.ts b/test/bot.test.ts index 4866bc4c..1d918ac8 100644 --- a/test/bot.test.ts +++ b/test/bot.test.ts @@ -2,7 +2,7 @@ import { Bot } from "../src/bot.ts"; import { assertEquals, assertThrows, -} from "https://deno.land/std@0.97.0/testing/asserts.ts"; +} from "https://deno.land/std@0.115.0/testing/asserts.ts"; function createBot(token: string) { return new Bot(token); diff --git a/test/filter.test.ts b/test/filter.test.ts index 02168bb0..20c3a076 100644 --- a/test/filter.test.ts +++ b/test/filter.test.ts @@ -2,7 +2,7 @@ import { FilterQuery, matchFilter } from "../src/filter.ts"; import { assert, assertThrows, -} from "https://deno.land/std@0.97.0/testing/asserts.ts"; +} from "https://deno.land/std@0.115.0/testing/asserts.ts"; import { Context } from "../src/context.ts"; Deno.test("should reject empty filters", () => { From 1ca96159450a123be70eb5246b1e3360fd88c441 Mon Sep 17 00:00:00 2001 From: KnorpelSenf Date: Tue, 23 Nov 2021 19:12:47 +0100 Subject: [PATCH 8/9] Allow URLs as strings for InputFile (#94) --- src/core/payload.ts | 2 +- src/platform.deno.ts | 34 +++++++++++++++++++++++----------- src/platform.node.ts | 38 +++++++++++++++++++++++++++----------- 3 files changed, 51 insertions(+), 23 deletions(-) diff --git a/src/core/payload.ts b/src/core/payload.ts index 17296dd2..ac54b11e 100644 --- a/src/core/payload.ts +++ b/src/core/payload.ts @@ -166,7 +166,7 @@ async function* filePart( origin: string, input: InputFile, ): AsyncIterableIterator { - const filename = input.filename ?? `${origin}.${getExt(origin)}`; + const filename = input.filename || `${origin}.${getExt(origin)}`; if (filename.includes("\r") || filename.includes("\n")) { throw new Error( `File paths cannot contain carriage-return (\\r) \ diff --git a/src/platform.deno.ts b/src/platform.deno.ts index f0f35fa2..f838664c 100644 --- a/src/platform.deno.ts +++ b/src/platform.deno.ts @@ -10,17 +10,18 @@ import { iterateReader } from "https://deno.land/std@0.113.0/streams/mod.ts"; export * from "https://cdn.skypack.dev/@grammyjs/types@v2.3.1?dts"; // === Export debug -import debug from "https://cdn.skypack.dev/debug@^4.3.2"; -export { debug }; +import d from "https://cdn.skypack.dev/debug@^4.3.2"; +export { d as debug }; if (isDeno) { - debug.useColors = () => !Deno.noColor; + d.useColors = () => !Deno.noColor; try { const val = Deno.env.get("DEBUG"); - if (val) debug.enable(val); + if (val) d.enable(val); } catch { // cannot access env var, treat as if it is not set } } +const debug = d("grammy:warn"); // === Export system-specific operations // Turn an AsyncIterable into a stream @@ -79,10 +80,23 @@ export class InputFile { filename?: string, ) { this.fileData = file; - if (filename === undefined && typeof file === "string") { - filename = basename(file); - } + filename ??= this.guessFilename(file); this.filename = filename; + if ( + typeof file === "string" && + (file.startsWith("http:") || file.startsWith("https:")) + ) { + d(`InputFile received the local file path '${file}' that looks like a URL. Is this a mistake?`); + } + } + private guessFilename( + file: ConstructorParameters[0], + ): string | undefined { + if (typeof file === "string") return basename(file); + if (typeof file !== "object") return undefined; + if ("url" in file) return basename(file.url); + if (!(file instanceof URL)) return undefined; + return basename(file.pathname) || basename(file.hostname); } async [toRaw](): Promise> { if (this.consumed) { @@ -101,7 +115,7 @@ export class InputFile { } if (data instanceof Blob) return data.stream(); if (isDenoFile(data)) return iterateReader(data); - // Handle URL and URLLike + // Handle URL and URLLike objects if (data instanceof URL) return fetchFile(data); if ("url" in data) return fetchFile(data.url); // Mark streams and iterators as consumed @@ -114,9 +128,7 @@ export class InputFile { async function* fetchFile(url: string | URL): AsyncIterable { const { body } = await fetch(url); if (body === null) { - throw new Error( - `Download failed, no response body from '${url}'`, - ); + throw new Error(`Download failed, no response body from '${url}'`); } yield* body; } diff --git a/src/platform.node.ts b/src/platform.node.ts index ce869761..a6ba881a 100644 --- a/src/platform.node.ts +++ b/src/platform.node.ts @@ -12,7 +12,9 @@ import { createReadStream } from "fs"; export * from "@grammyjs/types"; // === Export debug -export { debug } from "debug"; +import { debug as d } from "debug"; +export { d as debug }; +const debug = d("grammy:warn"); // === Export system-specific operations // Turn an AsyncIterable into a stream @@ -36,6 +38,7 @@ interface URLLike { */ url: string; } + // === InputFile handling and File augmenting // Accessor for file data in `InputFile` instances export const toRaw = Symbol("InputFile data"); @@ -75,19 +78,37 @@ export class InputFile { filename?: string, ) { this.fileData = file; - if (filename === undefined && typeof file === "string") { - filename = basename(file); - } + filename ??= this.guessFilename(file); this.filename = filename; + if ( + typeof file === "string" && + (file.startsWith("http:") || file.startsWith("https:")) + ) { + d(`InputFile received the local file path '${file}' that looks like a URL. Is this a mistake?`); + } + } + private guessFilename( + file: ConstructorParameters[0], + ): string | undefined { + if (typeof file === "string") return basename(file); + if (typeof file !== "object") return undefined; + if ("url" in file) return basename(file.url); + if (!(file instanceof URL)) return undefined; + return basename(file.pathname) || basename(file.hostname); } [toRaw](): Uint8Array | AsyncIterable { if (this.consumed) { throw new Error("Cannot reuse InputFile data source!"); } const data = this.fileData; - // Handle file paths, URLs, and URLLike objects + // Handle local files if (typeof data === "string") return createReadStream(data); - if (data instanceof URL) return fetchFile(data); + // Handle URLs and URLLike objects + if (data instanceof URL) { + return data.protocol === "file" // node-fetch does not support file URLs + ? createReadStream(data.pathname) + : fetchFile(data); + } if ("url" in data) return fetchFile(data.url); // Mark streams and iterators as consumed if (!(data instanceof Uint8Array)) this.consumed = true; @@ -98,11 +119,6 @@ export class InputFile { async function* fetchFile(url: string | URL): AsyncIterable { const { body } = await fetch(url); - if (body === null) { - throw new Error( - `Download failed, no response body from '${url}'`, - ); - } for await (const chunk of body) { if (typeof chunk === "string") { throw new Error( From 7fd43b1ea74cbffbbaa3cbfec1733e6a9a190d58 Mon Sep 17 00:00:00 2001 From: KnorpelSenf Date: Tue, 23 Nov 2021 19:15:34 +0100 Subject: [PATCH 9/9] Fix not printing debug messages --- src/platform.deno.ts | 4 +++- src/platform.node.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/platform.deno.ts b/src/platform.deno.ts index f838664c..4ad2cb79 100644 --- a/src/platform.deno.ts +++ b/src/platform.deno.ts @@ -86,7 +86,9 @@ export class InputFile { typeof file === "string" && (file.startsWith("http:") || file.startsWith("https:")) ) { - d(`InputFile received the local file path '${file}' that looks like a URL. Is this a mistake?`); + debug( + `InputFile received the local file path '${file}' that looks like a URL. Is this a mistake?`, + ); } } private guessFilename( diff --git a/src/platform.node.ts b/src/platform.node.ts index a6ba881a..f84a5242 100644 --- a/src/platform.node.ts +++ b/src/platform.node.ts @@ -84,7 +84,9 @@ export class InputFile { typeof file === "string" && (file.startsWith("http:") || file.startsWith("https:")) ) { - d(`InputFile received the local file path '${file}' that looks like a URL. Is this a mistake?`); + debug( + `InputFile received the local file path '${file}' that looks like a URL. Is this a mistake?`, + ); } } private guessFilename(