Issue-452 : Add support for annotation based input validation by aabashkin · Pull Request #903 · ESAPI/esapi-java-legacy

aabashkin · 2026-01-05T21:44:52Z

@kwwall please review at your convenience, thanks.

Copilot

Pull request overview

This PR implements annotation-based input validation support for ESAPI using the Java Bean Validation (JSR 380) API. It introduces 17 custom validation annotations that wrap existing ESAPI validation methods, enabling declarative validation of fields, parameters, and method return values.

Key changes:

Created 17 custom validation annotations (ValidString, ValidCreditCard, ValidURI, etc.) with corresponding validator implementations
Added ValidationUtil helper class for centralized error handling
Comprehensive test suite covering all validation annotations
Added javax.validation-api (2.0.1.Final) as a compile dependency and Hibernate Validator (6.2.5.Final) as a test dependency

Reviewed changes

Copilot reviewed 36 out of 37 changed files in this pull request and generated 23 comments.

Show a summary per file

File	Description
ValidationAnnotationsTest.java	Comprehensive test suite covering all 17 validation annotations with positive and negative test cases
ValidationUtil.java	Utility class for converting ValidationErrorList to constraint violations
ValidURIValidator.java / ValidURI.java	URI validation annotation and validator
ValidStringValidator.java / ValidString.java	String validation annotation and validator with type and length constraints
ValidSafeHTMLValidator.java / ValidSafeHTML.java	Safe HTML validation annotation and validator
ValidRedirectLocationValidator.java / ValidRedirectLocation.java	Redirect location validation annotation and validator
ValidPrintableValidator.java / ValidPrintableStringValidator.java / ValidPrintable.java	Printable character validation for char arrays and strings
ValidNumberValidator.java / ValidNumber.java	Number validation annotation and validator with range constraints
ValidListItemValidator.java / ValidListItem.java	List item validation annotation and validator
ValidIntegerValidator.java / ValidInteger.java	Integer validation annotation and validator with range constraints
ValidHTTPRequestParameterSetValidator.java / ValidHTTPRequestParameterSet.java	HTTP request parameter set validation
ValidFileUploadValidator.java / ValidFileUpload.java	File upload validation with size and location constraints
ValidFileNameValidator.java / ValidFileName.java	File name validation with extension constraints
ValidFileContentValidator.java / ValidFileContent.java	File content validation with size constraints
ValidDoubleValidator.java / ValidDouble.java	Double validation annotation and validator with range constraints
ValidDirectoryPathValidator.java / ValidDirectoryPath.java	Directory path validation with parent directory constraints
ValidDateValidator.java / ValidDate.java	Date validation with format and locale support
ValidCreditCardValidator.java / ValidCreditCard.java	Credit card validation using Luhn algorithm
DefaultValidator.java	Minor formatting change (trailing whitespace)
pom.xml	Added javax.validation-api, hibernate-validator, and javax.el dependencies

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/main/java/org/owasp/esapi/reference/DefaultValidator.java

Copilot · 2026-01-05T21:50:12Z

src/main/java/org/owasp/esapi/reference/validation/annotations/ValidStringValidator.java

+        if (input == null) {
+            return true;
+        }


The null check returns true even when allowNull is false. This means null values will always pass validation regardless of the allowNull setting. The validator should return allowNull when input is null to properly respect the allowNull configuration.

We need to make a design decision here:

null values are accepted unless @NotNull is present. Since the user would already be using annotations, it could make sense to assume that they would leverage an existing, often used constraint.
OR

Require allowNull everywhere

I was leaning towards 1 initially but I'm starting to consider 2 more seriously. Thoughts?

@kwwall ping

@aabashkin - So for the delay; my wife has been having medical problems (still is, in fact), so I've had to prioritize that.

I have been looking at this though a bit though and thinking about it deeply. The potential problem that I foresee with approach 1 is that, what happens if someone uses the @NotNull annotation, but then also specifies allowNull as true? How do we resolve that case? I'm not can be resolved at compile-time, but even at runtime, it seems as though may some RuntimeException or perhaps even some sort of custom subclass of java.lang.Error should be thrown.

The other way might be eliminate allowNull as an explicit parameter in all these annotations where it's used and simply treat it as true unless @NotNull is present, but I'm not sure how that would work because I'm not sure if annotation order matters or not. I would think it would, and in that case, if someone did something like this:

@NotNull @ValidNumber(context = "num", minValue = 1, maxValue = 10000) String queryParmNum;

versus an ordering like this:

@ValidNumber(context = "num", minValue = 1, maxValue = 10000) @NotNull String queryParmNum;

my intuition tells me that that difference may be significant from the way code gets generated,

Somewhat related to all of this is the testing. For example, a quick perusal of src/test/java/org/owasp/esapi/reference/validation/annotations/ValidationAnnotationsTest.java seems to indicate that only "sunny-day" tests are provided. In ESAPI, we need to test failure cases as well...ideally all of them, but at least enough to get code-coverage of 70%. Yes, there are some older, rarely used components that don't have anywhere near that. I hope to address that by getting genAI to create some additional test cases, but that's probably still 5-6 months out. But I have no objection to you using genAI with something like Copilot or Claude Code, etc. as long as the prompts are saved and included in GitHub somewhere and the generated code is maintainable by a human.

