Skip to content

Commit 335150c

Browse files
Copilotthomasturrell
Copilot
and
thomasturrell
committed
Replace regex with custom DurationValidator to address SonarCloud code smell
Co-authored-by: thomasturrell <1552612+thomasturrell@users.noreply.github.com>
1 parent 2807dde commit 335150c

3 files changed

Lines changed: 280 additions & 10 deletions

File tree

‎xapi-model/src/main/java/dev/learning/xapi/model/Result.java‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
import com.fasterxml.jackson.annotation.JsonInclude.Include;
99
import dev.learning.xapi.model.validation.constraints.HasScheme;
1010
import dev.learning.xapi.model.validation.constraints.VaildScore;
11+
import dev.learning.xapi.model.validation.constraints.ValidDuration;
1112
import jakarta.validation.Valid;
12-
import jakarta.validation.constraints.Pattern;
1313
import java.net.URI;
1414
import java.util.LinkedHashMap;
1515
import java.util.function.Consumer;
@@ -41,15 +41,7 @@ public class Result {
4141
private String response;
4242

4343
/** Period of time over which the Statement occurred. */
44-
// Java Duration does not store ISO 8601:2004 durations.
45-
// Using possessive quantifiers on digits to prevent ReDoS (Regular Expression Denial of Service)
46-
@Pattern(
47-
regexp =
48-
"^P\\d++W$|^P(?!$)(\\d++Y)?(\\d++M)?"
49-
+ "(\\d++D)?(T(?=\\d)(\\d++H)?(\\d++M)?((?:\\d++\\.\\d++|\\d++)S)?)?$",
50-
flags = Pattern.Flag.CASE_INSENSITIVE,
51-
message = "Must be a valid ISO 8601:2004 duration format.")
52-
private String duration;
44+
@ValidDuration private String duration;
5345

5446
private LinkedHashMap<@HasScheme URI, Object> extensions;
5547

‎xapi-model/src/main/java/dev/learning/xapi/model/validation/constraints/ValidDuration.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Copyright 2016-2025 Berry Cloud Ltd. All rights reserved.
3+
*/
4+
5+
package dev.learning.xapi.model.validation.constraints;
6+
7+
import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
8+
import static java.lang.annotation.ElementType.CONSTRUCTOR;
9+
import static java.lang.annotation.ElementType.FIELD;
10+
import static java.lang.annotation.ElementType.METHOD;
11+
import static java.lang.annotation.ElementType.PARAMETER;
12+
import static java.lang.annotation.ElementType.TYPE_USE;
13+
import static java.lang.annotation.RetentionPolicy.RUNTIME;
14+
15+
import dev.learning.xapi.model.validation.internal.validators.DurationValidator;
16+
import jakarta.validation.Constraint;
17+
import jakarta.validation.Payload;
18+
import java.lang.annotation.Documented;
19+
import java.lang.annotation.Retention;
20+
import java.lang.annotation.Target;
21+
22+
/**
23+
* The annotated element must be a valid ISO 8601:2004 duration format.
24+
*
25+
* <p>Accepts formats like:
26+
*
27+
* <ul>
28+
* <li>Week format: P1W, P52W
29+
* <li>Day format: P1D, P365D
30+
* <li>Time format: PT1H, PT30M, PT45S, PT1.5S
31+
* <li>Combined format: P1Y2M3D, P1DT1H30M45S
32+
* </ul>
33+
*
34+
* @author Berry Cloud
35+
* @see <a href="https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#result">xAPI
36+
* Result</a>
37+
*/
38+
@Documented
39+
@Constraint(validatedBy = {DurationValidator.class})
40+
@Target({METHOD, FIELD, ANNOTATION_TYPE, CONSTRUCTOR, PARAMETER, TYPE_USE})
41+
@Retention(RUNTIME)
42+
public @interface ValidDuration {
43+
44+
/**
45+
* Error Message.
46+
*
47+
* @return the error message
48+
*/
49+
String message() default "Must be a valid ISO 8601:2004 duration format.";
50+
51+
/**
52+
* Groups.
53+
*
54+
* @return the validation groups
55+
*/
56+
Class<?>[] groups() default {};
57+
58+
/**
59+
* Payload.
60+
*
61+
* @return the payload
62+
*/
63+
Class<? extends Payload>[] payload() default {};
64+
}

‎xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java‎

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
/*
2+
* Copyright 2016-2025 Berry Cloud Ltd. All rights reserved.
3+
*/
4+
5+
package dev.learning.xapi.model.validation.internal.validators;
6+
7+
import dev.learning.xapi.model.validation.constraints.ValidDuration;
8+
import jakarta.validation.ConstraintValidator;
9+
import jakarta.validation.ConstraintValidatorContext;
10+
import java.util.regex.Pattern;
11+
12+
/**
13+
* Validates ISO 8601:2004 duration format strings.
14+
*
15+
* <p>This validator uses a programmatic approach to avoid complex regex patterns that could be
16+
* flagged as code smells. The validation is broken down into logical parts:
17+
*
18+
* <ul>
19+
* <li>Week format: P[n]W (e.g., P1W, P52W)
20+
* <li>Date/time format: P[n]Y[n]M[n]DT[n]H[n]M[n]S or P[n]Y[n]M[n]D
21+
* </ul>
22+
*
23+
* <p>Uses possessive quantifiers (++) to prevent ReDoS attacks.
24+
*
25+
* @author Berry Cloud
26+
*/
27+
public class DurationValidator implements ConstraintValidator<ValidDuration, String> {
28+
29+
// Simple patterns using possessive quantifiers to prevent ReDoS
30+
private static final Pattern WEEK_PATTERN =
31+
Pattern.compile("^P\\d++W$", Pattern.CASE_INSENSITIVE);
32+
private static final Pattern DIGITS_PATTERN = Pattern.compile("^\\d++$");
33+
private static final Pattern DECIMAL_PATTERN = Pattern.compile("^\\d++\\.\\d++$");
34+
35+
@Override
36+
public boolean isValid(String value, ConstraintValidatorContext context) {
37+
if (value == null) {
38+
return true;
39+
}
40+
41+
// Check for week format first (P[n]W)
42+
if (WEEK_PATTERN.matcher(value).matches()) {
43+
return true;
44+
}
45+
46+
// Must start with P
47+
if (!value.toUpperCase().startsWith("P")) {
48+
return false;
49+
}
50+
51+
// Remove P prefix for processing
52+
String remaining = value.substring(1);
53+
54+
// Empty after P is invalid
55+
if (remaining.isEmpty()) {
56+
return false;
57+
}
58+
59+
// Check if there's a time component (T separator)
60+
int timeIndex = remaining.toUpperCase().indexOf('T');
61+
62+
String datePart;
63+
String timePart;
64+
65+
if (timeIndex >= 0) {
66+
datePart = remaining.substring(0, timeIndex);
67+
timePart = remaining.substring(timeIndex + 1);
68+
69+
// T must be followed by time components
70+
if (timePart.isEmpty()) {
71+
return false;
72+
}
73+
} else {
74+
datePart = remaining;
75+
timePart = null;
76+
}
77+
78+
// Validate date part (Y, M, D components)
79+
if (!datePart.isEmpty() && !isValidDatePart(datePart.toUpperCase())) {
80+
return false;
81+
}
82+
83+
// Validate time part (H, M, S components)
84+
if (timePart != null && !isValidTimePart(timePart.toUpperCase())) {
85+
return false;
86+
}
87+
88+
// Must have at least one component
89+
return !datePart.isEmpty() || timePart != null;
90+
}
91+
92+
/**
93+
* Validates the date part of the duration (Y, M, D components).
94+
*
95+
* <p>Components must appear in order: Years, Months, Days.
96+
*/
97+
private boolean isValidDatePart(String datePart) {
98+
if (datePart.isEmpty()) {
99+
return true;
100+
}
101+
102+
int pos = 0;
103+
104+
// Check for Years
105+
int yearIndex = datePart.indexOf('Y');
106+
if (yearIndex >= 0) {
107+
if (yearIndex == 0) {
108+
return false; // No digits before Y
109+
}
110+
String digits = datePart.substring(pos, yearIndex);
111+
if (!isValidDigits(digits)) {
112+
return false;
113+
}
114+
pos = yearIndex + 1;
115+
}
116+
117+
// Check for Months
118+
int monthIndex = datePart.indexOf('M', pos);
119+
if (monthIndex >= 0) {
120+
if (monthIndex == pos) {
121+
return false; // No digits before M
122+
}
123+
String digits = datePart.substring(pos, monthIndex);
124+
if (!isValidDigits(digits)) {
125+
return false;
126+
}
127+
pos = monthIndex + 1;
128+
}
129+
130+
// Check for Days
131+
int dayIndex = datePart.indexOf('D', pos);
132+
if (dayIndex >= 0) {
133+
if (dayIndex == pos) {
134+
return false; // No digits before D
135+
}
136+
String digits = datePart.substring(pos, dayIndex);
137+
if (!isValidDigits(digits)) {
138+
return false;
139+
}
140+
pos = dayIndex + 1;
141+
}
142+
143+
// Nothing should remain after processing
144+
return pos == datePart.length();
145+
}
146+
147+
/**
148+
* Validates the time part of the duration (H, M, S components).
149+
*
150+
* <p>Components must appear in order: Hours, Minutes, Seconds. Only seconds can have decimals.
151+
*/
152+
private boolean isValidTimePart(String timePart) {
153+
if (timePart.isEmpty()) {
154+
return false; // T must be followed by components
155+
}
156+
157+
int pos = 0;
158+
159+
// Check for Hours
160+
int hourIndex = timePart.indexOf('H');
161+
if (hourIndex >= 0) {
162+
if (hourIndex == 0) {
163+
return false; // No digits before H
164+
}
165+
String digits = timePart.substring(pos, hourIndex);
166+
if (!isValidDigits(digits)) {
167+
return false;
168+
}
169+
pos = hourIndex + 1;
170+
}
171+
172+
// Check for Minutes
173+
int minuteIndex = timePart.indexOf('M', pos);
174+
if (minuteIndex >= 0) {
175+
if (minuteIndex == pos) {
176+
return false; // No digits before M
177+
}
178+
String digits = timePart.substring(pos, minuteIndex);
179+
if (!isValidDigits(digits)) {
180+
return false;
181+
}
182+
pos = minuteIndex + 1;
183+
}
184+
185+
// Check for Seconds (can be decimal)
186+
int secondIndex = timePart.indexOf('S', pos);
187+
if (secondIndex >= 0) {
188+
if (secondIndex == pos) {
189+
return false; // No digits before S
190+
}
191+
String digits = timePart.substring(pos, secondIndex);
192+
if (!isValidDigitsOrDecimal(digits)) {
193+
return false;
194+
}
195+
pos = secondIndex + 1;
196+
}
197+
198+
// Nothing should remain after processing and at least one component must be present
199+
return pos == timePart.length() && timePart.length() > 0;
200+
}
201+
202+
/** Checks if the string contains only digits. */
203+
private boolean isValidDigits(String value) {
204+
return !value.isEmpty() && DIGITS_PATTERN.matcher(value).matches();
205+
}
206+
207+
/** Checks if the string contains digits or a decimal number. */
208+
private boolean isValidDigitsOrDecimal(String value) {
209+
if (value.isEmpty()) {
210+
return false;
211+
}
212+
return DIGITS_PATTERN.matcher(value).matches() || DECIMAL_PATTERN.matcher(value).matches();
213+
}
214+
}

0 commit comments

Comments
 (0)