************************* G4 reminder *************************
These new files:

        
/Users/playmobil/Documents/dev/gears/googleclient/gears/opensource/gears/base/safari/proxy_resolver_mac.cc

are missing unit tests.
***************************************************************

Hello michaeln, avi,

I'd like you to do a code review.  Please execute
        g4 diff -c 9015439

or point your web browser to
        http://mondrian/9015439
(this changelist has been uploaded to Mondrian)

to review the following code:

Change 9015439 by [EMAIL PROTECTED] on 2008/11/14 16:02:10 *pending*

        Fix bug 764 - pass System proxy configuration to CURL.
        
        PRESUBMIT=passed
        R=michaeln,avi
        [EMAIL PROTECTED]
        DELTA=314  (313 added, 0 deleted, 1 changed)
        OCL=9015439

Affected files ...

... //depot/googleclient/gears/opensource/gears/Makefile#209 edit
... 
//depot/googleclient/gears/opensource/gears/base/safari/curl_downloader.mm#3 
edit
... 
//depot/googleclient/gears/opensource/gears/base/safari/proxy_resolver_mac.cc#1 
add
... 
//depot/googleclient/gears/opensource/gears/base/safari/proxy_resolver_mac.h#1 
add
... //depot/googleclient/gears/opensource/gears/tools/config.mk#90 edit

314 delta lines: 313 added, 0 deleted, 1 changed

Also consider running:
        g4 lint -c 9015439

which verifies that the changelist doesn't introduce new style violations.

If you can't do the review, please let me know as soon as possible.  During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately.  Visit
http://www/eng/code_review.html for more information.

This is a semiautomated message from "g4 mail".  Complaints or suggestions?
Mail [EMAIL PROTECTED]
Change 9015439 by [EMAIL PROTECTED] on 2008/11/14 16:02:10 *pending*

        Fix bug 764 - pass System proxy configuration to CURL.

Affected files ...

... //depot/googleclient/gears/opensource/gears/Makefile#209 edit
... 
//depot/googleclient/gears/opensource/gears/base/safari/curl_downloader.mm#3 
edit
... 
//depot/googleclient/gears/opensource/gears/base/safari/proxy_resolver_mac.cc#1 
add
... 
//depot/googleclient/gears/opensource/gears/base/safari/proxy_resolver_mac.h#1 
add
... //depot/googleclient/gears/opensource/gears/tools/config.mk#90 edit

==== //depot/googleclient/gears/opensource/gears/Makefile#209 - 
/Users/playmobil/Documents/dev/gears/googleclient/gears/opensource/gears/Makefile
 ====
# action=edit type=text
--- googleclient/gears/opensource/gears/Makefile        2008-11-14 
16:02:18.000000000 -0800
+++ googleclient/gears/opensource/gears/Makefile        2008-11-14 
14:30:07.000000000 -0800
@@ -1177,6 +1177,7 @@
                ipc_message_queue_linux.cc \
                ipc_message_queue_test.cc \
                ipc_message_queue_test_osx.mm \
+               proxy_resolver_mac.cc \
                js_runner_np.cc \
                js_standalone_engine_mozjs.cc \
                messagebox.mm \
==== 
//depot/googleclient/gears/opensource/gears/base/safari/curl_downloader.mm#3 - 
/Users/playmobil/Documents/dev/gears/googleclient/gears/opensource/gears/base/safari/curl_downloader.mm
 ====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/safari/curl_downloader.mm  
2008-11-14 16:02:18.000000000 -0800
+++ googleclient/gears/opensource/gears/base/safari/curl_downloader.mm  
2008-11-14 15:50:36.000000000 -0800
@@ -30,6 +30,7 @@
 #import "gears/base/common/scoped_token.h"
 #import "gears/base/npapi/browser_utils.h"
 #import "gears/base/safari/curl_downloader.h"
+#import "gears/base/safari/proxy_resolver_mac.h"
 #import "gears/localserver/common/http_cookies.h"
 
 static bool curl_was_initialised = false;
@@ -119,6 +120,11 @@
     return false;
   }
   
