From 67b71ea2e11563a4c0363e1dce4e266cebcd063a Mon Sep 17 00:00:00 2001 From: markdittmer Date: Thu, 3 Mar 2016 14:40:03 -0800 Subject: Move GURL ParamTraits to url/ipc As a part of the GPU refactor for Mus, we are trying to eliminate spots where content/common/gpu depends on content/. One such dependency is GURL. As per discussion here: https://codereview.chromium.org/1703163002/, the plan of record is to move GURL ParamTraits out of content/ and into url/. BUG=586368 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation Committed: https://crrev.com/45eb2e749a8b08988ab45bfd6806c004e9f511fa Cr-Commit-Position: refs/heads/master@{#378859} Review URL: https://codereview.chromium.org/1722773002 Cr-Commit-Position: refs/heads/master@{#379122} --- url/ipc/BUILD.gn | 38 +++++++++++++++++++ url/ipc/url_ipc.gyp | 42 +++++++++++++++++++++ url/ipc/url_ipc_export.h | 29 +++++++++++++++ url/ipc/url_param_traits.cc | 52 ++++++++++++++++++++++++++ url/ipc/url_param_traits.h | 26 +++++++++++++ url/ipc/url_param_traits_unittest.cc | 72 ++++++++++++++++++++++++++++++++++++ url/url_constants.cc | 2 + url/url_constants.h | 4 ++ 8 files changed, 265 insertions(+) create mode 100644 url/ipc/BUILD.gn create mode 100644 url/ipc/url_ipc.gyp create mode 100644 url/ipc/url_ipc_export.h create mode 100644 url/ipc/url_param_traits.cc create mode 100644 url/ipc/url_param_traits.h create mode 100644 url/ipc/url_param_traits_unittest.cc (limited to 'url') diff --git a/url/ipc/BUILD.gn b/url/ipc/BUILD.gn new file mode 100644 index 0000000..1edbe20 --- /dev/null +++ b/url/ipc/BUILD.gn @@ -0,0 +1,38 @@ +# Copyright (c) 2016 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import("//testing/test.gni") + +component("url_ipc") { + sources = [ + "url_ipc_export.h", + "url_param_traits.cc", + "url_param_traits.h", + ] + + defines = [ "URL_IPC_IMPLEMENTATION" ] + + public_deps = [ + "//ipc", + "//url", + ] + deps = [ + "//base", + ] +} + +test("url_ipc_unittests") { + sources = [ + "url_param_traits_unittest.cc", + ] + + deps = [ + ":url_ipc", + "//base", + "//base/test:run_all_unittests", + "//ipc:test_support", + "//testing/gtest", + "//url:url", + ] +} diff --git a/url/ipc/url_ipc.gyp b/url/ipc/url_ipc.gyp new file mode 100644 index 0000000..f56b5e8 --- /dev/null +++ b/url/ipc/url_ipc.gyp @@ -0,0 +1,42 @@ +# Copyright 2016 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +{ + 'variables': { + 'chromium_code': 1, + }, + 'targets': [ + { + # GN version: //url/ipc + 'target_name': 'url_ipc', + 'type': '<(component)', + 'dependencies': [ + '../../base/base.gyp:base', + '../../ipc/ipc.gyp:ipc', + '../../url/url.gyp:url_lib', + ], + 'defines': [ 'URL_IPC_IMPLEMENTATION' ], + 'include_dirs': [ + '../..', + ], + 'sources': [ + 'url_ipc_export.h', + 'url_param_traits.cc', + 'url_param_traits.h', + ], + }, + { + 'target_name': 'url_ipc_unittests', + 'type': 'executable', + 'dependencies': [ + '../../base/base.gyp:run_all_unittests', + '../../ipc/ipc.gyp:test_support_ipc', + '../../testing/gtest.gyp:gtest', + '../url.gyp:url_lib', + 'url_ipc', + ], + 'sources': [ 'url_param_traits_unittest.cc' ], + }, + ], +} diff --git a/url/ipc/url_ipc_export.h b/url/ipc/url_ipc_export.h new file mode 100644 index 0000000..1da0fa6 --- /dev/null +++ b/url/ipc/url_ipc_export.h @@ -0,0 +1,29 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef URL_IPC_EXPORT_H_ +#define URL_IPC_EXPORT_H_ + +#if defined(COMPONENT_BUILD) +#if defined(WIN32) + +#if defined(URL_IPC_IMPLEMENTATION) +#define URL_IPC_EXPORT __declspec(dllexport) +#else +#define URL_IPC_EXPORT __declspec(dllimport) +#endif // defined(URL_IPC_IMPLEMENTATION) + +#else // defined(WIN32) +#if defined(URL_IPC_IMPLEMENTATION) +#define URL_IPC_EXPORT __attribute__((visibility("default"))) +#else +#define URL_IPC_EXPORT +#endif +#endif + +#else // defined(COMPONENT_BUILD) +#define URL_IPC_EXPORT +#endif + +#endif // URL_IPC_EXPORT_H_ diff --git a/url/ipc/url_param_traits.cc b/url/ipc/url_param_traits.cc new file mode 100644 index 0000000..e40ae8f --- /dev/null +++ b/url/ipc/url_param_traits.cc @@ -0,0 +1,52 @@ +// Copyright (c) 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "url/ipc/url_param_traits.h" + +#include "url/gurl.h" + +namespace IPC { + +void ParamTraits::Write(base::Pickle* m, const GURL& p) { + if (p.possibly_invalid_spec().length() > url::kMaxURLChars) { + m->WriteString(std::string()); + return; + } + + // Beware of print-parse inconsistency which would change an invalid + // URL into a valid one. Ideally, the message would contain this flag + // so that the read side could make the check, but performing it here + // avoids changing the on-the-wire representation of such a fundamental + // type as GURL. See https://crbug.com/166486 for additional work in + // this area. + if (!p.is_valid()) { + m->WriteString(std::string()); + return; + } + + m->WriteString(p.possibly_invalid_spec()); + // TODO(brettw) bug 684583: Add encoding for query params. +} + +bool ParamTraits::Read(const base::Pickle* m, + base::PickleIterator* iter, + GURL* p) { + std::string s; + if (!iter->ReadString(&s) || s.length() > url::kMaxURLChars) { + *p = GURL(); + return false; + } + *p = GURL(s); + if (!s.empty() && !p->is_valid()) { + *p = GURL(); + return false; + } + return true; +} + +void ParamTraits::Log(const GURL& p, std::string* l) { + l->append(p.spec()); +} + +} // namespace IPC diff --git a/url/ipc/url_param_traits.h b/url/ipc/url_param_traits.h new file mode 100644 index 0000000..f40150f1 --- /dev/null +++ b/url/ipc/url_param_traits.h @@ -0,0 +1,26 @@ +// Copyright (c) 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef URL_IPC_URL_PARAM_TRAITS_H_ +#define URL_IPC_URL_PARAM_TRAITS_H_ + +#include "ipc/ipc_message_utils.h" +#include "url/gurl.h" +#include "url/ipc/url_ipc_export.h" + +namespace IPC { + +template <> +struct URL_IPC_EXPORT ParamTraits { + typedef GURL param_type; + static void Write(base::Pickle* m, const param_type& p); + static bool Read(const base::Pickle* m, + base::PickleIterator* iter, + param_type* p); + static void Log(const param_type& p, std::string* l); +}; + +} // namespace IPC + +#endif // URL_IPC_URL_PARAM_TRAITS_H_ diff --git a/url/ipc/url_param_traits_unittest.cc b/url/ipc/url_param_traits_unittest.cc new file mode 100644 index 0000000..16eeab0 --- /dev/null +++ b/url/ipc/url_param_traits_unittest.cc @@ -0,0 +1,72 @@ +// Copyright (c) 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include "ipc/ipc_message.h" +#include "ipc/ipc_message_utils.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "url/gurl.h" +#include "url/ipc/url_param_traits.h" + +// Tests that serialize/deserialize correctly understand each other. +TEST(IPCMessageTest, Serialize) { + const char* serialize_cases[] = { + "http://www.google.com/", + "http://user:pass@host.com:888/foo;bar?baz#nop", + }; + + for (size_t i = 0; i < arraysize(serialize_cases); i++) { + GURL input(serialize_cases[i]); + IPC::Message msg(1, 2, IPC::Message::PRIORITY_NORMAL); + IPC::ParamTraits::Write(&msg, input); + + GURL output; + base::PickleIterator iter(msg); + EXPECT_TRUE(IPC::ParamTraits::Read(&msg, &iter, &output)); + + // We want to test each component individually to make sure its range was + // correctly serialized and deserialized, not just the spec. + EXPECT_EQ(input.possibly_invalid_spec(), output.possibly_invalid_spec()); + EXPECT_EQ(input.is_valid(), output.is_valid()); + EXPECT_EQ(input.scheme(), output.scheme()); + EXPECT_EQ(input.username(), output.username()); + EXPECT_EQ(input.password(), output.password()); + EXPECT_EQ(input.host(), output.host()); + EXPECT_EQ(input.port(), output.port()); + EXPECT_EQ(input.path(), output.path()); + EXPECT_EQ(input.query(), output.query()); + EXPECT_EQ(input.ref(), output.ref()); + } + + // Test an excessively long GURL. + { + const std::string url = std::string("http://example.org/").append( + url::kMaxURLChars + 1, 'a'); + GURL input(url.c_str()); + IPC::Message msg(1, 2, IPC::Message::PRIORITY_NORMAL); + IPC::ParamTraits::Write(&msg, input); + + GURL output; + base::PickleIterator iter(msg); + EXPECT_TRUE(IPC::ParamTraits::Read(&msg, &iter, &output)); + EXPECT_TRUE(output.is_empty()); + } + + // Test an invalid GURL. + { + IPC::Message msg; + msg.WriteString("#inva://idurl/"); + GURL output; + base::PickleIterator iter(msg); + EXPECT_FALSE(IPC::ParamTraits::Read(&msg, &iter, &output)); + } + + // Also test the corrupt case. + IPC::Message msg(1, 2, IPC::Message::PRIORITY_NORMAL); + msg.WriteInt(99); + GURL output; + base::PickleIterator iter(msg); + EXPECT_FALSE(IPC::ParamTraits::Read(&msg, &iter, &output)); +} diff --git a/url/url_constants.cc b/url/url_constants.cc index 2dc1478..0388fbc 100644 --- a/url/url_constants.cc +++ b/url/url_constants.cc @@ -25,4 +25,6 @@ const char kWssScheme[] = "wss"; const char kStandardSchemeSeparator[] = "://"; +const size_t kMaxURLChars = 2 * 1024 * 1024; + } // namespace url diff --git a/url/url_constants.h b/url/url_constants.h index c48dafc..fa71164 100644 --- a/url/url_constants.h +++ b/url/url_constants.h @@ -5,6 +5,8 @@ #ifndef URL_URL_CONSTANTS_H_ #define URL_URL_CONSTANTS_H_ +#include + #include "url/url_export.h" namespace url { @@ -30,6 +32,8 @@ URL_EXPORT extern const char kWssScheme[]; // Used to separate a standard scheme and the hostname: "://". URL_EXPORT extern const char kStandardSchemeSeparator[]; +URL_EXPORT extern const size_t kMaxURLChars; + } // namespace url #endif // URL_URL_CONSTANTS_H_ -- cgit v1.1