Title: [109820] trunk
Revision
109820
Author
schen...@chromium.org
Date
2012-03-05 17:00:17 -0800 (Mon, 05 Mar 2012)

Log Message

[Chromium] SVG Composite of Offset crashes
https://bugs.webkit.org/show_bug.cgi?id=77245

Reviewed by Stephen White.

The feComposite arithmetic mode filter could readily be made to
generate invalid pre-multiplied pixel values which would then go on to
pollute other filters and cause invalid final output pixels. This
patch checks for filters that require valid inputs, and checks that a
result is valid, and corrects the result if necessary. This matches
the behavior of FF and Opera while preventing crashes or other
undesirable behavior.

Source/WebCore:

Test: svg/filters/feComposite-arithmetic-invalid-rgba.svg

* platform/graphics/filters/FEComposite.h: Override the default validity checks and image cleanup methods.
* platform/graphics/filters/FEComposite.cpp:
(WebCore::FEComposite::correctFilterResultIfNeeded): Force valid pixels if this is an arithmetic filter
* platform/graphics/filters/FilterEffect.cpp:
(WebCore::FilterEffect::apply): Check for validity status and correct
(WebCore::FilterEffect::forceValidPremultipliedPixels): Make an image valid
(WebCore):
* platform/graphics/filters/FilterEffect.h: New virtual methods for image validity.
(FilterEffect):
(WebCore::FilterEffect::requiresValidPreMulultipliedPixels):
(WebCore::FilterEffect::forceValidPremultipliedPixels):
(WebCore::FilterEffect::correctFilterResultIfNeeded):
* rendering/svg/RenderSVGResourceFilter.cpp:
(WebCore::RenderSVGResourceFilter::postApplyResource): Check that the final filter result is valid

LayoutTests:

* svg/filters/feComposite-arithmetic-invalid-rgba-expected.svg: Added.
* svg/filters/feComposite-arithmetic-invalid-rgba.svg: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (109819 => 109820)


--- trunk/LayoutTests/ChangeLog	2012-03-06 00:54:16 UTC (rev 109819)
+++ trunk/LayoutTests/ChangeLog	2012-03-06 01:00:17 UTC (rev 109820)
@@ -1,3 +1,21 @@
+2012-03-05  Stephen Chenney  <schen...@chromium.org>
+
+        [Chromium] SVG Composite of Offset crashes
+        https://bugs.webkit.org/show_bug.cgi?id=77245
+
+        Reviewed by Stephen White.
+
+        The feComposite arithmetic mode filter could readily be made to
+        generate invalid pre-multiplied pixel values which would then go on to
+        pollute other filters and cause invalid final output pixels. This
+        patch checks for filters that require valid inputs, and checks that a
+        result is valid, and corrects the result if necessary. This matches
+        the behavior of FF and Opera while preventing crashes or other
+        undesirable behavior.
+
+        * svg/filters/feComposite-arithmetic-invalid-rgba-expected.svg: Added.
+        * svg/filters/feComposite-arithmetic-invalid-rgba.svg: Added.
+
 2012-03-05  Alexis Menard  <alexis.men...@openbossa.org>
 
         getComputedStyle gives incorrect information for 'height' property

Added: trunk/LayoutTests/svg/filters/feComposite-arithmetic-invalid-rgba-expected.svg (0 => 109820)


