Fix fuzzer-found deserialization bugs

src/core/SkPath.cpp

 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkBuffer.h"
 #include "SkCubicClipper.h"
 #include "SkErrorInternals.h"
@@ skipping 1891 matching lines @@
         return 0;
     }
 
     unsigned version = packed & 0xFF;
 
     fConvexity = (packed >> kConvexity_SerializationShift) & 0xFF;
     fFillType = (packed >> kFillType_SerializationShift) & 0xFF;
     uint8_t dir = (packed >> kDirection_SerializationShift) & 0x3;
     fIsVolatile = (packed >> kIsVolatile_SerializationShift) & 0x1;
     SkPathRef* pathRef = SkPathRef::CreateFromBuffer(&buffer);
+    if (!pathRef) {
+        return 0;
+    }
+
+    if (!buffer.isValid()) {
+        // If the buffer is not valid, pathRef should be nullptr.

[reed1, 2016/01/13 20:27:11]

why are we throwing, instead of just returning, as we do everywhere else we
encounter an invalid buffer?

[ajuma, 2016/01/13 20:47:54]

Looking at where this originated
(https://codereview.chromium.org/61913002#msg16), I believe the distinction is
that this should never be reached, even for an adversarially chosen buffer. The
other places where we just return are places that are reachable given particular
buffers.

[reed1, 2016/01/13 21:05:47]

Lets not add code in places that can never be reached. I think this makes the
code harder to understand, since it is so different from all other "checks" we
do on the buffer. 

I think we don't need a check at all, and we would still be ok.

[ajuma, 2016/01/13 21:11:41]

Removed the check.

+        sk_throw();
+    }
+
+    fPathRef.reset(pathRef);
+    SkDEBUGCODE(this->validate();)
+    buffer.skipToAlign4();
 
     // compatibility check
     if (version < kPathPrivFirstDirection_Version) {
         switch (dir) {  // old values
             case 0:
                 fFirstDirection = SkPathPriv::kUnknown_FirstDirection;
                 break;
             case 1:
                 fFirstDirection = SkPathPriv::kCW_FirstDirection;
                 break;
             case 2:
                 fFirstDirection = SkPathPriv::kCCW_FirstDirection;
                 break;
             default:
                 SkASSERT(false);
         }
     } else {
         fFirstDirection = dir;
     }
 
-    size_t sizeRead = 0;
-    if (buffer.isValid()) {
-        fPathRef.reset(pathRef);
-        SkDEBUGCODE(this->validate();)
-        buffer.skipToAlign4();
-        sizeRead = buffer.pos();
-    } else if (pathRef) {
-        // If the buffer is not valid, pathRef should be nullptr
-        sk_throw();
-    }
-    return sizeRead;
+    return buffer.pos();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 #include "SkStringUtils.h"
 #include "SkStream.h"
 
 static void append_params(SkString* str, const char label[], const SkPoint pts[],
                           int count, SkScalarAsStringType strType, SkScalar conicWeight = -1) {
     str->append(label);
@@ skipping 1134 matching lines @@
         }
     } while (!done);
     return SkToBool(tangents.count()) ^ isInverse;
 }
 
 int SkPath::ConvertConicToQuads(const SkPoint& p0, const SkPoint& p1, const SkPoint& p2,
                                 SkScalar w, SkPoint pts[], int pow2) {
     const SkConic conic(p0, p1, p2, w);
     return conic.chopIntoQuadsPOW2(pts, pow2);
 }