From 69791d09fe80b519ed518dafe68a1b9c5851a84c Mon Sep 17 00:00:00 2001
From: Daniel Holbert <dholbert@cs.stanford.edu>
Date: Thu, 29 Aug 2013 11:39:56 -0400
Subject: [PATCH] Bug 904158 - When creating a string for the SMIL-animated
 value of a mapped SVG attribute, use NS_strlen to get the StringBuffer's
 logical length, instead of using nsCheapString and the allocated length.
 r=dbaron

---
 content/base/src/nsAttrValue.h                |  7 ++++
 content/svg/content/src/nsSVGElement.cpp      | 33 +++++++++++++++----
 .../svg/smil/mapped-attr-long-url-1.svg       | 13 ++++++++
 layout/reftests/svg/smil/reftest.list         |  2 ++
 4 files changed, 48 insertions(+), 7 deletions(-)
 create mode 100644 layout/reftests/svg/smil/mapped-attr-long-url-1.svg

diff --git a/content/base/src/nsAttrValue.h b/content/base/src/nsAttrValue.h
index 3f84c77a439..47813972ae3 100644
--- a/content/base/src/nsAttrValue.h
+++ b/content/base/src/nsAttrValue.h
@@ -57,6 +57,13 @@ struct ImageValue;
 /**
  * A class used to construct a nsString from a nsStringBuffer (we might
  * want to move this to nsString at some point).
+ *
+ * WARNING: Note that nsCheapString doesn't take an explicit length -- it
+ * assumes the string is maximally large, given the nsStringBuffer's storage
+ * size.  This means the given string buffer *must* be sized exactly correctly
+ * for the string it contains (including one byte for a null terminator).  If
+ * it has any unused storage space, then that will result in bogus characters
+ * at the end of our nsCheapString.
  */
 class nsCheapString : public nsString {
 public:
diff --git a/content/svg/content/src/nsSVGElement.cpp b/content/svg/content/src/nsSVGElement.cpp
index b6a836ebfb8..1eed17aa111 100644
--- a/content/svg/content/src/nsSVGElement.cpp
+++ b/content/svg/content/src/nsSVGElement.cpp
@@ -1302,15 +1302,34 @@ ParseMappedAttrAnimValueCallback(void*    aObject,
                                  void*    aPropertyValue,
                                  void*    aData)
 {
-  NS_ABORT_IF_FALSE(aPropertyName != SMIL_MAPPED_ATTR_STYLERULE_ATOM,
-                    "animated content style rule should have been removed "
-                    "from properties table already (we're rebuilding it now)");
+  MOZ_ASSERT(aPropertyName != SMIL_MAPPED_ATTR_STYLERULE_ATOM,
+             "animated content style rule should have been removed "
+             "from properties table already (we're rebuilding it now)");
 
-  MappedAttrParser* mappedAttrParser =
-    static_cast<MappedAttrParser*>(aData);
+  MappedAttrParser* mappedAttrParser = static_cast<MappedAttrParser*>(aData);
+  MOZ_ASSERT(mappedAttrParser, "parser should be non-null");
 
-  nsStringBuffer* valueBuf = static_cast<nsStringBuffer*>(aPropertyValue);
-  mappedAttrParser->ParseMappedAttrValue(aPropertyName, nsCheapString(valueBuf));
+  nsStringBuffer* animValBuf = static_cast<nsStringBuffer*>(aPropertyValue);
+  MOZ_ASSERT(animValBuf, "animated value should be non-null");
+
+  PRUnichar* animValBufData = static_cast<PRUnichar*>(animValBuf->Data());
+  uint32_t logicalStringLen = NS_strlen(animValBufData);
+  // SANITY CHECK: In case the string buffer wasn't correctly
+  // null-terminated, let's check the allocated size, too, and make sure we
+  // don't read further than that. (Note that StorageSize() is in units of
+  // bytes, so we have to convert that to units of PRUnichars, and subtract
+  // 1 for the null-terminator.)
+  uint32_t allocStringLen =
+    (animValBuf->StorageSize() / sizeof(PRUnichar)) - 1;
+
+  MOZ_ASSERT(logicalStringLen <= allocStringLen,
+             "The string in our string buffer wasn't null-terminated!!");
+
+  nsString animValStr;
+  animValBuf->ToString(std::min(logicalStringLen, allocStringLen),
+                       animValStr);
+
+  mappedAttrParser->ParseMappedAttrValue(aPropertyName, animValStr);
 }
 
 // Callback for freeing animated content style rule, in property table.
diff --git a/layout/reftests/svg/smil/mapped-attr-long-url-1.svg b/layout/reftests/svg/smil/mapped-attr-long-url-1.svg
new file mode 100644
index 00000000000..7c4c3b66cbf
--- /dev/null
+++ b/layout/reftests/svg/smil/mapped-attr-long-url-1.svg
@@ -0,0 +1,13 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink">
+  <rect height="100%" width="100%" fill="lime" />
+  <rect height="100" width="100" fill="red">
+    <set attributeName="fill" attributeType="XML" dur="indefinite"
+         to="url(#reaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaallyLongURL) transparent"/>
+  </rect>
+</svg>
diff --git a/layout/reftests/svg/smil/reftest.list b/layout/reftests/svg/smil/reftest.list
index 6cb19150e5c..c7800dfff33 100644
--- a/layout/reftests/svg/smil/reftest.list
+++ b/layout/reftests/svg/smil/reftest.list
@@ -245,6 +245,8 @@ fuzzy-if(cocoaWidget&&layersGPUAccelerated,1,2) == anim-gradient-attr-presence-0
 == inactivate-with-active-unchanged-1.svg anim-standard-ref.svg
 == inactivate-with-active-unchanged-2.svg anim-standard-ref.svg
 
+== mapped-attr-long-url-1.svg lime.svg
+
 # interaction between xml mapped attributes and their css equivalents
 == mapped-attr-vs-css-prop-1.svg lime.svg