Parse out XSSI guards in Suggestion JSON response

chrome/browser/autocomplete/search_provider.cc

Index: chrome/browser/autocomplete/search_provider.cc
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index ee0a205b26cde6ff036799459825366e0a64c468..ea4328d76a484f58560ba07d4a55e5fb8c77f85b 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -600,24 +600,6 @@ void SearchProvider::OnURLFetchComplete(const net::URLFetcher* source) {
   suggest_results_pending_--;
   LogOmniboxSuggestRequest(REPLY_RECEIVED);
   DCHECK_GE(suggest_results_pending_, 0);  // Should never go negative.
-  const net::HttpResponseHeaders* const response_headers =
-      source->GetResponseHeaders();
-  std::string json_data;
-  source->GetResponseAsString(&json_data);
-  // JSON is supposed to be UTF-8, but some suggest service providers send JSON
-  // files in non-UTF-8 encodings.  The actual encoding is usually specified in
-  // the Content-Type header field.
-  if (response_headers) {
-    std::string charset;
-    if (response_headers->GetCharset(&charset)) {
-      string16 data_16;
-      // TODO(jungshik): Switch to CodePageToUTF8 after it's added.
-      if (base::CodepageToUTF16(json_data, charset.c_str(),
-                                base::OnStringConversionError::FAIL,
-                                &data_16))
-        json_data = UTF16ToUTF8(data_16);
-    }
-  }
 
   const bool is_keyword = (source == keyword_fetcher_.get());
   // Ensure the request succeeded and that the provider used is still available.
@@ -648,10 +630,46 @@ void SearchProvider::OnURLFetchComplete(const net::URLFetcher* source) {
 
   bool results_updated = false;
   if (request_succeeded) {
-    JSONStringValueSerializer deserializer(json_data);
-    deserializer.set_allow_trailing_comma(true);
-    scoped_ptr<Value> data(deserializer.Deserialize(NULL, NULL));
-    results_updated = data.get() && ParseSuggestResults(data.get(), is_keyword);
+    const net::HttpResponseHeaders* const response_headers =
+        source->GetResponseHeaders();
+    std::string json_data;
+    source->GetResponseAsString(&json_data);
+    // JSON is supposed to be UTF-8, but some suggest service providers send
+    // JSON files in non-UTF-8 encodings.  The actual encoding is usually
+    // specified in the Content-Type header field.
+    if (response_headers) {
+      std::string charset;
+      if (response_headers->GetCharset(&charset)) {
+        string16 data_16;
+        // TODO(jungshik): Switch to CodePageToUTF8 after it's added.
+        if (base::CodepageToUTF16(json_data, charset.c_str(),
+                                  base::OnStringConversionError::FAIL,
+                                  &data_16))
+          json_data = UTF16ToUTF8(data_16);
+      }
+    }
+
+    int error_code = 0;
+    while (true) {

[Peter Kasting, 2013/10/29 18:45:49]

This can infinite loop.

If we find the first '[', trim everything before that, then fail to
Deserialize(), we will loop, and the find() call will return 0 since the same
'[' we found the previous time is still present.

Why does there need to be a loop around this at all?  Why can't we just do all
this once?

[Maria, 2013/10/29 18:55:45]

I don't think it's likely that XSSI guard would change either. You could
actually look specifically for the guard characters at the start of the string
instead of just any opening square bracket to ensure that we are handling the
case we are expecting instead of possibly an error message with brackets in it.

[Anuj, 2013/10/29 18:58:05]

Please see the test cases - Hypothetically XSSI guard can itself contain "[".
I think I ran the test after uploading, and didn't upload the fix.

[Anuj, 2013/10/29 19:03:35]

XSSI guard may not change. But looking for response seems a better strategy to
me than stripping out specific XSSI guard.

And in any case, stripping XSSI guard doesn't help against the error message
scenario either.

+      // The JSON response should be an array.
+      size_t response_start_index = json_data.find("[");
+
+      if (response_start_index == std::string::npos)
+        break;
+
+      // Remove any XSSI guards to allow for JSON parsing.
+      if (response_start_index > 0)
+        json_data.erase(0, response_start_index - 1);
+
+      JSONStringValueSerializer deserializer(json_data);
+      deserializer.set_allow_trailing_comma(true);
+      scoped_ptr<Value> data(deserializer.Deserialize(&error_code, NULL));
+      if (error_code == 0) {
+        results_updated = data.get() &&
+            ParseSuggestResults(data.get(), is_keyword);
+        break;
+      }
+    }
   }
 
   UpdateMatches();