Skip to content

url: enforce valid UTF-8 in WHATWG parser #11436

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

Closed
TimothyGu wants to merge 4 commits into from
Closed

url: enforce valid UTF-8 in WHATWG parser #11436

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
db850d7
test: fix WPT.test()'s error handling
TimothyGu Feb 4, 2017
2a2b4c7
src: remove misleading flag in TwoByteValue
TimothyGu Feb 17, 2017
aa4b2be
url: enforce valid UTF-8 in WHATWG parser
TimothyGu Feb 4, 2017
19b37dc
benchmark: add USVString conversion benchmark
TimothyGu Feb 17, 2017
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
Prev Previous commit
Next Next commit
src: remove misleading flag in TwoByteValue 
String::REPLACE_INVALID_UTF8 is only applied in V8's
String::WriteUtf8() (i.e. Utf8Value).
  • Loading branch information
@TimothyGu
TimothyGu committed Feb 28, 2017
commit 2a2b4c7c8f21f291fac4c81b43c8f66197dcd0e8
3 changes: 1 addition & 2 deletions src/util.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ TwoByteValue::TwoByteValue(Isolate* isolate, Local<Value> value) {
const size_t storage = string->Length() + 1;
AllocateSufficientStorage(storage);

const int flags =
String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8;
const int flags = String::NO_NULL_TERMINATION;
Copy link
Copy Markdown
Contributor

@mscdex mscdex Feb 17, 2017

Choose a reason for hiding this comment

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

I'm not sure this is right, since this would affect more than just the WHATWG URL implementation?

Copy link
Copy Markdown
Member

@bnoordhuis bnoordhuis Feb 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Indeed, this is not an acceptable change. You could turn the flags into an optional argument that defaults to what is now or make it a template trait.

EDIT: Objection withdrawn.

Copy link
Copy Markdown
Member Author

@TimothyGu TimothyGu Feb 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

As I've explained in 424722af4541cd0eec1bf299aa8afcdff6284f52, this flag is actually a no-op. TwoByteValue::TwoByteValue uses v8::String::Write as opposed to v8::String::WriteUtf8. The flag is only respected by WriteUtf8, and hence misleading being applied here in TwoByteValue.

Copy link
Copy Markdown
Member

@bnoordhuis bnoordhuis Feb 19, 2017

Choose a reason for hiding this comment

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

Oh, you're right, I missed that it's the UTF-16 version. Objection withdrawn.

const int length = string->Write(out(), 0, storage, flags);
SetLengthAndZeroTerminate(length);
}
Expand Down