Beware of print-read inconsistency when serializing GURLs.

content/public/common/common_param_traits.cc

Index: content/public/common/common_param_traits.cc
===================================================================
--- content/public/common/common_param_traits.cc	(revision 172151)
+++ content/public/common/common_param_traits.cc	(working copy)
@@ -51,6 +51,7 @@
 void ParamTraits<GURL>::Write(Message* m, const GURL& p) {
   DCHECK(p.possibly_invalid_spec().length() <= content::kMaxURLChars);
   m->WriteString(p.possibly_invalid_spec());
+  m->WriteBool(p.is_valid());
   // TODO(brettw) bug 684583: Add encoding for query params.
 }
 
@@ -60,7 +61,20 @@
     *p = GURL();
     return false;
   }
+
   *p = GURL(s);
+
+  // Backwards compatibility with automation bots: permit older messages
+  // that don't contain the is_valid flag without returing an error.
+  bool is_valid;
+  if (m->ReadBool(iter, &is_valid)) {
+    // GURL doesn't always have print-parse consistency; beware of an invalid
+    // URL magically become valid.
+    if (p->is_valid() && !is_valid) {
+      DLOG(WARNING) << "GURL string " << s << " (marked invalid) parsed valid.";
+      *p = GURL();

[brettw, 2012/12/17 18:53:23]

If we're going to do this, why not just have Write() write an empty string if
the URL is invalid and skip the flag?

We're going to do another pass where the serialization is done "properly",
right? Getting an empty GURL out the other makes me a bit worried about causing
regressions.

+    }
+  }
   return true;
 }