--- trunk/LayoutTests/svg/filters/feComposite-arithmetic-invalid-rgba-expected.svg	                        (rev 0)
+++ trunk/LayoutTests/svg/filters/feComposite-arithmetic-invalid-rgba-expected.svg	2012-03-06 01:00:17 UTC (rev 109820)
@@ -0,0 +1,18 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="200" height="100">
+    <!-- Due to Bug 80321, we need to use a filter in this expected result reference. -->
+    <defs>
+        <!-- This filter produces the same as the input, but does color conversions along the way. -->
+        <filter id="identity">
+            <feComposite operator="arithmetic" in="SourceGraphic" in2="SourceGraphic" k1="0" k2="1.0" k3="0" k4="0" />
+        </filter>
+    </defs>
+
+    <!-- Background for color comparison. The border of the final rectangle should be the same as the interior color. -->
+    <rect x="20" y="20" width="60" height="60" fill="rgba(0,255,0,1)" />
+
+    <!-- The content of interest -->
+    <rect filter="url(#identity)" x="25" y="25" width="50" height="50" fill="rgba(0,255,0,1)" stroke="none" />
+
+    <!-- Border to show expected non-drawing area. -->
+    <rect x="125" y="25" width="50" height="50" fill="none" stroke="rgb(0,255,0)" />
+</svg>

Added: trunk/LayoutTests/svg/filters/feComposite-arithmetic-invalid-rgba.svg (0 => 109820)


--- trunk/LayoutTests/svg/filters/feComposite-arithmetic-invalid-rgba.svg	                        (rev 0)
+++ trunk/LayoutTests/svg/filters/feComposite-arithmetic-invalid-rgba.svg	2012-03-06 01:00:17 UTC (rev 109820)
@@ -0,0 +1,33 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="200" height="100">
+    <defs>
+        <!-- This filter produces results that are invalid pre-multiplied rgba pixels. Specifically, after the 4th step an -->
+        <!-- interior pixel will contain approximately (0, 0.8, 0, 0.6) which is invalid because g > a. When used in     -->
+        <!-- other operations this may generate bad results but we only want to clamp the values when passed on to other   -->
+        <!-- operations, not for intermediate arithmetic results. -->
+        <filter id="arithmetic">
+            <feComposite operator="arithmetic" in="SourceGraphic" in2="SourceGraphic" k1="0" k2="0.2" k3="0" k4="0" result="rgba02" />
+            <feComposite operator="arithmetic" in="SourceAlpha" in2="SourceAlpha" k1="0" k2="0.2" k3="0" k4="0" result="alpha05" />
+            <feComposite operator="arithmetic" in="rgba02" in2="alpha05" k1="0" k2="1" k3="1" k4="0" result="tmp" />
+            <feComposite operator="arithmetic" in="SourceGraphic" in2="tmp" k1="0" k2="1" k3="-1" k4="0" />
+            <feComposite operator="arithmetic" in="tmp" k1="0" k2="1" k3="1" k4="0" />
+        </filter>
+
+        <!-- This filter will produce images with zero alpha but non-zero color components. -->
+        <filter id="zero-alpha">
+            <feComposite operator="arithmetic" in="SourceAlpha" in2="SourceAlpha" k1="0" k2="1.0" k3="0" k4="0" result="alpha" />
+            <feComposite operator="arithmetic" in="SourceGraphic" in2="alpha" k1="0" k2="1.0" k3="-1.0" k4="0" />
+        </filter>
+    </defs>
+
+    <!-- Background for color comparison. The border of the final rectangle should be the same as the interior color. -->
+    <rect x="20" y="20" width="60" height="60" fill="rgba(0,255,0,1)" />
+
+    <!-- The content of interest -->
+    <rect filter="url(#arithmetic)" x="25" y="25" width="50" height="50" fill="rgba(0,255,0,1)" stroke="none" />
+
+    <!-- Border to show expected non-drawing area. -->
+    <rect x="125" y="25" width="50" height="50" fill="none" stroke="rgb(0,255,0)" />
+
+    <!-- This should produce nothing, and not cause a crash. -->
+    <rect filter="url(#zero-alpha)" x="125" y="25" width="50" height="50" fill="rgba(0,255,0,1)" stroke="none" />
+</svg>

Modified: trunk/Source/WebCore/ChangeLog (109819 => 109820)


