[Author: dmaclach]

Change GTMGoogleSearch to not do escaping of URL arguments, because as it turns
out the client often wants to handle that themselves. Also, made it so that
you didn't have to have a query argument, and that you could override
other arguments by setting them to [NSNull null]. These changes make the API
far more flexible.

DELTA=48 (12 added, 23 deleted, 13 changed)
R=thomasvl

3 files changed, 65 insertions, 40 deletions

diff --git a/Foundation/GTMGoogleSearch.h b/Foundation/GTMGoogleSearch.h
index b8f4b5b..d18aadf 100644
--- a/Foundation/GTMGoogleSearch.h
+++ b/Foundation/GTMGoogleSearch.h
@@ -32,7 +32,8 @@
 #define GTMGoogleSearchBooks @"books"
 #define GTMGoogleSearchWeb @"search"
 
-// Composes URLs and searches for google properties in the correct language and domain.
+// Composes URLs and searches for google properties in the correct language 
+// and domain.
 @interface GTMGoogleSearch : NSObject {
   // the cached values
   NSString *allAppsCachedDomain_;
@@ -58,7 +59,11 @@
 // or override settings stored in globalSearchArguments.
 // example dictionary to do an I'm feeling lucky search would be:
 // [NSDictionary dictionaryWithObject:@"1" key:@"btnI"];
-//
+// If queryText is nil, no query will be put in. 
+// Arguments passed in in args must be properly URL escaped.
+// If you want to remove one of the arguments that will be included in the
+// global search arguments, set the object for the key you want to remove to
+// [NSNull null].
 - (NSString*)searchURLFor:(NSString *)queryText 
                    ofType:(NSString *)type
                 arguments:(NSDictionary *)args;
@@ -79,7 +84,8 @@
 // to strings where they are the args you want passed to all Google searches.
 // You can override these with your localArgs when you actually perform the
 // search if you wish.
-// This arguments will affect all searches.
+// This arguments will affect all searches. Arguments must be properly URL
+// escaped.
 - (void)setGlobalSearchArguments:(NSDictionary *)args;
 
 // Returns the global search arguments.
@@ -131,6 +137,4 @@
 //
 - (void)clearPreferredDomainAndLanguageForAllApps;
 
- 
-
 @end
diff --git a/Foundation/GTMGoogleSearch.m b/Foundation/GTMGoogleSearch.m
index 6bb5d58..586efd3 100644
--- a/Foundation/GTMGoogleSearch.m
+++ b/Foundation/GTMGoogleSearch.m
@@ -18,8 +18,6 @@
 
 #import "GTMGoogleSearch.h"
 #import "GTMObjectSingleton.h"
-#import "GTMNSString+URLArguments.h"
-#import "GTMMethodCheck.h"
 #import "GTMGarbageCollection.h"
 
 #if GTM_IPHONE_SDK
@@ -37,9 +35,6 @@ typedef struct {
 // this is a seed mapping from languages to domains for google search.
 // this doesn't have to be complete, as it is just a seed.
 //
-// initial values for the table were taken from the GDWin code.
-// (/googleclient/totalrecall/common/url_tools.cpp, but moved to
-// /google3/java/com/google/totalrecall/production/config/gpac.xml)
 //
 static LanguageDefaultInfo kLanguageListDefaultMappingTable[] = {
   // order is important, first match is taken
@@ -126,14 +121,11 @@ static NSString *const kSearchURLTemplate = @"http://www.google.%@/%@?%@";
              language:(NSString**)preferredLanguage;
 - (void)reloadAllAppCachedValues:(NSNotification*)notification;
 - (void)updateAllAppsDomain:(NSString*)domain language:(NSString*)language;
-- (NSDictionary *)normalizeGoogleArguments:(NSDictionary *)args;
 @end
 
 
 @implementation GTMGoogleSearch
 
-GTM_METHOD_CHECK(NSString, gtm_stringByEscapingForURLArgument);
-
 GTMOBJECT_SINGLETON_BOILERPLATE(GTMGoogleSearch, sharedInstance);
 
 - (id)init {
@@ -178,7 +170,7 @@ GTMOBJECT_SINGLETON_BOILERPLATE(GTMGoogleSearch, sharedInstance);
     
     NSDictionary *appArgs 
       = [bundle objectForInfoDictionaryKey:GTMGoogleSearchClientAppArgsKey];
-    globalSearchArguments_ = [[self normalizeGoogleArguments:appArgs] retain];
+    globalSearchArguments_ = [appArgs retain];
   }
   return self;
 }
@@ -309,19 +301,17 @@ GTMOBJECT_SINGLETON_BOILERPLATE(GTMGoogleSearch, sharedInstance);
 }
 
 - (void)setGlobalSearchArguments:(NSDictionary *)args {
-  args = [self normalizeGoogleArguments:args];
   [globalSearchArguments_ autorelease];
-  globalSearchArguments_ = [args retain];
+  globalSearchArguments_ = [args copy];
 }
 
 - (NSString*)searchURLFor:(NSString*)queryText
                    ofType:(NSString*)type 
                 arguments:(NSDictionary *)localArgs {
-  NSString *url = nil;
-  if (!queryText) {
+  if (!type) {
     return nil;
   }
-
+  
   NSString *language;
   NSString *domain;
   [self preferredDomain:&domain
@@ -333,28 +323,50 @@ GTMOBJECT_SINGLETON_BOILERPLATE(GTMGoogleSearch, sharedInstance);
        @"UTF-8", @"ie",
        @"UTF-8", @"oe",
        language, @"hl", 
-       [queryText gtm_stringByEscapingForURLArgument], @"q", 
        nil];
+  if (queryText) {
+    [args setObject:queryText forKey:@"q"];
+  }
   
   NSDictionary *globalSearchArgs = [self globalSearchArguments];
   if (globalSearchArgs) {
     [args addEntriesFromDictionary:globalSearchArgs];
   }
   if (localArgs) {
-    localArgs = [self normalizeGoogleArguments:localArgs];
     [args addEntriesFromDictionary:localArgs];
   }
   
   NSMutableArray *clientArgs = [NSMutableArray array];
   NSString *key;
+  NSNull *nsNull = [NSNull null];
   GTM_FOREACH_KEY(key, args) {
     NSString *object = [args objectForKey:key];
-    NSString *arg = [NSString stringWithFormat:@"%@=%@", key, object];
-    [clientArgs addObject:arg];
+    if (![object isEqualTo:nsNull]) {
+#if DEBUG
+      // In debug we check key and object for things that should be escaped.
+      // Note that percent is not in there because escaped strings will have
+      // percents in them
+      NSCharacterSet *cs = [NSCharacterSet characterSetWithCharactersInString:
+                            @"!*'();:@&=+$,/?#[] "];
+      NSRange range = [key rangeOfCharacterFromSet:cs];
+      if (range.location != NSNotFound) {
+        _GTMDevLog(@"Unescaped string %@ in argument pair {%@, %@} in -[%@ %@]", 
+                   key, key, object, [self class], NSStringFromSelector(_cmd));
+      }
+      range = [object rangeOfCharacterFromSet:cs];
+      if (range.location != NSNotFound) {
+        _GTMDevLog(@"Unescaped string %@ in argument pair {%@,%@ } in -[%@ %@]",
+                   object, key, object, [self class], 
+                   NSStringFromSelector(_cmd));
+      }
+#endif  // DEBUG
+      NSString *arg = [NSString stringWithFormat:@"%@=%@", key, object];
+      [clientArgs addObject:arg];
+    }
   }
   NSString *clientArg = [clientArgs componentsJoinedByString:@"&"];
-  url = [NSString stringWithFormat:kSearchURLTemplate,
-         domain, type, clientArg];
+  NSString *url = [NSString stringWithFormat:kSearchURLTemplate,
+                   domain, type, clientArg];
   return url;
 }
 
@@ -512,16 +524,4 @@ GTMOBJECT_SINGLETON_BOILERPLATE(GTMGoogleSearch, sharedInstance);
                           userInfo:nil];
 }
 
