Limit the maximum buffer size of SkRawBufferedStream

src/codec/SkRawCodec.cpp

Index: src/codec/SkRawCodec.cpp
diff --git a/src/codec/SkRawCodec.cpp b/src/codec/SkRawCodec.cpp
index 524a438d2905023d60de3183df0ac6358aa73f2e..337aa79785e37b4ca649ee6464d167a6c44b2729 100644
--- a/src/codec/SkRawCodec.cpp
+++ b/src/codec/SkRawCodec.cpp
@@ -199,6 +199,24 @@ public:
    virtual SkMemoryStream* transferBuffer(size_t offset, size_t size) = 0;
 };
 
+class SkSizeLimitedDynamicMemoryWStream : public SkDynamicMemoryWStream {
+public:
+    virtual ~SkSizeLimitedDynamicMemoryWStream() {}
+
+    bool write(const void* buffer, size_t size) override {
+        size_t newSize;
+        if (!safe_add_to_size_t(this->bytesWritten(), size, &newSize) ||

[scroggo, 2016/03/09 16:32:45]

If we limit to 10,240,000, do we still need to check safe_add_to_size_t? That
fits in a 32 bit integer, and I assume size_t is at least that size.

I guess we *could* revisit that number, at which point we will need
safe_add_to_size_t. If you remove the call, add a comment why it's not
necessary.

[yujieqin, 2016/03/09 17:40:30]

I still prefer to check here, because there is not limit on the argument "size",
which can surely lead to overflow as soon as the bytesWritten() is larger than
0.

+            newSize > kMaxStreamSize) {

[scroggo, 2016/03/09 16:32:45]

nit: this lines up with the line below. Would be better either if it's indented
an extra four spaces or if the brace is on the next line by itself. (Unless we
remove safe_add_to_size_t, in which case, this fits on one line.

[yujieqin, 2016/03/09 17:40:30]

Done.

+            return false;

[scroggo, 2016/03/09 16:32:45]

Should we print some kind of error message? It seems like this *could* be a
valid DNG, but we've set a cutoff.

[yujieqin, 2016/03/09 17:40:30]

added a SkCodecPrintf

+        }
+        return SkDynamicMemoryWStream::write(buffer, size);

[scroggo, 2016/03/09 16:32:45]

typically in Skia, we use a typedef INHERITED to refer to the base class. So
this should have a typedef in private, and this line should look like:

return this->INHERITED::write(buffer, size);

[yujieqin, 2016/03/09 17:40:30]

Done.

+    }
+
+private:
+    const size_t kMaxStreamSize = 100 * 1024 * 1024;  // 100MB
+};
+
+// Note: the maximum buffer size is 100MB (limited by SkSizeLimitedDynamicMemoryWStream).
 class SkRawBufferedStream : public SkRawStream {
 public:
     // Will take the ownership of the stream.
@@ -301,7 +319,8 @@ private:
     SkAutoTDelete<SkStream> fStream;
     bool fWholeStreamRead;
 
-    SkDynamicMemoryWStream fStreamBuffer;
+    // Use a size-limited stream to avoid holding too huge buffer.
+    SkSizeLimitedDynamicMemoryWStream fStreamBuffer;
 
     const size_t kReadToEnd = 0;
 };