Use "GURL::possibly_invalid_spec()" rather than "GURL::spec()", in case the GURL that was passed to URLRequest(const GURL&) is invalid.

BUG=http://crbug.com/21423

Review URL: http://codereview.chromium.org/192057

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@25903 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 29 insertions, 7 deletions

diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index bf24c2d..9076a75 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -101,11 +101,10 @@ URLRequest::InstanceTracker::ExtractInfo(URLRequest* url_request) {
   info.original_url = url_request->original_url();
   info.load_log = url_request->load_log();
 
-  // Paranoia check: truncate really big URLs.
-  if (info.original_url.spec().size() > kMaxGraveyardURLSize) {
-    info.original_url = GURL(url_request->original_url().spec().substr(
-        0, kMaxGraveyardURLSize));
-  }
+  // Paranoia check: truncate |info.original_url| if it is really big.
+  const std::string& spec = info.original_url.possibly_invalid_spec();
+  if (spec.size() > kMaxGraveyardURLSize)
+    info.original_url = GURL(spec.substr(0, kMaxGraveyardURLSize));
   return info;
 }
 
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index d18ffeb..2f2f2e1 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -269,7 +269,8 @@ TEST_F(URLRequestTest, Tracking) {
 TEST_F(URLRequestTest, TrackingGraveyardBounded) {
   URLRequest::InstanceTracker::Get()->ClearRecentlyDeceased();
   EXPECT_EQ(0u, URLRequest::InstanceTracker::Get()->GetLiveRequests().size());
-  EXPECT_EQ(0u, URLRequest::InstanceTracker::Get()->GetLiveRequests().size());
+  EXPECT_EQ(0u,
+            URLRequest::InstanceTracker::Get()->GetRecentlyDeceased().size());
 
   const size_t kMaxGraveyardSize =
       URLRequest::InstanceTracker::kMaxGraveyardSize;
@@ -312,6 +313,28 @@ TEST_F(URLRequestTest, TrackingGraveyardBounded) {
                 .original_url.spec().size());
 }
 
+// Test the instance tracking functionality of URLRequest does not
+// fail if the URL was invalid. http://crbug.com/21423.
+TEST_F(URLRequestTest, TrackingInvalidURL) {
+  URLRequest::InstanceTracker::Get()->ClearRecentlyDeceased();
+  EXPECT_EQ(0u, URLRequest::InstanceTracker::Get()->GetLiveRequests().size());
+  EXPECT_EQ(0u,
+            URLRequest::InstanceTracker::Get()->GetRecentlyDeceased().size());
+
+  {
+    GURL invalid_url("xabc");
+    EXPECT_FALSE(invalid_url.is_valid());
+    URLRequest req(invalid_url, NULL);
+  }
+
+  // Check that the invalid URL made it into graveyard.
+  URLRequest::InstanceTracker::RecentRequestInfoList recent_reqs =
+      URLRequest::InstanceTracker::Get()->GetRecentlyDeceased();
+
+  ASSERT_EQ(1u, recent_reqs.size());
+  EXPECT_FALSE(recent_reqs[0].original_url.is_valid());
+}
+
 TEST_F(URLRequestTest, QuitTest) {
   // Don't use shared server here because we order it to quit.
   // It would impact other tests.
diff --git a/net/url_request/url_request_view_net_internal_job.cc b/net/url_request/url_request_view_net_internal_job.cc
index 51fd0aa..1095ae4 100644
--- a/net/url_request/url_request_view_net_internal_job.cc
+++ b/net/url_request/url_request_view_net_internal_job.cc
@@ -275,7 +275,7 @@ void OutputURLAndLoadLog(const GURL& url,
                          std::string* out) {
   out->append("<li>");
   out->append("<nobr>");
-  out->append(EscapeForHTML(url.spec()));
+  out->append(EscapeForHTML(url.possibly_invalid_spec()));
   out->append("</nobr>");
   if (log)
     OutputTextInPre(net::LoadLogUtil::PrettyPrintAsEventTree(log), out);