-- (NSDictionary *)normalizeGoogleArguments:(NSDictionary *)args {
-  NSMutableDictionary *outArgs = [NSMutableDictionary dictionary];
-  NSString *key;
-  GTM_FOREACH_KEY(key, args) {
-    NSString *object = [args objectForKey:key];
-    key = [[key gtm_stringByEscapingForURLArgument] lowercaseString];
-    object = [object gtm_stringByEscapingForURLArgument];
-    [outArgs setObject:object forKey:key];
-  }
-  return outArgs;
-}
-
 @end
diff --git a/Foundation/GTMGoogleSearchTest.m b/Foundation/GTMGoogleSearchTest.m
index 3e2ec79..8e737c9 100644
--- a/Foundation/GTMGoogleSearchTest.m
+++ b/Foundation/GTMGoogleSearchTest.m
@@ -18,6 +18,7 @@
 
 #import "GTMGoogleSearch.h"
 #import "GTMSenTestCase.h"
+#import "GTMUnitTestDevLog.h"
 #import <unistd.h>
 
 @interface GTMGoogleSearchTest : GTMTestCase
@@ -52,13 +53,13 @@
   
   size_t count = sizeof(testSearches) / sizeof(testSearches[0]);
   NSDictionary *globalArgs 
-    = [NSDictionary dictionaryWithObject:@"f" forKey:@"fo o"];
+    = [NSDictionary dictionaryWithObject:@"f" forKey:@"foo"];
   [googleSearch setGlobalSearchArguments:globalArgs];
-  NSDictionary *args = [NSDictionary dictionaryWithObject:@"Ba ba"
+  NSDictionary *args = [NSDictionary dictionaryWithObject:@"Baba"
                                                   forKey:@"BaR"];
   NSString *expectedStrings[] = { 
     @"oe=UTF-8", @"hl=yyy", @"q=Foobar", 
-    @"fo%20o=f", @"ie=UTF-8", @"bar=Ba%20ba" 
+    @"foo=f", @"ie=UTF-8", @"BaR=Baba" 
   };
   for (size_t i = 0; i < count; i++) {
     // test building the url
@@ -82,6 +83,26 @@
   [googleSearch clearPreferredDomainAndLanguageForCurrentApplication];
 }
 
+- (void)testBadInputs {
+  GTMGoogleSearch *googleSearch = [GTMGoogleSearch sharedInstance];
+  STAssertNotNil(googleSearch, nil);
+  NSDictionary *args = [NSDictionary dictionaryWithObject:@"Ba!ba"
+                                                   forKey:@"Ba=R"];
+  [GTMUnitTestDevLogDebug expectString:
+   @"Unescaped string Foo bar in argument pair {q,Foo bar } "
+   @"in -[GTMGoogleSearch searchURLFor:ofType:arguments:]"];
+  [GTMUnitTestDevLogDebug expectString:
+   @"Unescaped string Ba=R in argument pair {Ba=R, Ba!ba} "
+   @"in -[GTMGoogleSearch searchURLFor:ofType:arguments:]"];
+  [GTMUnitTestDevLogDebug expectString:
+   @"Unescaped string Ba!ba in argument pair {Ba=R,Ba!ba } "
+   @"in -[GTMGoogleSearch searchURLFor:ofType:arguments:]"];
+  NSString *urlString = [googleSearch searchURLFor:@"Foo bar" 
+                                            ofType:GTMGoogleSearchFroogle
+                                         arguments:args];
+  STAssertNotNil(urlString, nil);
+}
+  
 - (void)testPreferredDefaults {
   GTMGoogleSearch *googleSearch = [GTMGoogleSearch sharedInstance];
   STAssertNotNil(googleSearch, nil);