Commit f78d671d authored by rsleevi@chromium.org's avatar rsleevi@chromium.org

Revert 98885 - Third attempt at the following. Unreviewed, this was...

Revert 98885 - Third attempt at the following.  Unreviewed, this was originally reviewed in http://codereview.chromium.org/7550002 .

Clean up SSL false start blacklist code. Numerous changes, including:
* Handle trailing dots in LastTwoLabels() as in http://codereview.chromium.org/7518035/ . Rename this function to LastTwoComponents() to match the terminology used in the RegistryControlledDomainService and elsewhere in Chrome.
* Since callers are using std::string anyway, make the functions in the header take const std::string& instead of char*. This also allows doing string operations on them.
* Use string operations (like find_last_of()) in place of hand-written algorithms, for brevity, clarity, and safety.
* Avoid "unsigned", which the style guide forbids, and use allowed types like size_t, uint32, or int (depending on the situation).
* Avoid #define and "using".
* Use standard algorithms for similar reasons as using string ops.
* Use file_util functions to significantly abbreviate file reading/writing code.
* Use wmain() (on Windows) in combination with FilePath to avoid issues if the provided pathname has extended characters that don't flatten losslessly to the default codepage (thanks Darin for pointing out this issue).
* Avoid casting where possible. Avoid some casts for printf()-style calls by using a string stream, which also allows for slightly less boilerplate.
* Convert non-error uses of stderr to the chrome-standard VLOG(1).
* Correctly handle hostnames with trailing dots in the input file.
* In general, shorten code where possible.

Because this adds a dependency on base, and ssl_false_start_blacklist_process has the "#host" specifier in net.gyp, bradnelson tells me that base and its dependencies need an explicit "host, target" toolchain list for the Linux builds to work correctly. It would be nice if we could avoid this but I guess gyp would have to be smarter or something.

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/7804001

