Issue 178473027: Make sure string passed to sendMessageToEmbedder is a valid unicode string

Issue 178473027: Make sure string passed to sendMessageToEmbedder is a valid unicode string (Closed)

Created:
6 years, 9 months ago by yurys

Modified:
6 years, 9 months ago

Reviewers:
alph, pfeldman, loislo

CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, alph+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, aandrey+blink_chromium.org

Base URL:
svn://svn.chromium.org/blink/trunk

Visibility:
Public.

More Reviews

Description

Make sure string passed to sendMessageToEmbedder is a valid unicode string The message passed to the method is a result of JSON.stringify and may contain noncharacter unicode code points (http://www.unicode.org/faq/private_use.html#noncharacters). Since we know that the message is a JSON string we can use \uXXXX escape sequences for bad symbols. The function escapes not only noncharacters which produces valid output and allows to keep implementation simple. BUG=347899 R=pfeldman@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168448

Patch Set 1 #

Total comments: 2

Patch Set 2 : Comments addressed #

Patch Set 3 : Do not escape whitespaces as they are allowed between tokens in JSON #

Patch Set 4 : Rebase #

Total comments: 2

Created: 6 years, 9 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+19 lines, -2 lines)			Patch
	M	Source/core/inspector/InspectorFrontendHost.cpp	View	1 2	1 chunk	+19 lines, -2 lines	2 comments	Download

Messages

Total messages: 10 (0 generated)

Expand Messages | Collapse Messages

pfeldman

lgtm https://codereview.chromium.org/178473027/diff/1/Source/core/inspector/InspectorFrontendHost.cpp File Source/core/inspector/InspectorFrontendHost.cpp (right): https://codereview.chromium.org/178473027/diff/1/Source/core/inspector/InspectorFrontendHost.cpp#newcode166 Source/core/inspector/InspectorFrontendHost.cpp:166: m_client->sendMessageToBackend(message); Lets use it here as well.

6 years, 9 months ago (2014-03-04 11:45:46 UTC) #2

yurys

https://codereview.chromium.org/178473027/diff/1/Source/core/inspector/InspectorFrontendHost.cpp File Source/core/inspector/InspectorFrontendHost.cpp (right): https://codereview.chromium.org/178473027/diff/1/Source/core/inspector/InspectorFrontendHost.cpp#newcode166 Source/core/inspector/InspectorFrontendHost.cpp:166: m_client->sendMessageToBackend(message); On 2014/03/04 11:45:46, pfeldman wrote: > Lets use ...

6 years, 9 months ago (2014-03-04 12:15:35 UTC) #3

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yurys@chromium.org/178473027/40001

6 years, 9 months ago (2014-03-04 12:37:20 UTC) #7

yurys

Committed patchset #4 manually as r168448 (presubmit successful).

6 years, 9 months ago (2014-03-05 08:18:38 UTC) #8

alph

https://codereview.chromium.org/178473027/diff/60001/Source/core/inspector/InspectorFrontendHost.cpp File Source/core/inspector/InspectorFrontendHost.cpp (right): https://codereview.chromium.org/178473027/diff/60001/Source/core/inspector/InspectorFrontendHost.cpp#newcode168 Source/core/inspector/InspectorFrontendHost.cpp:168: if (c > 126) { 126 seems to be ...

6 years, 9 months ago (2014-03-05 09:01:53 UTC) #9

yurys

6 years, 9 months ago (2014-03-05 11:33:36 UTC) #10

Message was sent while issue was closed.

https://codereview.chromium.org/178473027/diff/60001/Source/core/inspector/In...
File Source/core/inspector/InspectorFrontendHost.cpp (right):

https://codereview.chromium.org/178473027/diff/60001/Source/core/inspector/In...
Source/core/inspector/InspectorFrontendHost.cpp:168: if (c > 126) {
On 2014/03/05 09:01:54, alph wrote:
> 126 seems to be too conservative. I'd suggest escaping only values larger than
> the first surrogate value 0xD800.

PTAL: https://codereview.chromium.org/180113008/

Expand Messages | Collapse Messages