Enforce C-style identifier rules for matrix markers everywhere

Change-Id: I3cc189a77ce5001a828cdc4e73ea9c80c91554b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/286438
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>

src/core/SkCanvas.cpp

@@ -1542,7 +1542,7 @@ void SkCanvas::resetMatrix() {
 }
 
 void SkCanvas::markCTM(const char* name) {
-    if (name && name[0]) {
+    if (SkCanvasPriv::ValidateMarker(name)) {
         fMarkerStack->setMarker(SkOpts::hash_fn(name, strlen(name), 0),
                                 this->getLocalToDevice(), fMCRec);
         this->onMarkCTM(name);
@@ -1550,7 +1550,8 @@ void SkCanvas::markCTM(const char* name) {
 }
 
 bool SkCanvas::findMarkedCTM(const char* name, SkM44* mx) const {
-    return name && name[0] && fMarkerStack->findMarker(SkOpts::hash_fn(name, strlen(name), 0), mx);
+    return SkCanvasPriv::ValidateMarker(name) &&
+           fMarkerStack->findMarker(SkOpts::hash_fn(name, strlen(name), 0), mx);
 }
 
 //////////////////////////////////////////////////////////////////////////////

src/core/SkCanvasPriv.cpp

@@ -10,6 +10,8 @@
 #include "src/core/SkReadBuffer.h"
 #include "src/core/SkWriter32.h"
 
+#include <locale>
+
 SkAutoCanvasMatrixPaint::SkAutoCanvasMatrixPaint(SkCanvas* canvas, const SkMatrix* matrix,
                                                  const SkPaint* paint, const SkRect& bounds)
 : fCanvas(canvas)
@@ -97,3 +99,20 @@ void SkCanvasPriv::GetDstClipAndMatrixCounts(const SkCanvas::ImageSetEntry set[]
     *totalDstClipCount = dstClipCount;
     *totalMatrixCount = maxMatrixIndex + 1;
 }
+
+bool SkCanvasPriv::ValidateMarker(const char* name) {
+    if (!name) {
+        return false;
+    }
+
+    std::locale loc(std::locale::classic());
+    if (!std::isalpha(*name, loc)) {
+        return false;
+    }
+    while (*(++name)) {
+        if (!std::isalnum(*name, loc) && *name != '_') {
+            return false;
+        }
+    }
+    return true;
+}

src/core/SkCanvasPriv.h

@@ -53,6 +53,10 @@ public:
     // computes the minimum length for these arrays that would provide index access errors.
     static void GetDstClipAndMatrixCounts(const SkCanvas::ImageSetEntry set[], int count,
                                           int* totalDstClipCount, int* totalMatrixCount);
+
+    // Checks that the marker name is an identifier ([a-zA-Z][a-zA-Z0-9_]*)
+    // Identifiers with leading underscores are reserved (not allowed).
+    static bool ValidateMarker(const char*);
 };
 
 #endif

src/core/SkRuntimeEffect.cpp

@@ -10,6 +10,7 @@
 #include "include/effects/SkRuntimeEffect.h"
 #include "include/private/SkChecksum.h"
 #include "include/private/SkMutex.h"
+#include "src/core/SkCanvasPriv.h"
 #include "src/core/SkMatrixProvider.h"
 #include "src/core/SkRasterPipeline.h"
 #include "src/core/SkReadBuffer.h"
@@ -53,21 +54,16 @@ private:
 SkSL::Compiler* SharedCompiler::gCompiler = nullptr;
 }
 
-// Accepts either "[a-zA-Z0-9_]+" or "normals([a-zA-Z0-9_]+)"
+// Accepts a valid marker, or "normals(<marker>)"
 static bool parse_marker(const SkSL::StringFragment& marker, uint32_t* id, uint32_t* flags) {
     SkString s = marker;
     if (s.startsWith("normals(") && s.endsWith(')')) {
         *flags |= SkRuntimeEffect::Variable::kMarkerNormals_Flag;
         s.set(marker.fChars + 8, marker.fLength - 9);
     }
-    if (s.isEmpty()) {
+    if (!SkCanvasPriv::ValidateMarker(s.c_str())) {
         return false;
     }
-    for (size_t i = 0; i < s.size(); ++i) {
-        if (!std::isalnum(s[i]) && s[i] != '_') {
-            return false;
-        }
-    }
     *id = SkOpts::hash_fn(s.c_str(), s.size(), 0);
     return true;
 }

src/core/SkVertices.cpp

@@ -9,6 +9,7 @@
 
 #include "include/core/SkData.h"
 #include "include/private/SkTo.h"
+#include "src/core/SkCanvasPriv.h"
 #include "src/core/SkOpts.h"
 #include "src/core/SkReader32.h"
 #include "src/core/SkSafeMath.h"
@@ -31,7 +32,7 @@ static int32_t next_id() {
 SkVertices::Attribute::Attribute(Type t, Usage u, const char* markerName)
         : fType(t)
         , fUsage(u)
-        , fMarkerName((markerName && markerName[0]) ? markerName : nullptr) {
+        , fMarkerName(markerName) {
     fMarkerID = fMarkerName ? SkOpts::hash_fn(fMarkerName, strlen(fMarkerName), 0) : 0;
     SkASSERT(!fMarkerName || fMarkerID != 0);
 }
@@ -67,6 +68,9 @@ size_t SkVertices::Attribute::bytesPerVertex() const {
 }
 
 bool SkVertices::Attribute::isValid() const {
+    if (fMarkerName && !SkCanvasPriv::ValidateMarker(fMarkerName)) {
+        return false;
+    }
     switch (fUsage) {
         case Usage::kRaw:
             return fMarkerID == 0;