fix regex for non strict header value validation

Author: jonbodner-buf

src/main/java/build/buf/protovalidate/StringRulesEvaluator.java

@@ -109,7 +109,8 @@ private static RuleSite site(int fieldNumber, String ruleId) {
       Pattern.compile("^:?[0-9a-zA-Z!#$%&'*+.\\-^_|~`]+$");
   private static final Pattern HEADER_VALUE_REGEX =
       Pattern.compile("^[^\\x00-\\x08\\x0A-\\x1F\\x7F]*$");
-  private static final Pattern LOOSE_REGEX = Pattern.compile("^[^\\x00\\x0A\\x0D]+$");
+  private static final Pattern LOOSE_HEADER_NAME_REGEX = Pattern.compile("^[^\\x00\\x0A\\x0D]+$");
+  private static final Pattern LOOSE_HEADER_VALUE_REGEX = Pattern.compile("^[^\\x00\\x0A\\x0D]*$");
 
   // --- Well-known string formats ---
 
@@ -778,19 +779,16 @@ public List<RuleViolation.Builder> evaluate(Value val, boolean failFast) {
           return NativeViolations.newViolation(
               HEADER_NAME_EMPTY_SITE, null, null, val, knownRegex.getNumber());
         }
-        matcher = HEADER_NAME_REGEX;
+        matcher = knownRegexStrict ? HEADER_NAME_REGEX : LOOSE_HEADER_NAME_REGEX;
         site = HEADER_NAME_SITE;
         break;
       case KNOWN_REGEX_HTTP_HEADER_VALUE:
-        matcher = HEADER_VALUE_REGEX;
+        matcher = knownRegexStrict ? HEADER_VALUE_REGEX : LOOSE_HEADER_VALUE_REGEX;
         site = HEADER_VALUE_SITE;
         break;
       default:
         return null;
     }
-    if (!knownRegexStrict) {
-      matcher = LOOSE_REGEX;
-    }
     if (!matcher.matches(strVal)) {
       return NativeViolations.newViolation(site, null, null, val, knownRegex.getNumber());
     }

src/test/java/build/buf/protovalidate/WellKnownRegexTest.java

@@ -14,13 +14,19 @@
 
 package build.buf.protovalidate;
 
+import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import build.buf.protovalidate.exceptions.ValidationException;
 import com.example.noimports.validationtest.HttpHeaderName;
 import com.example.noimports.validationtest.HttpHeaderNameLoose;
 import com.example.noimports.validationtest.HttpHeaderValue;
+import com.example.noimports.validationtest.HttpHeaderValueLoose;
+import com.google.protobuf.Message;
+import java.util.List;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 /**
  * Tests for the {@code well_known_regex} oneof case in {@link StringRulesEvaluator}: HTTP header
@@ -32,6 +38,10 @@ class WellKnownRegexTest {
       ValidatorFactory.newBuilder()
           .withConfig(Config.newBuilder().setEnableNativeRules(true).build())
           .build();
+  private final Validator celValidator =
+      ValidatorFactory.newBuilder()
+          .withConfig(Config.newBuilder().setEnableNativeRules(false).build())
+          .build();
 
   @Test
   void headerName_strict_passesValidName() throws ValidationException {
@@ -92,4 +102,65 @@ void headerValue_emptyValueIsValid() throws ValidationException {
     HttpHeaderValue msg = HttpHeaderValue.newBuilder().setVal("").build();
     assertThat(nativeValidator.validate(msg).isSuccess()).isTrue();
   }
+
+  @Test
+  void headerValue_loose_emptyValueIsValid() throws ValidationException {
+    // The loose header value pattern is also '*', unlike the loose header name pattern.
+    HttpHeaderValueLoose msg = HttpHeaderValueLoose.newBuilder().setVal("").build();
+    assertThat(nativeValidator.validate(msg).isSuccess()).isTrue();
+  }
+
+  @Test
+  void headerValue_loose_failsNullCrLf() throws ValidationException {
+    for (String val : new String[] {"a\u0000b", "a\nb", "a\rb"}) {
+      HttpHeaderValueLoose msg = HttpHeaderValueLoose.newBuilder().setVal(val).build();
+      ValidationResult result = nativeValidator.validate(msg);
+      assertThat(result.getViolations()).hasSize(1);
+      build.buf.validate.Violation v = result.getViolations().get(0).toProto();
+      assertThat(v.getRuleId()).isEqualTo("string.well_known_regex.header_value");
+    }
+  }
+
+  // Comma in a strict header name is excluded: the bundled validate.proto still allows it via
+  // the '+-.' range, while native follows upstream's fix (bufbuild/protovalidate#528).
+  @ParameterizedTest
+  @ValueSource(
+      strings = {
+        "",
+        "X-Request-Id",
+        ":authority",
+        "::method",
+        "not a header",
+        "text/plain; charset=utf-8",
+        "tab\there",
+        "a\u0000b",
+        "a\u0001b",
+        "a\u0008b",
+        "a\nb",
+        "a\rb",
+        "a\u001fb",
+        "a\u007fb",
+        "a\u0080b",
+        "naïve",
+        "内容类型",
+        "😀",
+        "trailing\n",
+        "\n",
+      })
+  void nativeMatchesCel(String val) throws ValidationException {
+    assertSameRuleIds(HttpHeaderName.newBuilder().setVal(val).build());
+    assertSameRuleIds(HttpHeaderNameLoose.newBuilder().setVal(val).build());
+    assertSameRuleIds(HttpHeaderValue.newBuilder().setVal(val).build());
+    assertSameRuleIds(HttpHeaderValueLoose.newBuilder().setVal(val).build());
+  }
+
+  private void assertSameRuleIds(Message msg) throws ValidationException {
+    assertThat(ruleIds(nativeValidator.validate(msg)))
+        .as("%s{val=%s}", msg.getDescriptorForType().getName(), msg)
+        .isEqualTo(ruleIds(celValidator.validate(msg)));
+  }
+
+  private static List<String> ruleIds(ValidationResult result) {
+    return result.getViolations().stream().map(v -> v.toProto().getRuleId()).collect(toList());
+  }
 }

src/test/resources/proto/validationtest/validationtest.proto

@@ -371,6 +371,13 @@ message HttpHeaderValue {
   string val = 1 [(buf.validate.field).string.well_known_regex = KNOWN_REGEX_HTTP_HEADER_VALUE];
 }
 
+message HttpHeaderValueLoose {
+  string val = 1 [(buf.validate.field).string = {
+    well_known_regex: KNOWN_REGEX_HTTP_HEADER_VALUE
+    strict: false
+  }];
+}
+
 // google.protobuf.*Value wrapper fixtures for WrappedValueEvaluator coverage.
 message Int64WrapperConst {
   google.protobuf.Int64Value val = 1 [(buf.validate.field).int64.const = 5];