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)
@@ -50,6 +50,22 @@
 
 void ParamTraits<GURL>::Write(Message* m, const GURL& p) {
   DCHECK(p.possibly_invalid_spec().length() <= content::kMaxURLChars);
+
+  // Beware of print-parse inconsistency which would change an invalid
+  // URL into a valid one. Ideally, the message would contain this flag
+  // so that the read side could make the check, but performing it here
+  // avoids changing the on-the-wire representation of such a fundamental
+  // type as GURL.

[brettw, 2012/12/17 20:25:00]

Can you file a bug for doing said improvements and link to it here?

[Tom Sepez, 2012/12/17 20:53:38]

Done.

+  if (!p.is_valid()) {
+    GURL reconstructed_url(p.possibly_invalid_spec());
+    if (reconstructed_url.is_valid()) {
+      DLOG(WARNING) << "GURL string " << p.possibly_invalid_spec()
+                    << " (marked invalid) but parsed as valid.";
+      m->WriteString("");

[brettw, 2012/12/17 20:25:00]

Can you do WriteString(std::string()) instead?

[Tom Sepez, 2012/12/17 20:53:38]

Done.

+      return;
+    }
+  }
+
   m->WriteString(p.possibly_invalid_spec());
   // TODO(brettw) bug 684583: Add encoding for query params.
 }