TBR=pkasting@chromium.org
Review URL: http://codereview.chromium.org/7778035

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@98917 0039d316-1c4b-4281-b951-d872f2087c98
parent 565d4100
...@@ -60,7 +60,6 @@ ...@@ -60,7 +60,6 @@
# base depends on base_static. # base depends on base_static.
'target_name': 'base_static', 'target_name': 'base_static',
'type': 'static_library', 'type': 'static_library',
'toolsets': ['host', 'target'],
'sources': [ 'sources': [
'base_switches.cc', 'base_switches.cc',
'base_switches.h', 'base_switches.h',
......
...@@ -450,7 +450,6 @@ ...@@ -450,7 +450,6 @@
{ {
'target_name': 'base', 'target_name': 'base',
'type': '<(component)', 'type': '<(component)',
'toolsets': ['host', 'target'],
'variables': { 'variables': {
'base_target': 1, 'base_target': 1,
}, },
...@@ -667,7 +666,6 @@ ...@@ -667,7 +666,6 @@
{ {
'target_name': 'symbolize', 'target_name': 'symbolize',
'type': 'static_library', 'type': 'static_library',
'toolsets': ['host', 'target'],
'variables': { 'variables': {
'chromium_code': 0, 'chromium_code': 0,
}, },
...@@ -693,7 +691,6 @@ ...@@ -693,7 +691,6 @@
{ {
'target_name': 'xdg_mime', 'target_name': 'xdg_mime',
'type': 'static_library', 'type': 'static_library',
'toolsets': ['host', 'target'],
'variables': { 'variables': {
'chromium_code': 0, 'chromium_code': 0,
}, },
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
{ {
'target_name': 'dynamic_annotations', 'target_name': 'dynamic_annotations',
'type': 'static_library', 'type': 'static_library',
'toolsets': ['host', 'target'],
'include_dirs': [ 'include_dirs': [
'../../../', '../../../',
], ],
......
...@@ -31,7 +31,6 @@ ...@@ -31,7 +31,6 @@
{ {
'target_name': 'gtk', 'target_name': 'gtk',
'type': 'settings', 'type': 'settings',
'toolsets': ['host', 'target'],
'conditions': [ 'conditions': [
['_toolset=="target"', { ['_toolset=="target"', {
'direct_dependent_settings': { 'direct_dependent_settings': {
...@@ -47,27 +46,12 @@ ...@@ -47,27 +46,12 @@
'<!@(<(pkg-config) --libs-only-l gtk+-2.0 gthread-2.0)', '<!@(<(pkg-config) --libs-only-l gtk+-2.0 gthread-2.0)',
], ],
}, },
}, { }],
'direct_dependent_settings': { [ 'chromeos==1', {
'cflags': [ 'link_settings': {
'<!@(pkg-config --cflags gtk+-2.0 gthread-2.0)', 'libraries': [ '-lXtst' ]
], }
}, }]]
'link_settings': {
'ldflags': [
'<!@(pkg-config --libs-only-L --libs-only-other gtk+-2.0 gthread-2.0)',
],
'libraries': [
'<!@(pkg-config --libs-only-l gtk+-2.0 gthread-2.0)',
],
},
}],
['chromeos==1', {
'link_settings': {
'libraries': [ '-lXtst' ]
}
}],
],
}, },
{ {
'target_name': 'gtkprint', 'target_name': 'gtkprint',
...@@ -87,8 +71,7 @@ ...@@ -87,8 +71,7 @@
'<!@(<(pkg-config) --libs-only-l gtk+-unix-print-2.0)', '<!@(<(pkg-config) --libs-only-l gtk+-unix-print-2.0)',
], ],
}, },
}], }]]
],
}, },
{ {
'target_name': 'ssl', 'target_name': 'ssl',
...@@ -165,8 +148,7 @@ ...@@ -165,8 +148,7 @@
'<!@(<(pkg-config) --libs-only-l freetype2)', '<!@(<(pkg-config) --libs-only-l freetype2)',
], ],
}, },
}], }]]
],
}, },
{ {
'target_name': 'fontconfig', 'target_name': 'fontconfig',
...@@ -186,8 +168,7 @@ ...@@ -186,8 +168,7 @@
'<!@(<(pkg-config) --libs-only-l fontconfig)', '<!@(<(pkg-config) --libs-only-l fontconfig)',
], ],
}, },
}], }]]
],
}, },
{ {
'target_name': 'gdk', 'target_name': 'gdk',
...@@ -207,8 +188,7 @@ ...@@ -207,8 +188,7 @@
'<!@(<(pkg-config) --libs-only-l gdk-2.0)', '<!@(<(pkg-config) --libs-only-l gdk-2.0)',
], ],
}, },
}], }]]
],
}, },
{ {
'target_name': 'gconf', 'target_name': 'gconf',
...@@ -231,8 +211,7 @@ ...@@ -231,8 +211,7 @@
'<!@(<(pkg-config) --libs-only-l gconf-2.0)', '<!@(<(pkg-config) --libs-only-l gconf-2.0)',
], ],
}, },
}], }]]
],
}, },
{ {
'target_name': 'gio', 'target_name': 'gio',
...@@ -267,13 +246,11 @@ ...@@ -267,13 +246,11 @@
}], }],
], ],
}, },
}], }]]
],
}, },
{ {
'target_name': 'x11', 'target_name': 'x11',
'type': 'settings', 'type': 'settings',
'toolsets': ['host', 'target'],
'conditions': [ 'conditions': [
['_toolset=="target"', { ['_toolset=="target"', {
'direct_dependent_settings': { 'direct_dependent_settings': {
...@@ -289,21 +266,7 @@ ...@@ -289,21 +266,7 @@
'<!@(<(pkg-config) --libs-only-l x11 xi)', '<!@(<(pkg-config) --libs-only-l x11 xi)',
], ],
}, },
}, { }],
'direct_dependent_settings': {
'cflags': [
'<!@(pkg-config --cflags x11)',
],
},
'link_settings': {
'ldflags': [
'<!@(pkg-config --libs-only-L --libs-only-other x11 xi)',
],
'libraries': [
'<!@(pkg-config --libs-only-l x11 xi)',
],
},
}],
], ],
}, },
{ {
...@@ -324,8 +287,7 @@ ...@@ -324,8 +287,7 @@
'<!@(<(pkg-config) --libs-only-l xext)', '<!@(<(pkg-config) --libs-only-l xext)',
], ],
}, },
}], }]]
],
}, },
{ {
'target_name': 'libgcrypt', 'target_name': 'libgcrypt',
...@@ -342,8 +304,7 @@ ...@@ -342,8 +304,7 @@
'<!@(libgcrypt-config --libs)', '<!@(libgcrypt-config --libs)',
], ],
}, },
}], }]]
],
}, },
{ {
'target_name': 'selinux', 'target_name': 'selinux',
...@@ -355,8 +316,7 @@ ...@@ -355,8 +316,7 @@
'-lselinux', '-lselinux',
], ],
}, },
}], }]]
],
}, },
{ {
'target_name': 'gnome_keyring', 'target_name': 'gnome_keyring',
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
{ {
'target_name': 'lastchange', 'target_name': 'lastchange',
'type': 'none', 'type': 'none',
'toolsets': ['host', 'target'],
'variables': { 'variables': {
'lastchange_out_path': '<(SHARED_INTERMEDIATE_DIR)/build/LASTCHANGE', 'lastchange_out_path': '<(SHARED_INTERMEDIATE_DIR)/build/LASTCHANGE',
'default_lastchange_path': '../LASTCHANGE.in', 'default_lastchange_path': '../LASTCHANGE.in',
......
...@@ -52,7 +52,7 @@ SSLConfigService::SSLConfigService() ...@@ -52,7 +52,7 @@ SSLConfigService::SSLConfigService()
// static // static
bool SSLConfigService::IsKnownFalseStartIncompatibleServer( bool SSLConfigService::IsKnownFalseStartIncompatibleServer(
const std::string& hostname) { const std::string& hostname) {
return SSLFalseStartBlacklist::IsMember(hostname); return SSLFalseStartBlacklist::IsMember(hostname.c_str());
} }
static bool g_cached_info_enabled = false; static bool g_cached_info_enabled = false;
......
// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Copyright (c) 2010 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
...@@ -7,19 +7,22 @@ ...@@ -7,19 +7,22 @@
namespace net { namespace net {
// static // static
bool SSLFalseStartBlacklist::IsMember(const std::string& host) { bool SSLFalseStartBlacklist::IsMember(const char* host) {
const std::string last_two_components(LastTwoComponents(host)); const char* last_two_labels = LastTwoLabels(host);
if (last_two_components.empty()) if (!last_two_labels)
return false; return false;
const unsigned bucket = Hash(last_two_labels) & (kBuckets - 1);
const uint32 start = kHashTable[bucket];
const uint32 end = kHashTable[bucket + 1];
const size_t len = strlen(host);
const size_t bucket = Hash(last_two_components) & (kBuckets - 1); for (size_t i = start; i < end;) {
for (size_t i = kHashTable[bucket]; i < kHashTable[bucket + 1]; ) {
const size_t blacklist_entry_len = static_cast<uint8>(kHashData[i]); const size_t blacklist_entry_len = static_cast<uint8>(kHashData[i]);
if (host.length() >= blacklist_entry_len && if (len >= blacklist_entry_len &&
!host.compare(host.length() - blacklist_entry_len, blacklist_entry_len, memcmp(&host[len - blacklist_entry_len], &kHashData[i + 1],
&kHashData[i + 1], blacklist_entry_len) && blacklist_entry_len) == 0 &&
(host.length() == blacklist_entry_len || (len == blacklist_entry_len ||
host[host.length() - blacklist_entry_len - 1] == '.')) { host[len - blacklist_entry_len - 1] == '.')) {
return true; return true;
} }
i += blacklist_entry_len + 1; i += blacklist_entry_len + 1;
......
...@@ -5,9 +5,7 @@ ...@@ -5,9 +5,7 @@
#ifndef NET_BASE_SSL_FALSE_START_BLACKLIST_H_ #ifndef NET_BASE_SSL_FALSE_START_BLACKLIST_H_
#define NET_BASE_SSL_FALSE_START_BLACKLIST_H_ #define NET_BASE_SSL_FALSE_START_BLACKLIST_H_
#include <string> #include "base/basictypes.h"
#include "base/logging.h"
#include "net/base/net_export.h" #include "net/base/net_export.h"
namespace net { namespace net {
...@@ -18,42 +16,64 @@ namespace net { ...@@ -18,42 +16,64 @@ namespace net {
// table for fast lookups. // table for fast lookups.
class SSLFalseStartBlacklist { class SSLFalseStartBlacklist {
public: public:
// Returns true if |host| (a DNS name in dotted form, e.g. "www.example.com") // IsMember returns true if the given host is in the blacklist.
// is in the blacklist. // host: a DNS name in dotted form (i.e. "www.example.com")
NET_EXPORT_PRIVATE static bool IsMember(const std::string& host); NET_EXPORT_PRIVATE static bool IsMember(const char* host);
// Returns the modified djb2 hash of |host|. // Hash returns the modified djb2 hash of the given string.
// NOTE: This is inline because the code which generates the hash table needs static unsigned Hash(const char* str) {
// to use it. However, the generating code cannot link against // This is inline because the code which generates the hash table needs to
// ssl_false_start_blacklist.cc because that needs the tables which it // use it. However, the generating code cannot link against
// generates. // ssl_false_start_blacklist.cc because that needs the tables which it
static uint32 Hash(const std::string& host) { // generates.
uint32 hash = 5381; const unsigned char* in = reinterpret_cast<const unsigned char*>(str);
for (const uint8* in = reinterpret_cast<const uint8*>(host.c_str()); unsigned hash = 5381;
*in != 0; ++in) unsigned char c;
hash = ((hash << 5) + hash) ^ *in;
while ((c = *in++))
hash = ((hash << 5) + hash) ^ c;
return hash; return hash;
} }
// Returns the last two dot-separated components of |host|, ignoring any // LastTwoLabels returns a pointer within |host| to the last two labels of
// trailing dots. For example, returns "c.d" for "a.b.c.d.". Returns an // |host|. For example, if |host| is "a.b.c.d" then LastTwoLabels will return
// empty string if |host| does not have two dot-separated components. // "c.d".
// NOTE: Inline for the same reason as Hash(). // host: a DNS name in dotted form.
static std::string LastTwoComponents(const std::string& host) { // returns: NULL on error, otherwise a pointer inside |host|.
size_t last_nondot = host.find_last_not_of('.'); static const char* LastTwoLabels(const char* host) {
if (last_nondot == std::string::npos) // See comment in |Hash| for why this function is inline.
return std::string(); const size_t len = strlen(host);
size_t last_dot = host.find_last_of('.', last_nondot); if (len == 0)
if ((last_dot == 0) || (last_dot == std::string::npos)) return NULL;
return std::string();
// NOTE: This next line works correctly even when the call returns npos. unsigned dots_found = 0;
size_t components_begin = host.find_last_of('.', last_dot - 1) + 1; size_t i;
return host.substr(components_begin, last_nondot - components_begin + 1); for (i = len - 1; i < len; i--) {
if (host[i] == '.') {
dots_found++;
if (dots_found == 2) {
i++;
break;
}
}
}
if (i > len)
i = 0;
if (dots_found == 0)
return NULL; // no names with less than two labels are in the blacklist.
if (dots_found == 1) {
if (host[0] == '.')
return NULL; // ditto
}
return &host[i];
} }
// This is the number of buckets in the blacklist hash table. (Must be a // This is the number of buckets in the blacklist hash table. (Must be a
// power of two). // power of two).
static const size_t kBuckets = 128; static const unsigned kBuckets = 128;
private: private:
// The following two members are defined in // The following two members are defined in
......
...@@ -7,17 +7,18 @@ ...@@ -7,17 +7,18 @@
namespace net { namespace net {
TEST(SSLFalseStartBlacklistTest, LastTwoComponents) { TEST(SSLFalseStartBlacklistTest, LastTwoLabels) {
EXPECT_EQ(SSLFalseStartBlacklist::LastTwoComponents("a.b.c.d"), "c.d"); #define F SSLFalseStartBlacklist::LastTwoLabels
EXPECT_EQ(SSLFalseStartBlacklist::LastTwoComponents("a.b"), "a.b"); EXPECT_STREQ(F("a.b.c.d"), "c.d");
EXPECT_EQ(SSLFalseStartBlacklist::LastTwoComponents("www.a.de"), "a.de"); EXPECT_STREQ(F("a.b"), "a.b");
EXPECT_EQ(SSLFalseStartBlacklist::LastTwoComponents("www.www.a.de"), "a.de"); EXPECT_STREQ(F("example.com"), "example.com");
EXPECT_EQ(SSLFalseStartBlacklist::LastTwoComponents("a.com."), "a.com"); EXPECT_STREQ(F("www.example.com"), "example.com");
EXPECT_EQ(SSLFalseStartBlacklist::LastTwoComponents("a.com.."), "a.com"); EXPECT_STREQ(F("www.www.example.com"), "example.com");
EXPECT_TRUE(SSLFalseStartBlacklist::LastTwoComponents("com").empty()); EXPECT_TRUE(F("com") == NULL);
EXPECT_TRUE(SSLFalseStartBlacklist::LastTwoComponents(".com").empty()); EXPECT_TRUE(F(".com") == NULL);
EXPECT_TRUE(SSLFalseStartBlacklist::LastTwoComponents("").empty()); EXPECT_TRUE(F("") == NULL);
#undef F
} }
TEST(SSLFalseStartBlacklistTest, IsMember) { TEST(SSLFalseStartBlacklistTest, IsMember) {
......
...@@ -205,7 +205,6 @@ ...@@ -205,7 +205,6 @@
'base/ssl_config_service_defaults.cc', 'base/ssl_config_service_defaults.cc',
'base/ssl_config_service_defaults.h', 'base/ssl_config_service_defaults.h',
'base/ssl_false_start_blacklist.cc', 'base/ssl_false_start_blacklist.cc',
'base/ssl_false_start_blacklist.h',
'base/ssl_info.cc', 'base/ssl_info.cc',
'base/ssl_info.h', 'base/ssl_info.h',
'base/static_cookie_policy.cc', 'base/static_cookie_policy.cc',
...@@ -1394,16 +1393,11 @@ ...@@ -1394,16 +1393,11 @@
'target_name': 'ssl_false_start_blacklist_process', 'target_name': 'ssl_false_start_blacklist_process',
'type': 'executable', 'type': 'executable',
'toolsets': ['host'], 'toolsets': ['host'],
'dependencies': [
'../base/base.gyp:base',
'../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
],
'include_dirs': [ 'include_dirs': [
'..', '..',
], ],
'sources': [ 'sources': [
'base/ssl_false_start_blacklist_process.cc', 'base/ssl_false_start_blacklist_process.cc',
'base/ssl_false_start_blacklist.h',
], ],
}, },
], ],
...@@ -1418,7 +1412,7 @@ ...@@ -1418,7 +1412,7 @@
], ],
'dependencies': [ 'dependencies': [
'../base/base.gyp:base', '../base/base.gyp:base',
'net', 'net.gyp:net',
'../third_party/openssl/openssl.gyp:openssl', '../third_party/openssl/openssl.gyp:openssl',
], ],
'sources': [ 'sources': [
...@@ -1480,7 +1474,7 @@ ...@@ -1480,7 +1474,7 @@
'type': 'static_library', 'type': 'static_library',
'dependencies': [ 'dependencies': [
'../base/base.gyp:base', '../base/base.gyp:base',
'net', 'net.gyp:net',
], ],
'sources': [ 'sources': [
'curvecp/circular_buffer.cc', 'curvecp/circular_buffer.cc',
...@@ -1515,8 +1509,8 @@ ...@@ -1515,8 +1509,8 @@
'type': 'executable', 'type': 'executable',
'dependencies': [ 'dependencies': [
'../base/base.gyp:base', '../base/base.gyp:base',
'curvecp', 'net.gyp:curvecp',
'net', 'net.gyp:net',
'net_test_support', 'net_test_support',
'../testing/gmock.gyp:gmock', '../testing/gmock.gyp:gmock',
'../testing/gtest.gyp:gtest', '../testing/gtest.gyp:gtest',
......
...@@ -13,7 +13,6 @@ ...@@ -13,7 +13,6 @@
'target_name': 'libevent', 'target_name': 'libevent',
'product_name': 'event', 'product_name': 'event',
'type': 'static_library', 'type': 'static_library',
'toolsets': ['host', 'target'],
'sources': [ 'sources': [
'buffer.c', 'buffer.c',
'evbuffer.c', 'evbuffer.c',
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
{ {
'target_name': 'modp_b64', 'target_name': 'modp_b64',
'type': 'static_library', 'type': 'static_library',
'toolsets': ['host', 'target'],
'sources': [ 'sources': [
'modp_b64.cc', 'modp_b64.cc',
'modp_b64.h', 'modp_b64.h',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment