Bug 430070 - Give a specific error message for POST search engines. Patch by Sean Murphy <murph@seanmurph.com>, r=smorgan, sr=pink

git-svn-id: svn://10.0.0.236/trunk@251465 18797224-902f-48f8-a5cc-f745e15eee43

mozilla/camino/resources/localized/English.lproj/Localizable.strings.in

@@ -465,7 +465,7 @@
 /* Web Search Plug-ins */
 "InstallSearchPluginMenuItem" = "Add “%@”";
 "SearchPluginInstallationErrorTitle" = "Problem adding the search engine.";
-"SearchPluginInstallationErrorMessage" = "Camino was unable to add “%@” to the web search field. The search plug-in could be unavailable or in an unsupported format.";
+"SearchPluginInstallationErrorMessage" = "Camino was unable to add “%1$@” to the web search field. %2$@";
 "UnknownSearchPluginName" = "Unknown Plug-in";
 "SearchPluginInstallationConfirmationTitle" = "Add search engine?";
 "SearchPluginInstallationConfirmationMessage" = "Would you like to add the search engine “%@” to the search field?";
@@ -474,6 +474,12 @@
 "SearchPluginAlreadyInstalledTitle" = "Duplicate search engine.";
 "SearchPluginAlreadyInstalledMessage" = "The search engine “%@” is already installed.";
 
+/* XMLSearchPluginParser Error Descriptions: */
+"XMLSearchPluginParserInvalidPluginFormat" = "The search plug-in is in an unrecognized or unsupported format.";
+"XMLSearchPluginParserPluginNotFound" = "The search plug-in could not be found on the server.";
+"XMLSearchPluginParserUnsupportedSearchURL" = "The search method used by this plug-in is not yet supported.";
+"XMLSearchPluginParserUnknownError" = "The search plug-in could be unavailable or in an unsupported format.";
+
 /* Auto-Update */
 "AutoUpdateDisabledToolTip" = "Automatic updates are available in official builds";
 

mozilla/camino/src/browser/BrowserWindowController.mm

@@ -4668,7 +4668,8 @@ public:
 - (IBAction)installSearchPlugin:(id)sender
 {
   id searchPlugin = [sender representedObject];
-  BOOL addedOK = [[SearchEngineManager sharedSearchEngineManager] addSearchEngineFromPlugin:searchPlugin];
+  NSError *parsingError;
+  BOOL addedOK = [[SearchEngineManager sharedSearchEngineManager] addSearchEngineFromPlugin:searchPlugin error:&parsingError];
 
   if (addedOK) {
     // Start using the installed engine.
@@ -4680,10 +4681,15 @@ public:
   }
   else {
     NSString* searchPluginName = [searchPlugin valueForKey:kWebSearchPluginNameKey];
+    if (!searchPluginName)
+      searchPluginName = NSLocalizedString(@"UnknownSearchPluginName", nil);
     NSAlert* alert = [[[NSAlert alloc] init] autorelease];
     [alert addButtonWithTitle:NSLocalizedString(@"OKButtonText", nil)];
     [alert setMessageText:NSLocalizedString(@"SearchPluginInstallationErrorTitle", nil)];
-    [alert setInformativeText:[NSString stringWithFormat:NSLocalizedString(@"SearchPluginInstallationErrorMessage", nil), searchPluginName]];
+    NSString* explanatoryText = [NSString stringWithFormat:NSLocalizedString(@"SearchPluginInstallationErrorMessage", nil),
+                                                           searchPluginName,
+                                                           [parsingError localizedDescription]];
+    [alert setInformativeText:explanatoryText];
     [alert setAlertStyle:NSWarningAlertStyle];
     if ([[self window] attachedSheet]) {
       [alert runModal];

mozilla/camino/src/websearch/AddSearchProviderHandler.mm

@@ -117,10 +117,12 @@ NS_IMETHODIMP AddSearchProviderHandler::AddSearchProvider(const nsAString &aDesc
 
     XMLSearchPluginParser* pluginParser =
       [XMLSearchPluginParser searchPluginParserWithMIMEType:kOpenSearchMIMEType];
-    if (![pluginParser parseSearchPluginAtURL:[NSURL URLWithString:searchDescriptionURL]]) {
+    NSError *parsingError;
+    if (![pluginParser parseSearchPluginAtURL:[NSURL URLWithString:searchDescriptionURL] error:&parsingError]) {
       NSString* explanatoryText =
         [NSString stringWithFormat:NSLocalizedString(@"SearchPluginInstallationErrorMessage", nil),
-                                   NSLocalizedString(@"UnknownSearchPluginName", nil)];
+                                   NSLocalizedString(@"UnknownSearchPluginName", nil),
+                                   [parsingError localizedDescription]];
       NSAlert* parseErrorAlert = [[[NSAlert alloc] init] autorelease];
       [parseErrorAlert addButtonWithTitle:NSLocalizedString(@"OKButtonText", nil)];
       [parseErrorAlert setMessageText:NSLocalizedString(@"SearchPluginInstallationErrorTitle", nil)];

mozilla/camino/src/websearch/OpenSearchParser.mm

@@ -115,12 +115,14 @@ static NSString *const kDefaultLanguage = @"*"; // '*' is defined in OS spec to
     if (!method || [method isEqualToString:@""])
       method = @"GET";
 
-    if ([self browserSupportsSearchQueryURLWithMIMEType:mimeType requestMethod:method] &&
+    // The spec alows multiple <Url> elements, so we can't just abort parsing if this one isn't supported.
+    if ([self browserSupportsSearchQueryURLWithMIMEType:mimeType] &&
         [attributeDict objectForKey:@"template"])
     {
       NSMutableString *searchURLTemplate = [NSMutableString stringWithString:[attributeDict objectForKey:@"template"]];
       [self insertValuesForParametersInURLTemplate:searchURLTemplate];
       [self setSearchEngineURL:searchURLTemplate];
+      [self setSearchEngineURLRequestMethod:method];
     }
   }
   else if ([elementName isEqualToString:kSearchEngineURLParameterElement]) {

mozilla/camino/src/websearch/SearchEngineManager.h

@@ -77,7 +77,9 @@ extern NSString *const kWebSearchEngineWhereFromKey;
 
 // Adds the plugin to the end of |installedSearchEngines|.
 // Return value indicates whether the plugin was successfully parsed and a new engine added.
-- (BOOL)addSearchEngineFromPlugin:(NSDictionary *)searchPluginInfoDict;
+// If NO is returned, |outError| is populated with an NSError object containing a localized
+// description of the problem. Pass NULL if you do not want error information.
+- (BOOL)addSearchEngineFromPlugin:(NSDictionary *)searchPluginInfoDict error:(NSError**)error;
 
 - (BOOL)hasSearchEngineFromPluginURL:(NSString *)pluginURL;
 - (NSDictionary *)searchEngineFromPluginURL:(NSString *)pluginURL;

mozilla/camino/src/websearch/SearchEngineManager.mm

@@ -298,14 +298,25 @@ static NSString *const kPreferredSearchEngineNameKey = @"PreferredSearchEngine";
     [self installedSearchEnginesChanged];
 }
 
-- (BOOL)addSearchEngineFromPlugin:(NSDictionary *)searchPluginInfoDict
+- (BOOL)addSearchEngineFromPlugin:(NSDictionary *)searchPluginInfoDict error:(NSError **)outError
 {
+  if (outError)
+    *outError = nil;
+
   XMLSearchPluginParser *pluginParser = [XMLSearchPluginParser searchPluginParserWithMIMEType:[searchPluginInfoDict objectForKey:kWebSearchPluginMIMETypeKey]];
-  if (!pluginParser)
+  if (!pluginParser) {
+    if (outError) {
+      NSDictionary *errorInfo = [NSDictionary dictionaryWithObject:NSLocalizedString(@"XMLSearchPluginParserInvalidPluginFormat", nil)
+                                                            forKey:NSLocalizedDescriptionKey];
+      *outError = [NSError errorWithDomain:kXMLSearchPluginParserErrorDomain 
+                                      code:eXMLSearchPluginParserInvalidPluginFormatError 
+                                  userInfo:errorInfo];
+    }
     return NO;
+  }
 
   NSURL *pluginURL = [searchPluginInfoDict objectForKey:kWebSearchPluginURLKey];
-  BOOL parsedOk = [pluginParser parseSearchPluginAtURL:pluginURL];
+  BOOL parsedOk = [pluginParser parseSearchPluginAtURL:pluginURL error:outError];
 
   if (parsedOk) {
     [self addSearchEngineWithName:[pluginParser searchEngineName]

mozilla/camino/src/websearch/XMLSearchPluginParser.h

@@ -45,6 +45,17 @@ extern NSString *const kWebSearchPluginURLKey;
 // Supported MIME types:
 extern NSString *const kOpenSearchMIMEType;
 
+// For use with XMLSearchPluginParser's error reporting:
+extern NSString *const kXMLSearchPluginParserErrorDomain;
+typedef enum {
+  // The search query URL template used by the plugin is not supported by the browser (e.g. it uses a POST method type):
+  eXMLSearchPluginParserUnsupportedSearchURLError,
+  // The search plugin description file could not be found on the server:
+  eXMLSearchPluginParserPluginNotFoundError,
+  // Indicates a parsing error, meaning the plugin is invalid for the MIME type it represents:
+  eXMLSearchPluginParserInvalidPluginFormatError
+} EXMLSearchPluginParserErrorCode;
+
 //
 // XMLSearchPluginParser
 //
@@ -63,8 +74,9 @@ extern NSString *const kOpenSearchMIMEType;
 
   BOOL            mShouldParseContentsOfCurrentElement;
 
-  NSString        *mSearchEngineName; // strong
-  NSString        *mSearchEngineURL;  // strong
+  NSString        *mSearchEngineName;             // strong
+  NSString        *mSearchEngineURL;              // strong
+  NSString        *mSearchEngineURLRequestMethod; // strong
 
   NSMutableString *mCurrentElementBuffer;
 }
@@ -75,11 +87,14 @@ extern NSString *const kOpenSearchMIMEType;
 + (id)searchPluginParserWithMIMEType:(NSString *)mimeType;
 - (id)initWithPluginMIMEType:(NSString *)mimeType;
 
-- (BOOL)parseSearchPluginAtURL:(NSURL *)searchPluginURL;
+// If a parsing error occurs, returns NO and populates |outError| with an NSError object containing a
+// localized description of the problem. Pass NULL if you do not want error information.
+- (BOOL)parseSearchPluginAtURL:(NSURL *)searchPluginURL error:(NSError **)outError;
 
 // Accessors to obtain parsed information:
 - (NSString *)searchEngineName;
 - (NSString *)searchEngineURL;
+- (NSString *)searchEngineURLRequestMethod;
 
 @end
 
@@ -109,7 +124,9 @@ extern NSString *const kOpenSearchMIMEType;
 // Set parsed properties:
 - (void)setSearchEngineName:(NSString *)newSearchEngineName;
 - (void)setSearchEngineURL:(NSString *)newSearchEngineURL;
+- (void)setSearchEngineURLRequestMethod:(NSString *)newMethod;
 
-- (BOOL)browserSupportsSearchQueryURLWithMIMEType:(NSString *)mimeType requestMethod:(NSString *)method;
+- (BOOL)browserSupportsSearchQueryURLWithMIMEType:(NSString *)mimeType;
+- (BOOL)browserSupportsSearchQueryURLWithRequestMethod:(NSString *)requestMethod;
 
 @end

mozilla/camino/src/websearch/XMLSearchPluginParser.mm

@@ -38,19 +38,24 @@
 #import "XMLSearchPluginParser.h"
 
 #import "OpenSearchParser.h"
+#import "NSString+Utils.h"
 
 NSString *const kWebSearchPluginNameKey = @"SearchPluginName";
 NSString *const kWebSearchPluginMIMETypeKey = @"SearchPluginMIMEType";
 NSString *const kWebSearchPluginURLKey = @"SearchPluginURL";
 
 #define PLUGIN_DOWNLOAD_TIMEOUT_INTERVAL 3.0
+#define HTTP_SUCCESS_STATUS_CODE 200
 
 NSString *const kOpenSearchMIMEType = @"application/opensearchdescription+xml";
 
+NSString *const kXMLSearchPluginParserErrorDomain = @"XMLSearchPluginParserErrorDomain";
+
 @interface XMLSearchPluginParser (PrivateToSuperclass)
 
-- (BOOL)parseSearchPluginData:(NSData *)pluginData;
+- (BOOL)parseSearchPluginData:(NSData *)pluginData error:(NSError **)outError;
 - (BOOL)searchEngineInformationWasFound;
+- (NSError *)parsingErrorWithCode:(EXMLSearchPluginParserErrorCode)errorCode;
 
 @end
 
@@ -113,39 +118,103 @@ static NSDictionary const *sSubclassToPluginTypeMap = nil;
   [mElementsToParseAttributesFor release];
   [mSearchEngineName release];
   [mSearchEngineURL release];
+  [mSearchEngineURLRequestMethod release];
   [super dealloc];
 }
 
 #pragma mark -
 
-- (BOOL)parseSearchPluginAtURL:(NSURL *)searchPluginURL
+- (BOOL)parseSearchPluginAtURL:(NSURL *)searchPluginURL error:(NSError **)outError
 {
+  if (outError)
+    *outError = nil;
+
+  if (!searchPluginURL) {
+    if (outError)
+      *outError = [self parsingErrorWithCode:eXMLSearchPluginParserPluginNotFoundError];
+    return NO;
+  }
+
   // |...WithContentsOfURL| methods throughout the Foundation will fail whenever the 
   // requested web server offers to return a gzipped data stream.  To work around 
   // this issue, we have to use NSURLConnection instead (which will automatically
   // decompress gzipped data when necessary).
-  if (!searchPluginURL)
-    return NO;
-
   NSMutableURLRequest *urlRequest = [NSMutableURLRequest requestWithURL:searchPluginURL];
   [urlRequest setCachePolicy:NSURLRequestReloadIgnoringCacheData];
   [urlRequest setTimeoutInterval:PLUGIN_DOWNLOAD_TIMEOUT_INTERVAL];
-  NSData *xmlData = [NSURLConnection sendSynchronousRequest:urlRequest returningResponse:NULL error:NULL];
+  NSURLResponse *urlResponse;
+  NSData *xmlData = [NSURLConnection sendSynchronousRequest:urlRequest returningResponse:&urlResponse error:NULL];
 
-  BOOL parsedOK = [self parseSearchPluginData:xmlData];
-  return parsedOK;
+  // Check of the definition was found on the server.
+  // If it doesn't exist, +[NSURLConnection sendSync...] is allowed to redirect and return 
+  // the contents of the server's alternate (404) error page, instead of no data.
+  // So, we can't just rely on [data length] because there will often be information.
+  // Furthermore, the response object is not always an NSHTTPUrlResponse.  On errors though, it should
+  // be: "If the response is an NSHTTPURLResponse object, and the statusCode is 4xx or 5xx, then the server
+  // is attempting to redirect to an alternate error page. -ADC"
+  if ([xmlData length] == 0 ||
+      ([urlResponse respondsToSelector:@selector(statusCode)] &&
+      ([(NSHTTPURLResponse *)urlResponse statusCode] != HTTP_SUCCESS_STATUS_CODE)))
+  {
+    if (outError)
+      *outError = [self parsingErrorWithCode:eXMLSearchPluginParserPluginNotFoundError];
+    return NO;
+  }
+
+  return [self parseSearchPluginData:xmlData error:outError];
 }
 
-- (BOOL)parseSearchPluginData:(NSData *)pluginData
+- (BOOL)parseSearchPluginData:(NSData *)pluginData error:(NSError **)outError
 {
-  if (!pluginData)
-    return NO;
+  if (outError)
+    *outError = nil;
 
   NSXMLParser *xmlParser = [[NSXMLParser alloc] initWithData:pluginData];
   [xmlParser setDelegate:self];
   BOOL parsingFinishedWithoutErrors = [xmlParser parse];
   [xmlParser release];
-  return (parsingFinishedWithoutErrors && [self searchEngineInformationWasFound]);
+
+  // Check for parsing errors.
+  if (!parsingFinishedWithoutErrors || ![self searchEngineInformationWasFound]) {
+    if (outError)
+      *outError = [self parsingErrorWithCode:eXMLSearchPluginParserInvalidPluginFormatError];
+    return NO;
+  }
+
+  // Check if our search fields support the specified method request type.
+  if (![self browserSupportsSearchQueryURLWithRequestMethod:[self searchEngineURLRequestMethod]]) { 
+    if (outError)
+      *outError = [self parsingErrorWithCode:eXMLSearchPluginParserUnsupportedSearchURLError];
+    return NO;
+  }
+
+  // If we got this far, parsing was successful.
+  return YES;
+}
+
+- (NSError *)parsingErrorWithCode:(EXMLSearchPluginParserErrorCode)errorCode
+{
+  NSString *localizedErrorDesc = nil;
+  switch (errorCode) {
+    case eXMLSearchPluginParserInvalidPluginFormatError:
+      localizedErrorDesc = NSLocalizedString(@"XMLSearchPluginParserInvalidPluginFormat", nil);
+      break;
+    case eXMLSearchPluginParserPluginNotFoundError:
+      localizedErrorDesc = NSLocalizedString(@"XMLSearchPluginParserPluginNotFound", nil);
+      break;
+    case eXMLSearchPluginParserUnsupportedSearchURLError:
+      localizedErrorDesc = NSLocalizedString(@"XMLSearchPluginParserUnsupportedSearchURL", nil);
+      break;
+    default:
+      localizedErrorDesc = NSLocalizedString(@"XMLSearchPluginParserUnknownError", nil);
+  }
+
+  NSDictionary *errorUserInfo = [NSDictionary dictionaryWithObject:localizedErrorDesc
+                                                            forKey:NSLocalizedDescriptionKey];
+
+  return [NSError errorWithDomain:kXMLSearchPluginParserErrorDomain
+                             code:errorCode 
+                         userInfo:errorUserInfo];
 }
 
 #pragma mark -
@@ -225,7 +294,7 @@ didStartElement:(NSString *)elementName
 
 - (NSString *)searchEngineName
 {
-  return mSearchEngineName;
+  return [[mSearchEngineName retain] autorelease];
 }
 
 - (void)setSearchEngineName:(NSString *)newSearchEngineName
@@ -238,7 +307,7 @@ didStartElement:(NSString *)elementName
 
 - (NSString *)searchEngineURL
 {
-  return mSearchEngineURL;
+  return [[mSearchEngineURL retain] autorelease];
 }
 
 - (void)setSearchEngineURL:(NSString *)newSearchEngineURL
@@ -249,24 +318,38 @@ didStartElement:(NSString *)elementName
   }
 }
 
+- (NSString *)searchEngineURLRequestMethod
+{
+  return [[mSearchEngineURLRequestMethod retain] autorelease]; 
+}
+
+- (void)setSearchEngineURLRequestMethod:(NSString *)newSearchEngineURLRequestMethod
+{
+  if (mSearchEngineURLRequestMethod != newSearchEngineURLRequestMethod) {
+    [mSearchEngineURLRequestMethod release];
+    mSearchEngineURLRequestMethod = [newSearchEngineURLRequestMethod retain];
+  }
+}
+
 #pragma mark -
 
-- (BOOL)browserSupportsSearchQueryURLWithMIMEType:(NSString *)mimeType requestMethod:(NSString *)method
+- (BOOL)browserSupportsSearchQueryURLWithMIMEType:(NSString *)mimeType
 {
-  BOOL isSupported = NO;
-  if ([mimeType isEqualToString:@"text/html"] &&
-      [method caseInsensitiveCompare:@"GET"] == NSOrderedSame)
-  {
-      isSupported = YES;
-  }
-  return isSupported;
+  return ([mimeType isEqualToString:@"text/html"]);
+}
+
+- (BOOL)browserSupportsSearchQueryURLWithRequestMethod:(NSString *)requestMethod
+{
+  return [requestMethod isEqualToStringIgnoringCase:@"GET"];
 }
 
 // NSXMLParser finishing without errors is not enough to know whether we successfully obtained enough
 // information for a new search engine. This method ensures a value was set for each required property.
 - (BOOL)searchEngineInformationWasFound;
 {
-  return ([[self searchEngineName] length] > 0 && [[self searchEngineURL] length] > 0);
+  return ([[self searchEngineName] length] > 0 && 
+          [[self searchEngineURL] length] > 0 &&
+          [[self searchEngineURLRequestMethod] length] > 0);
 }
 
 @end