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
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 29 Aug 2013 11:39:56 -0400
changeset 144992 065aa28e30d3f58b36efd63909145b3f9c907cf8
parent 144991 73c39de65fe1b2be5a408f320c06acfba7038b56
child 144993 630b25ba7f69c8c83a1ee4b9dffa9115601dff4b
push id25186
push userryanvm@gmail.com
push dateThu, 29 Aug 2013 20:40:43 +0000
treeherdermozilla-central@8c5a94ba1096 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs904158
milestone26.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
content/svg/content/src/nsSVGElement.cpp
layout/reftests/svg/smil/mapped-attr-long-url-1.svg
layout/reftests/svg/smil/reftest.list
--- a/content/base/src/nsAttrValue.h
+++ b/content/base/src/nsAttrValue.h
@@ -52,16 +52,23 @@ struct ImageValue;
 #define NS_ATTRVALUE_ENUMTABLE_VALUE_NEEDS_TO_UPPER (1 << (NS_ATTRVALUE_ENUMTABLEINDEX_BITS - 1))
 #define NS_ATTRVALUE_ENUMTABLEINDEX_MAXVALUE (NS_ATTRVALUE_ENUMTABLE_VALUE_NEEDS_TO_UPPER - 1)
 #define NS_ATTRVALUE_ENUMTABLEINDEX_MASK \
   (uintptr_t((((1 << NS_ATTRVALUE_ENUMTABLEINDEX_BITS) - 1) &~ NS_ATTRVALUE_ENUMTABLE_VALUE_NEEDS_TO_UPPER)))
 
 /**
  * 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:
   nsCheapString(nsStringBuffer* aBuf)
   {
     if (aBuf)
       aBuf->ToString(aBuf->StorageSize()/sizeof(PRUnichar) - 1, *this);
   }
--- a/content/svg/content/src/nsSVGElement.cpp
+++ b/content/svg/content/src/nsSVGElement.cpp
@@ -1297,25 +1297,44 @@ nsSVGElement::UpdateContentStyleRule()
 }
 
 static void
 ParseMappedAttrAnimValueCallback(void*    aObject,
                                  nsIAtom* aPropertyName,
                                  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)");
-
-  MappedAttrParser* mappedAttrParser =
-    static_cast<MappedAttrParser*>(aData);
-
-  nsStringBuffer* valueBuf = static_cast<nsStringBuffer*>(aPropertyValue);
-  mappedAttrParser->ParseMappedAttrValue(aPropertyName, nsCheapString(valueBuf));
+  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);
+  MOZ_ASSERT(mappedAttrParser, "parser should be non-null");
+
+  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.
 static void
 ReleaseStyleRule(void*    aObject,       /* unused */
                  nsIAtom* aPropertyName,
                  void*    aPropertyValue,
                  void*    aData          /* unused */)
new file mode 100644
--- /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>
--- a/layout/reftests/svg/smil/reftest.list
+++ b/layout/reftests/svg/smil/reftest.list
@@ -240,16 +240,18 @@ fuzzy-if(cocoaWidget&&layersGPUAccelerat
 == freeze-applied-late-2.svg anim-standard-ref.svg
 == freeze-applied-late-3.svg anim-standard-ref.svg
 == freeze-applied-late-4.svg anim-standard-ref.svg
 == frozen-to-anim-1.svg lime.svg
 
 == 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
 
 == smil-transitions-interaction-1a.svg lime.svg
 == smil-transitions-interaction-1b.svg lime.svg
 == smil-transitions-interaction-2a.svg lime.svg
 skip-if(B2G) == smil-transitions-interaction-2b.svg lime.svg
 == smil-transitions-interaction-3a.svg lime.svg