--- trunk/Source/WebCore/ChangeLog	2012-03-06 00:54:16 UTC (rev 109819)
+++ trunk/Source/WebCore/ChangeLog	2012-03-06 01:00:17 UTC (rev 109820)
@@ -1,3 +1,35 @@
+2012-03-05  Stephen Chenney  <schen...@chromium.org>
+
+        [Chromium] SVG Composite of Offset crashes
+        https://bugs.webkit.org/show_bug.cgi?id=77245
+
+        Reviewed by Stephen White.
+
+        The feComposite arithmetic mode filter could readily be made to
+        generate invalid pre-multiplied pixel values which would then go on to
+        pollute other filters and cause invalid final output pixels. This
+        patch checks for filters that require valid inputs, and checks that a
+        result is valid, and corrects the result if necessary. This matches
+        the behavior of FF and Opera while preventing crashes or other
+        undesirable behavior.
+
+        Test: svg/filters/feComposite-arithmetic-invalid-rgba.svg
+
+        * platform/graphics/filters/FEComposite.h: Override the default validity checks and image cleanup methods.
+        * platform/graphics/filters/FEComposite.cpp:
+        (WebCore::FEComposite::correctFilterResultIfNeeded): Force valid pixels if this is an arithmetic filter
+        * platform/graphics/filters/FilterEffect.cpp:
+        (WebCore::FilterEffect::apply): Check for validity status and correct
+        (WebCore::FilterEffect::forceValidPremultipliedPixels): Make an image valid
+        (WebCore):
+        * platform/graphics/filters/FilterEffect.h: New virtual methods for image validity.
+        (FilterEffect):
+        (WebCore::FilterEffect::requiresValidPreMulultipliedPixels):
+        (WebCore::FilterEffect::forceValidPremultipliedPixels):
+        (WebCore::FilterEffect::correctFilterResultIfNeeded):
+        * rendering/svg/RenderSVGResourceFilter.cpp:
+        (WebCore::RenderSVGResourceFilter::postApplyResource): Check that the final filter result is valid
+
 2012-03-05  Alexis Menard  <alexis.men...@openbossa.org>
 
         getComputedStyle gives incorrect information for 'height' property

Modified: trunk/Source/WebCore/platform/graphics/filters/FEComposite.cpp (109819 => 109820)


--- trunk/Source/WebCore/platform/graphics/filters/FEComposite.cpp	2012-03-06 00:54:16 UTC (rev 109819)
+++ trunk/Source/WebCore/platform/graphics/filters/FEComposite.cpp	2012-03-06 01:00:17 UTC (rev 109820)
@@ -116,6 +116,14 @@
     return true;
 }
 
+void FEComposite::correctFilterResultIfNeeded()
+{
+    if (m_type != FECOMPOSITE_OPERATOR_ARITHMETIC)
+        return;
+
+    forceValidPreMultipliedPixels();
+}
+
 template <int b1, int b2, int b3, int b4>
 static inline void computeArithmeticPixels(unsigned char* source, unsigned char* destination, int pixelArrayLength,
                                     float k1, float k2, float k3, float k4)

Modified: trunk/Source/WebCore/platform/graphics/filters/FEComposite.h (109819 => 109820)


--- trunk/Source/WebCore/platform/graphics/filters/FEComposite.h	2012-03-06 00:54:16 UTC (rev 109819)
+++ trunk/Source/WebCore/platform/graphics/filters/FEComposite.h	2012-03-06 01:00:17 UTC (rev 109820)
@@ -59,6 +59,8 @@
     float k4() const;
     bool setK4(float);
 
+    virtual void correctFilterResultIfNeeded() OVERRIDE;
+
     virtual void platformApplySoftware();
     virtual void dump();
     
@@ -66,6 +68,9 @@
 
     virtual TextStream& externalRepresentation(TextStream&, int indention) const;
 
+protected:
+    virtual bool requiresValidPreMultipliedPixels() OVERRIDE { return m_type != FECOMPOSITE_OPERATOR_ARITHMETIC; }
+
 private:
     FEComposite(Filter*, const CompositeOperationType&, float, float, float, float);
 

Modified: trunk/Source/WebCore/platform/graphics/filters/FilterEffect.cpp (109819 => 109820)


