Add C++ Pattern Validation (#286)

Envoy now uses RE2 as a safe regex engine instead of std::regex (envoyproxy/envoy#7878). Because PGV already requires patterns to use RE2 syntax, one option is to use RE2 for C++ patterns as well. This implements it, for use in strings, bytes, repeated items, and may key/value pattern validation. Implements #22 WIP: I ran in to difficulty creating the regex because a regex containing a null character would get cut off... for example, the ascii character test used the pattern, ^[\x00-x7f]+$, and consuming this as a string resulted in creating a null-terminated string pattern ^[ instead of the actual pattern. I think this might be a problem across most of the C++ code? That's why there's a terrible string construction in the pattern creation. Signed-off-by: Asra Ali <[email protected]>
bufbuild · Dec 3, 2019 · 76a9789 · 76a9789
1 parent de8fa83
commit 76a9789
Show file tree

Hide file tree

Showing 9 changed files with 57 additions and 23 deletions.
diff --git a/bazel/pgv_proto_library.bzl b/bazel/pgv_proto_library.bzl
@@ -65,6 +65,7 @@ def pgv_cc_proto_library(
             "@com_envoyproxy_protoc_gen_validate//validate:cc_validate",
             "@com_envoyproxy_protoc_gen_validate//validate:validate_cc",
             "@com_google_protobuf//:protobuf",
+            "@com_googlesource_code_re2//:re2",
         ],
         copts = copts + select({
             "@com_envoyproxy_protoc_gen_validate//bazel:windows_x86_64": ["-DWIN32"],

diff --git a/bazel/protobuf.bzl b/bazel/protobuf.bzl
@@ -138,6 +138,9 @@ cc_proto_gen_validate = rule(
             mandatory = True,
             providers = [ProtoInfo],
         ),
+        "_validate_deps": attr.label_list(
+            default = [Label("@com_googlesource_code_re2//:re2")],
+        ),
         "_protoc": attr.label(
             cfg = "host",
             default = Label("@com_google_protobuf//:protoc"),

diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl
@@ -63,6 +63,14 @@ def pgv_dependencies():
             artifact = "com.google.re2j:re2j:1.2",
         )
 
+    if not native.existing_rule("com_googlesource_code_re2"):
+        http_archive(
+            name = "com_googlesource_code_re2",
+            sha256 = "38bc0426ee15b5ed67957017fd18201965df0721327be13f60496f2b356e3e01",
+            strip_prefix = "re2-2019-08-01",
+            urls = ["https://github.com/google/re2/archive/2019-08-01.tar.gz"],
+        )
+
     if not native.existing_rule("com_google_guava"):
         native.maven_jar(
             name = "com_google_guava",

diff --git a/rule_comparison.md b/rule_comparison.md
@@ -22,7 +22,7 @@
 | const                  |✅|✅|✅|✅|✅|
 | len/min\_len/max_len   |✅|✅|✅|✅|✅|
 | min\_bytes/max\_bytes  |✅|✅|✅|✅|✅|
-| pattern                |✅|✅|❌|✅|✅|
+| pattern                |✅|✅|✅|✅|✅|
 | prefix/suffix/contains |✅|✅|✅|✅|✅|
 | contains/not_contains  |✅|✅|✅|✅|✅|
 | in/not_in              |✅|✅|✅|✅|✅|

diff --git a/templates/cc/bytes.go b/templates/cc/bytes.go
@@ -63,6 +63,15 @@ const bytesTpl = `
 	}
 	{{ end }}
 
+        {{ if $r.Pattern }}
+        {
+                if (!RE2::FullMatch(re2::StringPiece({{ accessor . }}.c_str(), {{ accessor . }}.size()),
+                                    {{ lookup $f "Pattern" }})) {
+		        {{ err . "value does not match regex pattern " (lit $r.GetPattern) }}
+	        }
+        }
+	{{ end }}
+
 	{{ if $r.GetIp }}
 		{{ unimplemented "C++ ip address validation is not implemented" }}
 		{{/* TODO(akonradi) implement all of this
@@ -85,13 +94,4 @@ const bytesTpl = `
 		}
 		*/}}
 	{{ end }}
-
-	{{ if $r.Pattern }}
-	{{ unimplemented "C++ pattern validation is not implemented" }}
-	{{/* TODO(akonradi) implement regular expression matching
-	if !{{ lookup $f "Pattern" }}.Match({{ accessor . }}) {
-		return {{ err . "value does not match regex pattern " (lit $r.GetPattern) }}
-	}
-	*/}}
-	{{ end }}
 `
diff --git a/templates/cc/file.go b/templates/cc/file.go
@@ -8,6 +8,7 @@ const moduleFileTpl = `// Code generated by protoc-gen-validate
 
 #include <google/protobuf/message.h>
 #include <google/protobuf/util/time_util.h>
+#include "re2/re2.h"
 
 namespace pgv {
 

diff --git a/templates/cc/msg.go b/templates/cc/msg.go
@@ -48,10 +48,30 @@ const msgTpl = `
 	{{ end }}{{ end }}
 	{{ end }}{{ end }}
 
-	{{ if has .Rules "Pattern"}}{{ if .Rules.Pattern }}
-	{{/* TODO(akonradi) implement pattern matching
-		var {{ lookup .Field "Pattern" }} = regexp.MustCompile({{ lit .Rules.GetPattern }})
-	*/}}
+        {{ if has .Rules "Pattern"}}{{ if .Rules.Pattern }}
+               const re2::RE2 {{ lookup .Field "Pattern" }}(re2::StringPiece({{ lit .Rules.GetPattern }},
+                                                            sizeof({{ lit .Rules.GetPattern }}) - 1));
+	{{ end }}{{ end }}
+
+	{{ if has .Rules "Items"}}{{ if .Rules.Items }}
+        {{ if has .Rules.Items.GetString_ "Pattern" }} {{ if .Rules.Items.GetString_.Pattern }}
+               const re2::RE2 {{ lookup .Field "Pattern" }}(re2::StringPiece({{ lit .Rules.Items.GetString_.GetPattern }},
+                                              sizeof({{ lit .Rules.Items.GetString_.GetPattern }}) - 1));
+	{{ end }}{{ end }}
+        {{ end }}{{ end }}
+
+        {{ if has .Rules "Keys"}}{{ if .Rules.Keys }}
+	{{ if has .Rules.Keys.GetString_ "Pattern" }} {{ if .Rules.Keys.GetString_.Pattern }}
+		const re2::RE2 {{ lookup .Field "Pattern" }}(re2::StringPiece({{ lit .Rules.Keys.GetString_.GetPattern }},
+                                              sizeof({{ lit .Rules.Keys.GetString_.GetPattern }}) - 1));
+	{{ end }}{{ end }}
+	{{ end }}{{ end }}
+
+	{{ if has .Rules "Values"}}{{ if .Rules.Values }}
+	{{ if has .Rules.Values.GetString_ "Pattern" }} {{ if .Rules.Values.GetString_.Pattern }}
+		const re2::RE2 {{ lookup .Field "Pattern" }}(re2::StringPiece({{ lit .Rules.Values.GetString_.GetPattern }},
+                                              sizeof({{ lit .Rules.Values.GetString_.GetPattern }}) - 1));
+	{{ end }}{{ end }}
 	{{ end }}{{ end }}
 
 {{ end }}{{ end }}

diff --git a/templates/cc/string.go b/templates/cc/string.go
@@ -106,6 +106,15 @@ const strTpl = `
 	}
 	{{ end }}
 
+        {{ if $r.Pattern }}
+        {
+                if (!RE2::FullMatch(re2::StringPiece({{ accessor . }}.c_str(), {{ accessor . }}.size()),
+                                    {{ lookup $f "Pattern" }})) {
+		        {{ err . "value does not match regex pattern " (lit $r.GetPattern) }}
+	        }
+        }
+	{{ end }}
+
 	{{ if $r.GetIp }}
 	{
 		const std::string& value = {{ accessor . }};
@@ -174,13 +183,4 @@ const strTpl = `
                 }
 		*/}}
 	{{ end }}
-
-	{{ if $r.Pattern }}
-	{{ unimplemented "C++ pattern validation is not implemented" }}
-	{{/* TODO(akonradi) implement regular expression constraints.
-	if !{{ lookup $f "Pattern" }}.MatchString({{ accessor . }}) {
-		return {{ err . "value does not match regex pattern " (lit $r.GetPattern) }}
-	}
-	*/}}
-	{{ end }}
 `
diff --git a/tests/harness/executor/cases.go b/tests/harness/executor/cases.go
@@ -770,6 +770,7 @@ var stringCases = []TestCase{
 	{"string - pattern - valid", &cases.StringPattern{Val: "Foo123"}, true},
 	{"string - pattern - invalid", &cases.StringPattern{Val: "!@#$%^&*()"}, false},
 	{"string - pattern - invalid (empty)", &cases.StringPattern{Val: ""}, false},
+	{"string - pattern - invalid (null)", &cases.StringPattern{Val: "a\000"}, false},
 
 	{"string - pattern (escapes) - valid", &cases.StringPatternEscapes{Val: "* \\ x"}, true},
 	{"string - pattern (escapes) - invalid", &cases.StringPatternEscapes{Val: "invalid"}, false},