From d308e8d0c0469419517e05a36ba070633168dc67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 29 Apr 2020 22:38:10 +0200 Subject: [PATCH] BREAKING: remove custom implementation of Deno.Buffer.toString() (#4992) Keep in mind Buffer.toString() still exists, but returns [object Object]. Reason for removal of Buffer.toString() was that it implicitly used TextDecoder with fixed "utf-8" encoding and no way to customize the encoding. --- cli/js/buffer.ts | 6 ------ cli/js/lib.deno.ns.d.ts | 14 +++++++------- cli/js/tests/buffer_test.ts | 1 - std/http/io_test.ts | 9 ++++++--- std/http/server_test.ts | 2 +- std/io/ioutil_test.ts | 4 ++-- std/mime/multipart.ts | 2 +- std/ws/test.ts | 18 ++++++++++++------ 8 files changed, 29 insertions(+), 27 deletions(-) diff --git a/cli/js/buffer.ts b/cli/js/buffer.ts index fdacf70a9f839..dbe7606fd670b 100644 --- a/cli/js/buffer.ts +++ b/cli/js/buffer.ts @@ -6,7 +6,6 @@ import { Reader, Writer, ReaderSync, WriterSync } from "./io.ts"; import { assert } from "./util.ts"; -import { TextDecoder } from "./web/text_encoding.ts"; // MIN_READ is the minimum ArrayBuffer size passed to a read call by // buffer.ReadFrom. As long as the Buffer has at least MIN_READ bytes beyond @@ -44,11 +43,6 @@ export class Buffer implements Reader, ReaderSync, Writer, WriterSync { return this.#buf.subarray(this.#off); } - toString(): string { - const decoder = new TextDecoder(); - return decoder.decode(this.#buf.subarray(this.#off)); - } - empty(): boolean { return this.#buf.byteLength <= this.#off; } diff --git a/cli/js/lib.deno.ns.d.ts b/cli/js/lib.deno.ns.d.ts index d7d885b5bef65..aca0fc1149fbc 100644 --- a/cli/js/lib.deno.ns.d.ts +++ b/cli/js/lib.deno.ns.d.ts @@ -847,12 +847,6 @@ declare namespace Deno { * least until the next buffer modification, so immediate changes to the * slice will affect the result of future reads. */ bytes(): Uint8Array; - /** Returns the contents of the unread portion of the buffer as a `string`. - * - * **Warning**: if multibyte characters are present when data is flowing - * through the buffer, this method may result in incorrect strings due to a - * character being split. */ - toString(): string; /** Returns whether the unread portion of the buffer is empty. */ empty(): boolean; /** A read only number of bytes of the unread portion of the buffer. */ @@ -873,9 +867,15 @@ declare namespace Deno { readSync(p: Uint8Array): number | null; /** Reads the next `p.length` bytes from the buffer or until the buffer is * drained. Resolves to the number of bytes read. If the buffer has no - * data to return, resolves to EOF (`null`). */ + * data to return, resolves to EOF (`null`). + * + * NOTE: This methods reads bytes sychronously; it's provided for + * compatibility with `Reader` interfaces. + */ read(p: Uint8Array): Promise; writeSync(p: Uint8Array): number; + /** NOTE: This methods writes bytes sychronously; it's provided for + * compatibility with `Writer` interface. */ write(p: Uint8Array): Promise; /** Grows the buffer's capacity, if necessary, to guarantee space for * another `n` bytes. After `.grow(n)`, at least `n` bytes can be written to diff --git a/cli/js/tests/buffer_test.ts b/cli/js/tests/buffer_test.ts index e45bcb976ac42..ac2d7db7b9856 100644 --- a/cli/js/tests/buffer_test.ts +++ b/cli/js/tests/buffer_test.ts @@ -35,7 +35,6 @@ function check(buf: Deno.Buffer, s: string): void { const decoder = new TextDecoder(); const bytesStr = decoder.decode(bytes); assertEquals(bytesStr, s); - assertEquals(buf.length, buf.toString().length); assertEquals(buf.length, s.length); } diff --git a/std/http/io_test.ts b/std/http/io_test.ts index b9bb56f1196d5..fd41b474e391f 100644 --- a/std/http/io_test.ts +++ b/std/http/io_test.ts @@ -51,7 +51,7 @@ test("chunkedBodyReader", async () => { await dest.write(buf.subarray(0, len)); } const exp = "aaabbbbbcccccccccccdddddddddddddddddddddd"; - assertEquals(dest.toString(), exp); + assertEquals(new TextDecoder().decode(dest.bytes()), exp); }); test("chunkedBodyReader with trailers", async () => { @@ -133,7 +133,10 @@ test("writeTrailer", async () => { new Headers({ "transfer-encoding": "chunked", trailer: "deno,node" }), new Headers({ deno: "land", node: "js" }) ); - assertEquals(w.toString(), "deno: land\r\nnode: js\r\n\r\n"); + assertEquals( + new TextDecoder().decode(w.bytes()), + "deno: land\r\nnode: js\r\n\r\n" + ); }); test("writeTrailer should throw", async () => { @@ -336,7 +339,7 @@ test("writeResponse with trailer", async () => { body, trailers: () => new Headers({ deno: "land", node: "js" }), }); - const ret = w.toString(); + const ret = new TextDecoder().decode(w.bytes()); const exp = [ "HTTP/1.1 200 OK", "transfer-encoding: chunked", diff --git a/std/http/server_test.ts b/std/http/server_test.ts index 4e7b4b69e588a..03256dd25ca64 100644 --- a/std/http/server_test.ts +++ b/std/http/server_test.ts @@ -63,7 +63,7 @@ test("responseWrite", async function (): Promise { request.conn = mockConn(); await request.respond(testCase.response); - assertEquals(buf.toString(), testCase.raw); + assertEquals(new TextDecoder().decode(buf.bytes()), testCase.raw); await request.done; } }); diff --git a/std/io/ioutil_test.ts b/std/io/ioutil_test.ts index 1f552d0e6d2d3..cf8eaf43739b4 100644 --- a/std/io/ioutil_test.ts +++ b/std/io/ioutil_test.ts @@ -76,7 +76,7 @@ Deno.test("testCopyN1", async function (): Promise { const r = stringsReader("abcdefghij"); const n = await copyN(r, w, 3); assertEquals(n, 3); - assertEquals(w.toString(), "abc"); + assertEquals(new TextDecoder().decode(w.bytes()), "abc"); }); Deno.test("testCopyN2", async function (): Promise { @@ -84,7 +84,7 @@ Deno.test("testCopyN2", async function (): Promise { const r = stringsReader("abcdefghij"); const n = await copyN(r, w, 11); assertEquals(n, 10); - assertEquals(w.toString(), "abcdefghij"); + assertEquals(new TextDecoder().decode(w.bytes()), "abcdefghij"); }); Deno.test("copyNWriteAllData", async function (): Promise { diff --git a/std/mime/multipart.ts b/std/mime/multipart.ts index 48e32c65d8c26..b2ac8dcdd3afb 100644 --- a/std/mime/multipart.ts +++ b/std/mime/multipart.ts @@ -302,7 +302,7 @@ export class MultipartReader { if (maxValueBytes < 0) { throw new RangeError("message too large"); } - const value = buf.toString(); + const value = new TextDecoder().decode(buf.bytes()); valueMap.set(p.formName, value); continue; } diff --git a/std/ws/test.ts b/std/ws/test.ts index c6e74dadcb5c5..796bf3bb81ac0 100644 --- a/std/ws/test.ts +++ b/std/ws/test.ts @@ -31,7 +31,8 @@ test("[ws] read unmasked text frame", async () => { const frame = await readFrame(buf); assertEquals(frame.opcode, OpCode.TextFrame); assertEquals(frame.mask, undefined); - assertEquals(new Buffer(frame.payload).toString(), "Hello"); + const actual = new TextDecoder().decode(new Buffer(frame.payload).bytes()); + assertEquals(actual, "Hello"); assertEquals(frame.isLastFrame, true); }); @@ -57,7 +58,8 @@ test("[ws] read masked text frame", async () => { const frame = await readFrame(buf); assertEquals(frame.opcode, OpCode.TextFrame); unmask(frame.payload, frame.mask); - assertEquals(new Buffer(frame.payload).toString(), "Hello"); + const actual = new TextDecoder().decode(new Buffer(frame.payload).bytes()); + assertEquals(actual, "Hello"); assertEquals(frame.isLastFrame, true); }); @@ -72,12 +74,14 @@ test("[ws] read unmasked split text frames", async () => { assertEquals(f1.isLastFrame, false); assertEquals(f1.mask, undefined); assertEquals(f1.opcode, OpCode.TextFrame); - assertEquals(new Buffer(f1.payload).toString(), "Hel"); + const actual1 = new TextDecoder().decode(new Buffer(f1.payload).bytes()); + assertEquals(actual1, "Hel"); assertEquals(f2.isLastFrame, true); assertEquals(f2.mask, undefined); assertEquals(f2.opcode, OpCode.Continue); - assertEquals(new Buffer(f2.payload).toString(), "lo"); + const actual2 = new TextDecoder().decode(new Buffer(f2.payload).bytes()); + assertEquals(actual2, "lo"); }); test("[ws] read unmasked ping / pong frame", async () => { @@ -87,7 +91,8 @@ test("[ws] read unmasked ping / pong frame", async () => { ); const ping = await readFrame(buf); assertEquals(ping.opcode, OpCode.Ping); - assertEquals(new Buffer(ping.payload).toString(), "Hello"); + const actual1 = new TextDecoder().decode(new Buffer(ping.payload).bytes()); + assertEquals(actual1, "Hello"); // prettier-ignore const pongFrame= [0x8a, 0x85, 0x37, 0xfa, 0x21, 0x3d, 0x7f, 0x9f, 0x4d, 0x51, 0x58] const buf2 = new BufReader(new Buffer(new Uint8Array(pongFrame))); @@ -95,7 +100,8 @@ test("[ws] read unmasked ping / pong frame", async () => { assertEquals(pong.opcode, OpCode.Pong); assert(pong.mask !== undefined); unmask(pong.payload, pong.mask); - assertEquals(new Buffer(pong.payload).toString(), "Hello"); + const actual2 = new TextDecoder().decode(new Buffer(pong.payload).bytes()); + assertEquals(actual2, "Hello"); }); test("[ws] read unmasked big binary frame", async () => {