Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix usage of textDecoder to not break utf8 characters #46564

Merged
merged 3 commits into from
Feb 28, 2023
Merged

Fix usage of textDecoder to not break utf8 characters #46564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a7a8937
Fix usage of textDecoder
vadzim Feb 28, 2023
c85d221
make textDecoder param in decodeText obligatory
vadzim Feb 28, 2023
945208c
Merge branch 'canary' into fix-decodetext-usage
ijjk Feb 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/next/src/server/app-render.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ function useFlightResponse(
const startScriptTag = nonce
? `<script nonce=${JSON.stringify(nonce)}>`
: '<script>'
const textDecoder = new TextDecoder()

function read() {
forwardReader.read().then(({ done, value }) => {
Expand All @@ -357,7 +358,7 @@ function useFlightResponse(
flightResponseRef.current = null
writer.close()
} else {
const responsePartial = decodeText(value)
const responsePartial = decodeText(value, textDecoder)
const scripts = `${startScriptTag}self.__next_f.push(${htmlEscapeJsonString(
JSON.stringify([1, responsePartial])
)})</script>`
Expand Down Expand Up @@ -1476,10 +1477,11 @@ export async function renderToHTMLOrFlight(
renderResult: RenderResult
): Promise<string> => {
const renderChunks: string[] = []
const textDecoder = new TextDecoder()

const writable = {
write(chunk: any) {
renderChunks.push(decodeText(chunk))
renderChunks.push(decodeText(chunk, textDecoder))
},
end() {},
destroy() {},
Copy link
Contributor

@jridgewell jridgewell Feb 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to perform a final call to the textDecoder here to flush any trailing bytes that have not completed. This only comes up when an incomplete octet set is encountered at the end of of the stream:

const chunk = new TextEncoder().encode('abc💩').slice(0, -1);

const decoder = new TextDecoder();
let output = decoder.decode(chunk, { stream: true });

console.assert(output === 'abc');

// Flush the decoder to receive the final REPLACEMENT CHARACTER
// for the unfinished emoji
output += decoder.decode();

console.assert(output === 'abc�');

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep
I've mentioned that here #46564 (comment)

I've guessed no one cares.
I can make another PR for all six usages if you need it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the example, will follow up later!

Expand Down
15 changes: 9 additions & 6 deletions packages/next/src/server/node-web-streams-helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ export function encodeText(input: string) {
return new TextEncoder().encode(input)
}

export function decodeText(input?: Uint8Array, textDecoder?: TextDecoder) {
return textDecoder
? textDecoder.decode(input, { stream: true })
: new TextDecoder().decode(input)
export function decodeText(
input: Uint8Array | undefined,
textDecoder: TextDecoder
) {
return textDecoder.decode(input, { stream: true })
}

export function readableStreamTee<T = any>(
Expand Down Expand Up @@ -161,6 +162,7 @@ function createHeadInsertionTransformStream(
): TransformStream<Uint8Array, Uint8Array> {
let inserted = false
let freezing = false
const textDecoder = new TextDecoder()

return new TransformStream({
async transform(chunk, controller) {
Expand All @@ -176,7 +178,7 @@ function createHeadInsertionTransformStream(
controller.enqueue(chunk)
freezing = true
} else {
const content = decodeText(chunk)
const content = decodeText(chunk, textDecoder)
const index = content.indexOf('</head>')
if (index !== -1) {
const insertedHeadContent =
Expand Down Expand Up @@ -293,11 +295,12 @@ export function createRootLayoutValidatorStream(
): TransformStream<Uint8Array, Uint8Array> {
let foundHtml = false
let foundBody = false
const textDecoder = new TextDecoder()

return new TransformStream({
async transform(chunk, controller) {
if (!foundHtml || !foundBody) {
const content = decodeText(chunk)
const content = decodeText(chunk, textDecoder)
if (!foundHtml && content.includes('<html')) {
foundHtml = true
}
Expand Down