Check the return value of base::StringToInt() in

net/proxy/proxy_config_service_linux.cc

Index: net/proxy/proxy_config_service_linux.cc
diff --git a/net/proxy/proxy_config_service_linux.cc b/net/proxy/proxy_config_service_linux.cc
index bf852420a231baa438bef167a2100a844222286f..4a53ae59d341920591c8a1dbc0500d2d9c9023e4 100644
--- a/net/proxy/proxy_config_service_linux.cc
+++ b/net/proxy/proxy_config_service_linux.cc
@@ -856,6 +856,15 @@ bool SettingGetterImplGSettings::LoadAndCheckVersion(
 }
 #endif  // defined(USE_GIO)
 
+// Converts |value| from a decimal string to an int. If there was a failure
+// parsing, returns |default_value|.
+int StringToIntOrDefault(base::StringPiece value, int default_value) {
+  int result;
+  if (base::StringToInt(value, &result))
+    return result;
+  return default_value;
+}
+
 // This is the KDE version that reads kioslaverc and simulates gconf.
 // Doing this allows the main Delegate code, as well as the unit tests
 // for it, to stay the same - and the settings map fairly well besides.
@@ -1088,11 +1097,8 @@ class SettingGetterImplKDE : public ProxyConfigServiceLinux::SettingGetter {
       const char* mode = "none";
       indirect_manual_ = false;
       auto_no_pac_ = false;
-      int int_value;
-      base::StringToInt(value, &int_value);
+      int int_value = StringToIntOrDefault(value, 0);
       switch (int_value) {
-        case 0:  // No proxy, or maybe kioslaverc syntax error.
-          break;
         case 1:  // Manual configuration.
           mode = "manual";
           break;
@@ -1107,6 +1113,8 @@ class SettingGetterImplKDE : public ProxyConfigServiceLinux::SettingGetter {
           mode = "manual";
           indirect_manual_ = true;
           break;
+        default:  // No proxy, or maybe kioslaverc syntax error.

[eroman, 2017/06/29 21:11:18]

(Previously didn't account for negative numbers)

+          break;
       }
       string_table_[PROXY_MODE] = mode;
     } else if (key == "Proxy Config Script") {
@@ -1124,17 +1132,14 @@ class SettingGetterImplKDE : public ProxyConfigServiceLinux::SettingGetter {
       AddProxy(PROXY_SOCKS_HOST, value);
     } else if (key == "ReversedException") {
       // We count "true" or any nonzero number as true, otherwise false.
-      // Note that if the value is not actually numeric StringToInt()
-      // will return 0, which we count as false.
-      int int_value;
-      base::StringToInt(value, &int_value);
+      // A failure parsing the integer will interpret it as 0 (false).
+      int int_value = StringToIntOrDefault(value, 0);
       reversed_bypass_list_ = (value == "true" || int_value);

[eroman, 2017/06/29 21:11:18]

(Some of this parsing is weird... but I am just trying to address the specific
use of StringToInt() in this CL, so meh).

[mmenke, 2017/06/29 22:21:20]

Could you are least change this to:

reversed_bypass_list_ = (value == "true" || StringToIntOrDefault(value, 0) !=
0);

"bool || int" is just weird, and brings back traumatizing memories of writing
Javascript.

[eroman, 2017/06/29 22:33:58]

Done.

     } else if (key == "NoProxyFor") {
       AddHostList(PROXY_IGNORE_HOSTS, value);
     } else if (key == "AuthMode") {
       // Check for authentication, just so we can warn.
-      int mode;
-      base::StringToInt(value, &mode);
+      int mode = StringToIntOrDefault(value, 0);
       if (mode) {
         // ProxyConfig does not support authentication parameters, but
         // Chrome will prompt for the password later. So we ignore this.