Skip to content

string_decoder: throw ERR_STRING_TOO_LONG for UTF-8 - #36661

Closed
targos wants to merge 1 commit into
nodejs:masterfrom
targos:fix-35676
Closed

string_decoder: throw ERR_STRING_TOO_LONG for UTF-8#36661
targos wants to merge 1 commit into
nodejs:masterfrom
targos:fix-35676

Conversation

@targos

@targostargos commented Dec 28, 2020

Copy link
Copy Markdown
Member

String::NewFromUtf8 doesn't generate an exception in V8 when the string
is too long but is guaranteed to return an empty MaybeLocal only in
that case. Generate a Node.js exception when it happens.

Fixes: #35676

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@targos
string_decoder: throw ERR_STRING_TOO_LONG for UTF-8
3736662
String::NewFromUtf8 doesn't generate an exception in V8 when the string
is too long but is guaranteed to return an empty MaybeLocal only in
that case. Generate a Node.js exception when it happens.
Fixes: nodejs#35676
@targostargos added the string_decoder Issues and PRs related to the string_decoder subsystem. label Dec 28, 2020
@targos
targos requested a review from addaleaxDecember 28, 2020 13:19
@nodejs-github-botnodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. labels Dec 28, 2020
@targostargos mentioned this pull request Dec 28, 2020
Closed
@targostargos added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 28, 2020
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 28, 2020
@nodejs-github-bot

nodejs-github-bot commented Dec 28, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/35120/

@targostargos added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 30, 2020
@jasnelljasnell added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 31, 2020
@github-actionsgithub-actionsBot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 31, 2020
@github-actions

github-actionsBot commented Dec 31, 2020

Copy link
Copy Markdown
Contributor

Landed in c380ee6...11b34a9

nodejs-github-bot pushed a commit that referenced this pull request Dec 31, 2020
@targos@nodejs-github-bot
string_decoder: throw ERR_STRING_TOO_LONG for UTF-8
11b34a9
String::NewFromUtf8 doesn't generate an exception in V8 when the string
is too long but is guaranteed to return an empty MaybeLocal only in
that case. Generate a Node.js exception when it happens.
Fixes: #35676
PR-URL: #36661
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mhdawsonmhdawson mentioned this pull request Jan 5, 2021
Closed
danielleadams pushed a commit that referenced this pull request Jan 12, 2021
@targos@danielleadams
string_decoder: throw ERR_STRING_TOO_LONG for UTF-8
d2b9e7c
String::NewFromUtf8 doesn't generate an exception in V8 when the string
is too long but is guaranteed to return an empty MaybeLocal only in
that case. Generate a Node.js exception when it happens.
Fixes: #35676
PR-URL: #36661
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@danielleadamsdanielleadams mentioned this pull request Jan 12, 2021
Merged
@targos
targos deleted the fix-35676 branch April 12, 2021 15:06
targos added a commit that referenced this pull request May 25, 2021
@targos
string_decoder: throw ERR_STRING_TOO_LONG for UTF-8
b58f915
String::NewFromUtf8 doesn't generate an exception in V8 when the string
is too long but is guaranteed to return an empty MaybeLocal only in
that case. Generate a Node.js exception when it happens.
Fixes: #35676
PR-URL: #36661
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos added a commit that referenced this pull request Jun 5, 2021
@targos
string_decoder: throw ERR_STRING_TOO_LONG for UTF-8
541322d
String::NewFromUtf8 doesn't generate an exception in V8 when the string
is too long but is guaranteed to return an empty MaybeLocal only in
that case. Generate a Node.js exception when it happens.
Fixes: #35676
PR-URL: #36661
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targostargos mentioned this pull request Jun 6, 2021
Merged
targos added a commit that referenced this pull request Jun 11, 2021
@targos
string_decoder: throw ERR_STRING_TOO_LONG for UTF-8
1dc4dea
String::NewFromUtf8 doesn't generate an exception in V8 when the string
is too long but is guaranteed to return an empty MaybeLocal only in
that case. Generate a Node.js exception when it happens.
Fixes: #35676
PR-URL: #36661
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes
@TrottTrottTrott approved these changes
@addaleaxaddaleaxaddaleax approved these changes
@cjihrigcjihrigcjihrig approved these changes
@gireeshpunathilgireeshpunathilgireeshpunathil approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.bufferIssues and PRs related to the buffer subsystem.c++Issues and PRs that require attention from people who are familiar with C++.string_decoderIssues and PRs related to the string_decoder subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

StringDecoder undocumented size limit

7 participants

@targos@nodejs-github-bot@jasnell@Trott@addaleax@cjihrig@gireeshpunathil