--- trunk/Source/WebCore/platform/graphics/filters/FilterEffect.cpp	2012-03-06 00:54:16 UTC (rev 109819)
+++ trunk/Source/WebCore/platform/graphics/filters/FilterEffect.cpp	2012-03-06 01:00:17 UTC (rev 109820)
@@ -102,6 +102,11 @@
             return;
     }
     determineAbsolutePaintRect();
+
+    if (requiresValidPreMultipliedPixels()) {
+        for (unsigned i = 0; i < size; ++i)
+            inputEffect(i)->correctFilterResultIfNeeded();
+    }
     
     // Add platform specific apply functions here and return earlier.
 #if USE(SKIA)
@@ -111,6 +116,35 @@
     platformApplySoftware();
 }
 
+void FilterEffect::forceValidPreMultipliedPixels()
+{
+    // Must operate on pre-multiplied results; other formats cannot have invalid pixels.
+    if (!m_premultipliedImageResult)
+        return;
+
+    ByteArray* imageArray = m_premultipliedImageResult.get();
+    unsigned char* pixelData = imageArray->data();
+    int pixelArrayLength = imageArray->length();
+
+    // We must have four bytes per pixel, and complete pixels
+    ASSERT(!(pixelArrayLength % 4));
+    int numPixels = pixelArrayLength / 4;
+
+    // Iterate over each pixel, checking alpha and adjusting color components if necessary
+    while (--numPixels >= 0) {
+        // Alpha is the 4th byte in a pixel
+        unsigned char a = *(pixelData + 3);
+        // Clamp each component to alpha, and increment the pixel location
+        for (int i = 0; i < 3; ++i) {
+            if (*pixelData > a)
+                *pixelData = a;
+            ++pixelData;
+        }
+        // Increment for alpha
+        ++pixelData;
+    }
+}
+
 void FilterEffect::clearResult()
 {
     if (m_imageBufferResult)

Modified: trunk/Source/WebCore/platform/graphics/filters/FilterEffect.h (109819 => 109820)


--- trunk/Source/WebCore/platform/graphics/filters/FilterEffect.h	2012-03-06 00:54:16 UTC (rev 109819)
+++ trunk/Source/WebCore/platform/graphics/filters/FilterEffect.h	2012-03-06 01:00:17 UTC (rev 109820)
@@ -86,6 +86,11 @@
 
     void apply();
     
+    // Correct any invalid pixels, if necessary, in the result of a filter operation.
+    // This method is used to ensure valid pixel values on filter inputs and the final result.
+    // Only the arithmetic composite filter ever needs to perform correction.
+    virtual void correctFilterResultIfNeeded() { }
+
     virtual void platformApplySoftware() = 0;
 #if USE(SKIA)
     virtual bool platformApplySkia() { return false; }
@@ -131,6 +136,13 @@
     ByteArray* createUnmultipliedImageResult();
     ByteArray* createPremultipliedImageResult();
 
+    // Return true if the filter will only operate correctly on valid RGBA values, with
+    // alpha in [0,255] and each color component in [0, alpha].
+    virtual bool requiresValidPreMultipliedPixels() { return true; }
+
+    // If a pre-multiplied image, check every pixel for validity and correct if necessary.
+    void forceValidPreMultipliedPixels();
+
 private:
     OwnPtr<ImageBuffer> m_imageBufferResult;
     RefPtr<ByteArray> m_unmultipliedImageResult;

Modified: trunk/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp (109819 => 109820)


--- trunk/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp	2012-03-06 00:54:16 UTC (rev 109819)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp	2012-03-06 01:00:17 UTC (rev 109820)
@@ -300,6 +300,7 @@
         // Always true if filterData is just built (filterData->builded is false).
         if (!lastEffect->hasResult()) {
             lastEffect->apply();
+            lastEffect->correctFilterResultIfNeeded();
 #if !USE(CG)
             ImageBuffer* resultImage = lastEffect->asImageBuffer();
             if (resultImage)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to