+  // Proxies
+  if (!SetupCURLProxies(url, handle.get())) {
+    return false;
+  }
+  
   // Fixup cookie formatting from ';' delimited to '; ' delimited.
   ReplaceAll(cookie_utf16,
              std::string16(STRING16(L";")),
==== 
//depot/googleclient/gears/opensource/gears/base/safari/proxy_resolver_mac.cc#1 
- 
/Users/playmobil/Documents/dev/gears/googleclient/gears/opensource/gears/base/safari/proxy_resolver_mac.cc
 ====
# action=add type=text
--- /dev/null   1969-12-31 16:00:00.000000000 -0800
+++ googleclient/gears/opensource/gears/base/safari/proxy_resolver_mac.cc       
2008-11-14 16:09:47.000000000 -0800
@@ -0,0 +1,271 @@
+// Copyright 2008, Google Inc.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+//  1. Redistributions of source code must retain the above copyright notice,
+//     this list of conditions and the following disclaimer.
+//  2. Redistributions in binary form must reproduce the above copyright 
notice,
+//     this list of conditions and the following disclaimer in the 
documentation
+//     and/or other materials provided with the distribution.
+//  3. Neither the name of Google Inc. nor the names of its contributors may be
+//     used to endorse or promote products derived from this software without
+//     specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED
+// WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
+// EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
+// OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+// OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+// ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include "gears/base/safari/proxy_resolver_mac.h"
+
+#include <CoreFoundation/CoreFoundation.h>
+#include <CoreServices/CoreServices.h>
+#include <SystemConfiguration/SystemConfiguration.h>
+#include </usr/include/curl/curl.h>
+#include <vector>
+
+#include "third_party/googleurl/src/gurl.h"
+#include "gears/base/common/string_utils_osx.h"
+#include "gears/base/safari/scoped_cf.h"
+
+namespace {
+
+// Convert the supplied CFString into the specified encoding, and return it as
+// an STL string of the template type.  Returns an empty string on failure.
+//
+// Do not assert in this function since it is used by the asssertion code!
+template<typename StringType>
+static StringType CFStringToSTLStringWithEncodingT(CFStringRef cfstring,
+                                                   CFStringEncoding encoding) {
+  CFIndex length = CFStringGetLength(cfstring);
+  if (length == 0)
+    return StringType();
+
+  CFRange whole_string = CFRangeMake(0, length);
+  CFIndex out_size;
+  CFIndex converted = CFStringGetBytes(cfstring,
+                                       whole_string,
+                                       encoding,
+                                       0,      // lossByte
+                                       false,  // isExternalRepresentation
+                                       NULL,   // buffer
+                                       0,      // maxBufLen
+                                       &out_size);
+  if (converted == 0 || out_size == 0)
+    return StringType();
+
+  // out_size is the number of UInt8-sized units needed in the destination.
+  // A buffer allocated as UInt8 units might not be properly aligned to
+  // contain elements of StringType::value_type.  Use a container for the
+  // proper value_type, and convert out_size by figuring the number of
+  // value_type elements per UInt8.  Leave room for a NUL terminator.
+  typename StringType::size_type elements =
+      out_size * sizeof(UInt8) / sizeof(typename StringType::value_type) + 1;
+
+  std::vector<typename StringType::value_type> out_buffer(elements);
+  converted = CFStringGetBytes(cfstring,
+                               whole_string,
+                               encoding,
+                               0,      // lossByte
+                               false,  // isExternalRepresentation
+                               reinterpret_cast<UInt8*>(&out_buffer[0]),
+                               out_size,
+                               NULL);  // usedBufLen
+  if (converted == 0)
+    return StringType();
+
+  out_buffer[elements - 1] = '\0';
+  return StringType(&out_buffer[0], elements - 1);
+}
+
+std::string SysCFStringRefToUTF8(CFStringRef ref) {
+  return CFStringToSTLStringWithEncodingT<std::string>(ref,
+                                                       kCFStringEncodingUTF8);
+}
+
+// Utility function to pull out a value from a dictionary, check its type, and
+// return it.  Returns NULL if the key is not present or of the wrong type.
+CFTypeRef GetValueFromDictionary(CFDictionaryRef dict,
+                                 CFStringRef key,
+                                 CFTypeID expected_type) {
+  CFTypeRef value = CFDictionaryGetValue(dict, key);
+  if (!value)
+    return value;
+  
+  if (CFGetTypeID(value) != expected_type) {
+    scoped_cftype<CFStringRef> expected_type_ref(
+        CFCopyTypeIDDescription(expected_type));
+    scoped_cftype<CFStringRef> actual_type_ref(
+        CFCopyTypeIDDescription(CFGetTypeID(value)));
+//    LOG(WARNING) << "Expected value for key "
+//                 << SysCFStringRefToUTF8(key)
+//                 << " to be "
+//                 << SysCFStringRefToUTF8(expected_type_ref)
+//                 << " but it was "
+//                 << SysCFStringRefToUTF8(actual_type_ref)
+//                 << " instead";
+    return NULL;
+  }
+  
+  return value;
+}
+
+// Utility function to pull out a boolean value from a dictionary and return 
it,
+// returning a default value if the key is not present.
+bool GetBoolFromDictionary(CFDictionaryRef dict,
+                           CFStringRef key,
+                           bool default_value) {
+  CFNumberRef number = (CFNumberRef)GetValueFromDictionary(dict, key,
+                                                           
CFNumberGetTypeID());
+  if (!number)
+    return default_value;
+  
+  int int_value;
+  if (CFNumberGetValue(number, kCFNumberIntType, &int_value))
+    return int_value;
+  else
+    return default_value;
+}
+
+// Utility function to pull out a host/port pair from a dictionary and format
+// them into a "<host>[:<port>]" style string. Pass in a dictionary that has a
+// value for the host key and optionally a value for the port key. Returns a
+// formatted string. In the error condition where the host value is especially
+// malformed, returns an empty string. (You may still want to check for that
+// result anyway.)
+std::string GetHostPortFromDictionary(CFDictionaryRef dict,
+                                      CFStringRef host_key,
+                                      CFStringRef port_key) {
+  std::string result;
+  
+  CFStringRef host_ref =
+      (CFStringRef)GetValueFromDictionary(dict, host_key,
+                                          CFStringGetTypeID());
+  if (!host_ref) {
+//    LOG(WARNING) << "Could not find expected key "
+//                 << SysCFStringRefToUTF8(host_key)
+//                 << " in the proxy dictionary";
+    return result;
+  }
+  result = SysCFStringRefToUTF8(host_ref);
+  
+  CFNumberRef port_ref =
+      (CFNumberRef)GetValueFromDictionary(dict, port_key,
+                                          CFNumberGetTypeID());
+  if (port_ref) {
+    int port;
+    CFNumberGetValue(port_ref, kCFNumberIntType, &port);
+    result += ":";
+    result += IntegerToString(port);
+  }
+  
+  return result;
+}
+
+} // namespace
+
+// static
+bool SetupCURLProxies(const std::string16 &request_url, CURL* curl_handle) {
+  scoped_cftype<CFDictionaryRef> config_dict(
+      SCDynamicStoreCopyProxies(NULL));
+  
+// TODO(playmobil): The Chromium project has some code that can parse pac files
+// but it uses 10.5 only APIs, we can incorporate it into Gears once we drop
+// 10.4 support.
+
+// PAC file  
+//  if (GetBoolFromDictionary(config_dict.get(),
+//                            kSCPropNetProxiesProxyAutoConfigEnable,
+//                            false)) {
+//    CFStringRef pac_url_ref =
+//        (CFStringRef)GetValueFromDictionary(
+//            config_dict.get(),
+//            kSCPropNetProxiesProxyAutoConfigURLString,
+//            CFStringGetTypeID());
+//    if (pac_url_ref)
+//      config->pac_url = SysCFStringRefToUTF8(pac_url_ref);
+//  }
+
+  // Parse the url
+  GURL request_url_gurl(request_url.c_str());
+
+  // TODO(playmobil): respect bypass local names
+//  bool bypass_local_namse = GetBoolFromDictionary(config_dict.get(),
+//                              kSCPropNetProxiesExcludeSimpleHostnames,
+//                                                  false);
+
+
+  // Check if this URL is on the proxy bypass list.
+  CFArrayRef bypass_array_ref =
+    (CFArrayRef)GetValueFromDictionary(config_dict.get(),
+                                       kSCPropNetProxiesExceptionsList,
+                                       CFArrayGetTypeID());
+  if (bypass_array_ref) {
+    CFIndex bypass_array_count = CFArrayGetCount(bypass_array_ref);
+    for (CFIndex i = 0; i < bypass_array_count; ++i) {
+      CFStringRef bypass_item_ref =
+          (CFStringRef)CFArrayGetValueAtIndex(bypass_array_ref, i);
+      if (CFGetTypeID(bypass_item_ref) != CFStringGetTypeID()) {
+//        LOG(("Expected value for item in the kSCPropNetProxiesExceptionsList"
+//                        " to be a CFStringRef but it was not"));
+        
+      } else {
+        std::string bypass_domain_name = SysCFStringRefToUTF8(bypass_item_ref);
+        
+        if (request_url_gurl.DomainIs(bypass_domain_name.c_str())) {
+          return true;
+        }
+      }
+    }
+  }
+
+  // SOCKS proxy
+  if (GetBoolFromDictionary(config_dict.get(),
+                            kSCPropNetProxiesSOCKSEnable,
+                            false)) {  
+    
+    std::string host_port =
+      GetHostPortFromDictionary(config_dict.get(),
+                                kSCPropNetProxiesSOCKSProxy,
+                                kSCPropNetProxiesSOCKSPort);
+                                
+    // TODO(playmobil): Can we tell if this is a SOCKS V4 or V5 proxy?
+    curl_easy_setopt(curl_handle, CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);
+    curl_easy_setopt(curl_handle, CURLOPT_PROXY, host_port.c_str());
+
+    return true;
+  }
+  
+  // Otherwise Try HTTP or HTTPS proxies.
+  // TODO(playmobil): is the failover to an HTTP proxy OK?
+  
+  if (request_url_gurl.SchemeIsSecure() && 
+      GetBoolFromDictionary(config_dict.get(),
+                            kSCPropNetProxiesHTTPSEnable,
+                            false)) {
+    std::string host_port =
+        GetHostPortFromDictionary(config_dict.get(),
+                                  kSCPropNetProxiesHTTPSProxy,
+                                  kSCPropNetProxiesHTTPSPort);
+    curl_easy_setopt(curl_handle, CURLOPT_PROXY, host_port.c_str());
+    return true;
+  } else if (GetBoolFromDictionary(config_dict.get(),
+                            kSCPropNetProxiesHTTPEnable,
+                            false)) {
+    std::string host_port =
+        GetHostPortFromDictionary(config_dict.get(),
+                                  kSCPropNetProxiesHTTPProxy,
+                                  kSCPropNetProxiesHTTPPort);
+    curl_easy_setopt(curl_handle, CURLOPT_PROXY, host_port.c_str());
+    return true;
+  }
+  
+  return true;
+}
==== 
//depot/googleclient/gears/opensource/gears/base/safari/proxy_resolver_mac.h#1 
- 
/Users/playmobil/Documents/dev/gears/googleclient/gears/opensource/gears/base/safari/proxy_resolver_mac.h
 ====
# action=add type=text
--- /dev/null   1969-12-31 16:00:00.000000000 -0800
+++ googleclient/gears/opensource/gears/base/safari/proxy_resolver_mac.h        
2008-11-14 16:08:54.000000000 -0800
@@ -0,0 +1,35 @@
+// Copyright 2008, Google Inc.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+//  1. Redistributions of source code must retain the above copyright notice,
+//     this list of conditions and the following disclaimer.
+//  2. Redistributions in binary form must reproduce the above copyright 
notice,
+//     this list of conditions and the following disclaimer in the 
documentation
+//     and/or other materials provided with the distribution.
+//  3. Neither the name of Google Inc. nor the names of its contributors may be
+//     used to endorse or promote products derived from this software without
+//     specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED
+// WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
+// EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
+// OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+// OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+// ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#ifndef GEARS_BASE_SAFARI_PROXY_RESOLVER_MAC_H__
+#define GEARS_BASE_SAFARI_PROXY_RESOLVER_MAC_H__
+
+#include </usr/include/curl/curl.h>
+
+#include "gears/base/common/string16.h"
+
+bool SetupCURLProxies(const std::string16 &request_url, CURL* curl_handle);
+
+#endif  // GEARS_BASE_SAFARI_PROXY_RESOLVER_MAC_H__
==== //depot/googleclient/gears/opensource/gears/tools/config.mk#90 - 
/Users/playmobil/Documents/dev/gears/googleclient/gears/opensource/gears/tools/config.mk
 ====
# action=edit type=text
--- googleclient/gears/opensource/gears/tools/config.mk 2008-11-14 
16:02:18.000000000 -0800
+++ googleclient/gears/opensource/gears/tools/config.mk 2008-11-14 
15:52:02.000000000 -0800
@@ -479,7 +479,7 @@
 MKDLL = g++
 DLL_PREFIX = lib
 DLL_SUFFIX = .dylib
-DLLFLAGS = $(SHARED_LINKFLAGS) -bundle -framework Carbon -framework 
CoreServices -framework Cocoa
+DLLFLAGS = $(SHARED_LINKFLAGS) -bundle -framework Carbon -framework 
CoreServices -framework Cocoa -framework SystemConfiguration
 ifeq ($(BROWSER),SF)
 DLLFLAGS += -mmacosx-version-min=10.4 -framework WebKit -lcurl
 DLLFLAGS += -Ltools/osx -lleopard_support

Reply via email to