Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions src/main/java/build/buf/protovalidate/StringRulesEvaluator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 ---

Expand Down Expand Up @@ -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());
}
Expand Down
71 changes: 71 additions & 0 deletions src/test/java/build/buf/protovalidate/WellKnownRegexTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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());
}
}
7 changes: 7 additions & 0 deletions src/test/resources/proto/validationtest/validationtest.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
Loading