chrome/browser/ui/browser_navigator_browsertest.cc - Issue 643963004: Unescape BiDi control chars while parsing data urls

Side by Side Diff: chrome/browser/ui/browser_navigator_browsertest.cc

Issue 643963004: Unescape BiDi control chars while parsing data urls (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Add a browsertest Created 6 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.	1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #include "chrome/browser/ui/browser_navigator_browsertest.h"	5 #include "chrome/browser/ui/browser_navigator_browsertest.h"

6	6

7 #include "base/command_line.h"	7 #include "base/command_line.h"

8 #include "base/prefs/pref_service.h"	8 #include "base/prefs/pref_service.h"

9 #include "base/strings/string_util.h"	9 #include "base/strings/string_util.h"

10 #include "base/strings/utf_string_conversions.h"	10 #include "base/strings/utf_string_conversions.h"

(...skipping 1358 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
1369	1369

1370 // Open a renderer initiated POST request in new foreground tab.	1370 // Open a renderer initiated POST request in new foreground tab.

1371 base::string16 expected_title(base::ASCIIToUTF16(kExpectedTitle));	1371 base::string16 expected_title(base::ASCIIToUTF16(kExpectedTitle));

1372 std::string post_data = kExpectedTitle;	1372 std::string post_data = kExpectedTitle;

1373 base::string16 title;	1373 base::string16 title;

1374 ASSERT_TRUE(OpenPOSTURLInNewForegroundTabAndGetTitle(	1374 ASSERT_TRUE(OpenPOSTURLInNewForegroundTabAndGetTitle(

1375 test_server()->GetURL(kEchoTitleCommand), post_data, false, &title));	1375 test_server()->GetURL(kEchoTitleCommand), post_data, false, &title));

1376 EXPECT_NE(expected_title, title);	1376 EXPECT_NE(expected_title, title);

1377 }	1377 }

1378	1378

	1379 // This test navigates to a data URL that contains BiDi control

	1380 // characters. For security reasons, BiDi control chars shouldn't be

	1381 // unescaped in the URL but they should be escaped in the rendered page.

	1382 IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
	mmenke 2014/10/17 15:24:43 Not sure if this is the best browsertest file for Not sure if this is the best browsertest file for this test, but otherwise, tests looks reasonable. meacer 2014/10/17 20:41:54 I looked at other browsertests but couldn't find a Show quoted text On 2014/10/17 15:24:43, mmenke wrote: > Not sure if this is the best browsertest file for this test, but otherwise, > tests looks reasonable. I looked at other browsertests but couldn't find anything data: url related. This seemed to be the best place.
	1383 NavigateToDataURLWithBiDiControlChars) {

	1384 // Uses a test sever to verify POST request.

	1385 ASSERT_TRUE(test_server()->Start());
	mmenke 2014/10/17 15:24:43 Not needed Not needed meacer 2014/10/17 20:41:54 Copy/paste artifact from previous test case. Remov Show quoted text On 2014/10/17 15:24:43, mmenke wrote: > Not needed Copy/paste artifact from previous test case. Removed.
	1386 // Text in Arabic.

	1387 std::string text = "\xD8\xA7\xD8\xAE\xD8\xAA\xD8\xA8\xD8\xA7\xD8\xB1";

	1388 // Page title starts with RTL mark.

	1389 std::string title = "\xE2\x80\x8F" + text;

	1390 GURL data_url("data:text/html;charset=utf-8, <html><title>" + title +
	mmenke 2014/10/17 15:24:43 Is the space before HTML needed? Is the space before HTML needed? meacer 2014/10/17 20:41:54 Removed. Show quoted text On 2014/10/17 15:24:43, mmenke wrote: > Is the space before HTML needed? Removed.
	1391 "</title></html>");

	1392 // BiDi control chars in URLs aren't unescaped, so the expected URL should

	1393 // have the title with the escaped RTL mark.
	mmenke 2014/10/17 15:24:43 If we're expect something not to be escape, should If we're expect something not to be escape, shouldn't we store it in a string, and compare it to p.target_contents->GetURL().spec()? meacer 2014/10/17 20:41:54 Done. Show quoted text On 2014/10/17 15:24:43, mmenke wrote: > If we're expect something not to be escape, shouldn't we store it in a string, > and compare it to p.target_contents->GetURL().spec()? Done.
	1394 std::string escaped_title = "%E2%80%8F" + text;

	1395 GURL expected_url("data:text/html;charset=utf-8, <html><title>" +

	1396 escaped_title + "</title></html>");

	1397

	1398 // Navigate to the page.

	1399 chrome::NavigateParams p(MakeNavigateParams());
	mmenke 2014/10/17 15:24:43 This name violates the google style guide. Sugges This name violates the google style guide. Suggest either params or navigate_params. meacer 2014/10/17 20:41:54 Another copy paste artifact. Fixed other test case Show quoted text On 2014/10/17 15:24:43, mmenke wrote: > This name violates the google style guide. Suggest either params or > navigate_params. Another copy paste artifact. Fixed other test cases too.
	1400 p.disposition = NEW_FOREGROUND_TAB;

	1401 p.url = GURL(data_url);

	1402 p.window_action = chrome::NavigateParams::SHOW_WINDOW;

	1403 ui_test_utils::NavigateToURL(&p);

	1404

	1405 base::string16 expected_title(base::UTF8ToUTF16(title));

	1406 EXPECT_TRUE(p.target_contents);

	1407 EXPECT_EQ(expected_url, p.target_contents->GetURL());

	1408 EXPECT_EQ(expected_title, p.target_contents->GetTitle());

	1409 }

	1410

	1411
	mmenke 2014/10/17 15:24:43 nit: Remove extra line break. nit: Remove extra line break. meacer 2014/10/17 20:41:54 Done. Show quoted text On 2014/10/17 15:24:43, mmenke wrote: > nit: Remove extra line break. Done.
1379 } // namespace	1412 } // namespace

OLD	NEW

« no previous file with comments | « no previous file | net/base/data_url.cc » ('j') | net/base/data_url_unittest.cc » ('J')