Lastly, the Javadoc documentation needs some enhancing. I haven't looked through it in detail yet, but I did notice the total absence of package.html in all your new packages. (We don't need to have that for src/test stuff, but do want it for src/main.)

I will try to look at this more closely in the following weeks, but as I mention, family comes first, so no promise of how soon. Thanks for your understanding in this matter.

Copilot · 2026-01-05T21:50:12Z

src/main/java/org/owasp/esapi/reference/validation/annotations/ValidIntegerValidator.java

+    @Override
+    public boolean isValid(String input, ConstraintValidatorContext constraintValidatorContext) {
+        if (input == null) {
+            return true;


The null check returns true even when allowNull is false. This means null values will always pass validation regardless of the allowNull setting. The validator should return allowNull when input is null to properly respect the allowNull configuration.

Suggested change

return true;

return allowNull;

Copilot · 2026-01-05T21:50:13Z

src/main/java/org/owasp/esapi/reference/validation/annotations/ValidationUtil.java

+
+public class ValidationUtil {
+
+    private ValidationUtil(){}


Missing space after closing brace. The constructor should have a space before the opening brace for consistency with Java formatting conventions.

Copilot · 2026-01-05T21:50:13Z

src/main/java/org/owasp/esapi/reference/validation/annotations/ValidURIValidator.java

+        if (input == null) {
+            return true;
+        }


The null check returns true even when allowNull is false. This means null values will always pass validation regardless of the allowNull setting. The validator should return allowNull when input is null to properly respect the allowNull configuration.

Copilot · 2026-01-05T21:50:17Z

src/main/java/org/owasp/esapi/reference/validation/annotations/ValidListItemValidator.java

+    @Override
+    public boolean isValid(String input, ConstraintValidatorContext constraintValidatorContext) {
+        if (input == null) {
+            return true;


The null check returns true even when allowNull is false. This means null values will always pass validation regardless of the allowNull setting. The validator should return allowNull when input is null to properly respect the allowNull configuration.

Suggested change

return true;

return allowNull;

Copilot · 2026-01-05T21:50:17Z

src/main/java/org/owasp/esapi/reference/validation/annotations/ValidFileNameValidator.java

+    @Override
+    public boolean isValid(String input, ConstraintValidatorContext constraintValidatorContext) {
+        if (input == null) {
+            return true;


The null check returns true even when allowNull is false. This means null values will always pass validation regardless of the allowNull setting. The validator should return allowNull when input is null to properly respect the allowNull configuration.

Suggested change

return true;

return allowNull;

Copilot · 2026-01-05T21:50:17Z

src/main/java/org/owasp/esapi/reference/validation/annotations/ValidDoubleValidator.java

+    @Override
+    public boolean isValid(String input, ConstraintValidatorContext constraintValidatorContext) {
+        if (input == null) {
+            return true;


The null check returns true even when allowNull is false. This means null values will always pass validation regardless of the allowNull setting. The validator should return allowNull when input is null to properly respect the allowNull configuration.

Suggested change

return true;

return allowNull;

Copilot · 2026-01-05T21:50:18Z

src/main/java/org/owasp/esapi/reference/validation/annotations/ValidDirectoryPathValidator.java

+    @Override
+    public boolean isValid(String input, ConstraintValidatorContext constraintValidatorContext) {
+        if (input == null) {
+            return true;


The null check returns true even when allowNull is false. This means null values will always pass validation regardless of the allowNull setting. The validator should return allowNull when input is null to properly respect the allowNull configuration.

Suggested change

return true;

return allowNull;

Copilot · 2026-01-05T21:50:18Z

src/main/java/org/owasp/esapi/reference/validation/annotations/ValidCreditCardValidator.java

+    @Override
+    public boolean isValid(String input, ConstraintValidatorContext constraintValidatorContext) {
+        if (input == null) {
+            return true;


The null check returns true even when allowNull is false. This means null values will always pass validation regardless of the allowNull setting. The validator should return allowNull when input is null to properly respect the allowNull configuration.

Suggested change

return true;

return allowNull;

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

aabashkin added 13 commits December 26, 2022 23:36

first commit

13bb087

change defaults in ValidString annotation

a5054c2

create remaining annotations

7465eab

Use user-facing messages for validation violations

cec4eb7

Use ESAPI.validator in bean validation constraints

9cf48c4

Allow nulls in bean validation constraints

880347c

Add message for invalid URI constraint

3a75f7d

Handle locale tags in date validator

602472c

Support @ValidPrintable on String values

00160db

Fix ValidationUtil helper typo

50f8fe8

Fix ValidSafeHTML bean validator

e7c61c4

Add bean validation annotation tests

7f0713b

Test consolidation

794119b

Copilot AI review requested due to automatic review settings January 5, 2026 21:44

Copilot started reviewing on behalf of aabashkin January 5, 2026 21:45 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes

Fix whitespace

4a29156

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Conversation

aabashkin commented Jan 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

aabashkin Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

aabashkin Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

kwwall Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants