This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new bb5761dff WW-5702 fix(core): tighten the HTML5 constraint provider's
attribute hygiene (#1935)
bb5761dff is described below
commit bb5761dff0c0c18cd7e5a859a2c20bee208d1045
Author: Lukasz Lenart <[email protected]>
AuthorDate: Mon Sep 14 08:45:34 2026 +0200
WW-5702 fix(core): tighten the HTML5 constraint provider's attribute
hygiene (#1935)
* WW-5702 fix(core): tighten the HTML5 constraint provider's attribute
hygiene
Items 2, 5, 6, 8, 9, 10 and 11 of the review follow-up to WW-5695.
- data-msg-* is emitted only for a control that submits a value; s:label
and unrecognised types get none. Select, checkbox and hidden keep the
documented script hook.
- max goes through a finiteness guard like min: a Date-typed range on a
numeric control no longer renders Date.toString(), a NaN or infinite
double bound is omitted.
- Integrality of min is decided on the decimal representation, which is
what gets rendered; a BigDecimal that only rounds to a whole number as
a double no longer shifts the step base.
- A type entry from a provider is discarded before rendering, and the
HtmlConstraintProvider javadoc no longer advertises the type="email"
override that produced a duplicate attribute.
- Dynamic attribute names are compared case-insensitively, as HTML does,
so a developer's MAXLENGTH wins over the derived maxlength.
- A validator type that is not a plain attribute name gets no data-msg
attribute; the name is outside FreeMarker's escaping.
- EcmaScriptSafeRegex admits \- only inside a character class, where the
browser's Unicode-mode compiler accepts it, and rejects a class opening
with a literal ] that the two engines read differently.
HtmlControlType.OTHER is renamed UNSUPPORTED now that the constant is
load-bearing: it names what happens (no constraint, no message), where
OTHER conflated unknown type values with non-submitting components.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
* WW-5702 fix(core): align EcmaScriptSafeRegex with the browser's
unicode-sets mode
Review of the hygiene change showed the allowlist still admitted patterns
the browser silently drops: HTML compiles `pattern` with the `v` flag,
under which ( ) { } / | must be escaped inside a class, a hyphen there is
only a range operator between two plain literals, doubled punctuators
are reserved, and a lone ] or } outside a class is an error. All read as
literals by Java, so `[a-z0-9._%+-]+@` passed and did nothing. Verified
on node 24.
Also from review: a provider's `Type`/`Maxlength` is dropped regardless
of case like the dynamic-attribute check already was, and a dotted
validator type (`acme.required`) keeps its data-msg attribute — only the
colon stays out, as an XML namespace prefix.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
* WW-5702 fix(core): treat [^ as the class opening and reject stacked
quantifiers
Second review pass on the unicode-sets alignment: the negation marker
was scanned as a literal a range could start from, so [^-a] passed while
the browser throws; and a{2}{3} passed although only Java stacks
quantifiers. Verified against Java and node 24 over a corpus of typical
validator regexes — every accepted pattern now compiles in both.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
* WW-5702 refactor(core): split the regex allowlist scan into a stateful
Scanner
Sonar S3776: isSafe reached cognitive complexity 43 once the unicode-sets
rules landed in one loop. The state (position, in-class, range start,
after-quantifier) moves into a private Scanner with one method per
branch; verdicts over the differential corpus are unchanged.
Sonar S1117: the lambda parameter in addConstraintAttributes hid the
UIBean.name field.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
* WW-5702 refactor(core): make the regex allowlist helpers Scanner methods
Sonar S3398 on the previous split: the static helpers were called only
from Scanner. They are instance methods now and read the regex and
position from the scanner instead of threading them through; corpus
verdicts are unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
---------
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../struts2/components/EcmaScriptSafeRegex.java | 244 ++++++++++++++++-----
.../struts2/components/HtmlConstraintProvider.java | 9 +-
.../apache/struts2/components/HtmlControlType.java | 15 +-
.../components/StrutsHtmlConstraintProvider.java | 43 +++-
.../java/org/apache/struts2/components/UIBean.java | 24 +-
.../components/ConstraintAttributesTest.java | 46 ++++
.../apache/struts2/components/ControlTypeTest.java | 2 +-
.../components/EcmaScriptSafeRegexTest.java | 73 ++++++
.../struts2/components/HtmlControlTypeTest.java | 14 +-
.../StrutsHtmlConstraintProviderTest.java | 80 ++++++-
10 files changed, 474 insertions(+), 76 deletions(-)
diff --git
a/core/src/main/java/org/apache/struts2/components/EcmaScriptSafeRegex.java
b/core/src/main/java/org/apache/struts2/components/EcmaScriptSafeRegex.java
index 2614c5c57..8400680ea 100644
--- a/core/src/main/java/org/apache/struts2/components/EcmaScriptSafeRegex.java
+++ b/core/src/main/java/org/apache/struts2/components/EcmaScriptSafeRegex.java
@@ -47,7 +47,15 @@ public final class EcmaScriptSafeRegex {
* every browser. On the Java 17 baseline that is a false reject, and no
version check could fix
* it: one {@code validation.xml} would have to mean two different things
depending on the JVM.
*/
- private static final String ALLOWED_ESCAPES = "dDwWnrtf\\.*+?()[]{}|^$/-";
+ private static final String ALLOWED_ESCAPES = "dDwWnrtf\\.*+?()[]{}|^$/";
+
+ /**
+ * Browsers compile {@code pattern} with the {@code v} (unicode sets)
flag. Inside a character
+ * class that mode reserves these unescaped, and any doubled {@link
#CLASS_PUNCTUATORS}, as syntax;
+ * Java reads them as literals.
+ */
+ private static final String CLASS_SYNTAX = "(){}/|";
+ private static final String CLASS_PUNCTUATORS = "&!#$%*+,.:;<=>?@^`~";
private EcmaScriptSafeRegex() {
}
@@ -56,69 +64,205 @@ public final class EcmaScriptSafeRegex {
if (regex == null || regex.isEmpty()) {
return false;
}
- boolean inCharClass = false;
- int i = 0;
- while (i < regex.length()) {
- char current = regex.charAt(i);
- if (!isPortable(regex, i, current, inCharClass)) {
+ Scanner scanner = new Scanner(regex);
+ while (scanner.hasMore()) {
+ if (!scanner.scanNext()) {
return false;
}
- if (current == '[') {
- inCharClass = true;
- } else if (current == ']') {
- inCharClass = false;
- }
- // an escape consumes the character it escapes, which must not be
scanned again
- i += (current == '\\') ? 2 : 1;
}
- return !inCharClass;
+ return !scanner.inCharClass;
}
/**
- * Whether the construct starting at {@code index} means the same thing to
both engines. This is
- * the whole allowlist: anything that reaches {@code default} is a
character with no special
- * meaning in either engine, or one whose meaning is shared.
+ * Walks the regex one unit at a time — a character, an escape pair, or a
whole {@code {n,m}}
+ * quantifier — and refuses the first one the two engines disagree on. The
allowlist lives in
+ * {@link #isPortable} and {@link #isPortableInClass}: anything that
reaches their default branch is
+ * a character with no special meaning in either engine, or one whose
meaning is shared.
*/
- private static boolean isPortable(String regex, int index, char current,
boolean inCharClass) {
- switch (current) {
- case '\\':
- return isAllowedEscape(regex, index);
- case '[':
- // Java allows nested classes and POSIX names; ECMAScript
allows neither
- return !inCharClass && !regex.startsWith("[:", index);
- case '&':
- // Java character-class intersection
- return !inCharClass || !isFollowedBy(regex, index, '&');
- case '(':
- return isPortableGroup(regex, index);
- case '*', '+', '?', '}':
- // possessive quantifier
- return !isFollowedBy(regex, index, '+');
- default:
+ private static final class Scanner {
+ private final String regex;
+ private int index;
+ private boolean inCharClass;
+ // true while the previous unit is a plain literal a range can start
from
+ private boolean rangeStartAvailable;
+ // true right after a quantifier: Java stacks them (a{2}{3}), the
browser has nothing to repeat
+ private boolean afterQuantifier;
+
+ Scanner(String regex) {
+ this.regex = regex;
+ }
+
+ boolean hasMore() {
+ return index < regex.length();
+ }
+
+ boolean scanNext() {
+ char current = regex.charAt(index);
+ if (current == '\\') {
+ return scanEscape();
+ }
+ if (inCharClass) {
+ return scanInClass(current);
+ }
+ if (current == '{') {
+ return scanQuantifier();
+ }
+ return scanOutsideClass(current);
+ }
+
+ private boolean scanEscape() {
+ if (!isAllowedEscape()) {
+ return false;
+ }
+ // an escape consumes the character it escapes, which must not be
scanned again;
+ // in unicode-sets mode a class escape cannot bound a range either
+ rangeStartAvailable = false;
+ afterQuantifier = false;
+ index += 2;
+ return true;
+ }
+
+ private boolean scanInClass(char current) {
+ if (current == ']') {
+ inCharClass = false;
+ index++;
return true;
+ }
+ if (current == '-') {
+ if (!isRangeOperator()) {
+ return false;
+ }
+ rangeStartAvailable = false;
+ index += 2;
+ return true;
+ }
+ if (!isPortableInClass(current)) {
+ return false;
+ }
+ rangeStartAvailable = true;
+ index++;
+ return true;
}
- }
- private static boolean isAllowedEscape(String regex, int index) {
- return index + 1 < regex.length() &&
ALLOWED_ESCAPES.indexOf(regex.charAt(index + 1)) >= 0;
- }
+ private boolean scanQuantifier() {
+ int close = endOfQuantifier();
+ if (afterQuantifier || close < 0 || isFollowedBy(close, '+')) {
+ return false;
+ }
+ afterQuantifier = true;
+ index = close + 1;
+ return true;
+ }
- /**
- * Only non-capturing groups and lookahead are portable; named groups,
lookbehind, atomic groups
- * and inline flags are not. A plain capturing group is always fine.
- */
- private static boolean isPortableGroup(String regex, int index) {
- if (!isFollowedBy(regex, index, '?')) {
+ private boolean scanOutsideClass(char current) {
+ boolean stacked = afterQuantifier && (current == '*' || current ==
'+');
+ if (stacked || !isPortable(current)) {
+ return false;
+ }
+ if (current == '[') {
+ inCharClass = true;
+ rangeStartAvailable = false;
+ // the negation marker is part of the class opening, not a
literal
+ index += isFollowedBy(index, '^') ? 2 : 1;
+ } else {
+ index++;
+ }
+ afterQuantifier = current == '*' || current == '+' || current ==
'?';
return true;
}
- if (index + 2 >= regex.length()) {
- return false;
+
+ /**
+ * Whether the construct at the current position, outside a character
class, means the same
+ * thing to both engines.
+ */
+ private boolean isPortable(char current) {
+ switch (current) {
+ case '[':
+ // Java allows POSIX names and a leading literal ];
ECMAScript allows neither
+ return !regex.startsWith("[:", index) &&
!opensWithLiteralBracket();
+ case ']', '}':
+ // a literal in Java, "lone quantifier brackets" in the
browser
+ return false;
+ case '(':
+ return isPortableGroup();
+ case '*', '+', '?':
+ // possessive quantifier
+ return !isFollowedBy(index, '+');
+ default:
+ return true;
+ }
+ }
+
+ private boolean isPortableInClass(char current) {
+ if (current == '[' || CLASS_SYNTAX.indexOf(current) >= 0) {
+ // nested classes are Java-only; the rest are unicode-sets
syntax characters
+ return false;
+ }
+ return CLASS_PUNCTUATORS.indexOf(current) < 0 ||
!isFollowedBy(index, current);
+ }
+
+ /**
+ * Inside a class an unescaped hyphen is portable only as a range
operator between two plain
+ * literals: {@code [a-z]}. Anywhere else Java reads it as a literal
and the browser throws.
+ */
+ private boolean isRangeOperator() {
+ if (!rangeStartAvailable || index + 1 >= regex.length()) {
+ return false;
+ }
+ char end = regex.charAt(index + 1);
+ return end != ']' && end != '\\' && end != '-' && end != '[' &&
CLASS_SYNTAX.indexOf(end) < 0;
}
- char kind = regex.charAt(index + 2);
- return kind == ':' || kind == '=' || kind == '!';
- }
- private static boolean isFollowedBy(String regex, int index, char
expected) {
- return index + 1 < regex.length() && regex.charAt(index + 1) ==
expected;
+ /**
+ * Index of the {@code }} closing the {@code {n}}, {@code {n,}} or
{@code {n,m}} quantifier that
+ * opens at the current position, or -1 when the braces do not form
one — which Java rejects as well.
+ */
+ private int endOfQuantifier() {
+ int close = regex.indexOf('}', index);
+ if (close < 0 || !regex.substring(index + 1,
close).matches("\\d+(,\\d*)?")) {
+ return -1;
+ }
+ return close;
+ }
+
+ private boolean isAllowedEscape() {
+ if (index + 1 >= regex.length()) {
+ return false;
+ }
+ char escaped = regex.charAt(index + 1);
+ // in unicode mode \- is only legal inside a class
+ if (escaped == '-') {
+ return inCharClass;
+ }
+ return ALLOWED_ESCAPES.indexOf(escaped) >= 0;
+ }
+
+ /**
+ * Java reads a {@code ]} directly after {@code [} or {@code [^} as a
literal member of the
+ * class; the browser's unicode-mode compiler rejects it.
+ */
+ private boolean opensWithLiteralBracket() {
+ int first = isFollowedBy(index, '^') ? index + 2 : index + 1;
+ return first < regex.length() && regex.charAt(first) == ']';
+ }
+
+ /**
+ * Only non-capturing groups and lookahead are portable; named groups,
lookbehind, atomic
+ * groups and inline flags are not. A plain capturing group is always
fine.
+ */
+ private boolean isPortableGroup() {
+ if (!isFollowedBy(index, '?')) {
+ return true;
+ }
+ if (index + 2 >= regex.length()) {
+ return false;
+ }
+ char kind = regex.charAt(index + 2);
+ return kind == ':' || kind == '=' || kind == '!';
+ }
+
+ private boolean isFollowedBy(int at, char expected) {
+ return at + 1 < regex.length() && regex.charAt(at + 1) == expected;
+ }
}
}
diff --git
a/core/src/main/java/org/apache/struts2/components/HtmlConstraintProvider.java
b/core/src/main/java/org/apache/struts2/components/HtmlConstraintProvider.java
index 6fdeaa52b..926065984 100644
---
a/core/src/main/java/org/apache/struts2/components/HtmlConstraintProvider.java
+++
b/core/src/main/java/org/apache/struts2/components/HtmlConstraintProvider.java
@@ -27,8 +27,13 @@ import java.util.Map;
* Maps a field's validators onto the HTML attributes a theme should render
for it.
* <p>
* The default implementation is deliberately conservative — see {@link
StrutsHtmlConstraintProvider}.
- * Applications wanting a best-effort mapping (an {@code email} validator
becoming
- * {@code type="email"}, say) should register their own implementation instead.
+ * Applications wanting a less conservative mapping (a rewritten {@code
pattern} for a
+ * case-insensitive regex, say) should register their own implementation
instead.
+ * <p>
+ * A {@code type} entry in the returned map is discarded: the templates have
already written the
+ * input's {@code type} by the time the map renders, so changing it needs a
template override, not a
+ * provider. Any entry whose name matches an attribute the developer set on
the tag is discarded too —
+ * the developer's own value always wins.
*
* @since 7.4.0
*/
diff --git
a/core/src/main/java/org/apache/struts2/components/HtmlControlType.java
b/core/src/main/java/org/apache/struts2/components/HtmlControlType.java
index 6e69617b9..c432042f7 100644
--- a/core/src/main/java/org/apache/struts2/components/HtmlControlType.java
+++ b/core/src/main/java/org/apache/struts2/components/HtmlControlType.java
@@ -38,7 +38,12 @@ public enum HtmlControlType {
DATE, MONTH, WEEK, TIME, DATETIME_LOCAL,
CHECKBOX, RADIO, FILE, HIDDEN, SELECT,
TEXTAREA,
- OTHER;
+ /**
+ * A control no constraint applies to and no {@code data-msg-*} message is
rendered for: a
+ * {@code type} the framework does not recognise, or a component with no
+ * {@code getControlType()} override ({@code <s:label>}, {@code
<s:submit>} and the like).
+ */
+ UNSUPPORTED;
private static final Set<HtmlControlType> TEXT_ENTRY = EnumSet.of(TEXT,
SEARCH, TEL, PASSWORD, EMAIL, URL);
private static final Set<HtmlControlType> NUMERIC = EnumSet.of(NUMBER,
RANGE);
@@ -46,21 +51,21 @@ public enum HtmlControlType {
/**
* Resolves a raw {@code type} attribute value. Never throws: the
attribute is OGNL-evaluated, so at
- * runtime it can be any string. Anything unrecognised becomes {@link
#OTHER}, which supports no
+ * runtime it can be any string. Anything unrecognised becomes {@link
#UNSUPPORTED}, which supports no
* constraints at all — so an unknown control degrades to emitting nothing.
*/
public static HtmlControlType from(String type) {
if (type == null) {
- return OTHER;
+ return UNSUPPORTED;
}
String normalised = type.trim().toUpperCase(Locale.ROOT).replace('-',
'_');
if (normalised.isEmpty()) {
- return OTHER;
+ return UNSUPPORTED;
}
try {
return valueOf(normalised);
} catch (IllegalArgumentException e) {
- return OTHER;
+ return UNSUPPORTED;
}
}
diff --git
a/core/src/main/java/org/apache/struts2/components/StrutsHtmlConstraintProvider.java
b/core/src/main/java/org/apache/struts2/components/StrutsHtmlConstraintProvider.java
index 79790aabb..4f538037c 100644
---
a/core/src/main/java/org/apache/struts2/components/StrutsHtmlConstraintProvider.java
+++
b/core/src/main/java/org/apache/struts2/components/StrutsHtmlConstraintProvider.java
@@ -28,9 +28,11 @@ import
org.apache.struts2.validator.validators.RequiredFieldValidator;
import org.apache.struts2.validator.validators.RequiredStringValidator;
import org.apache.struts2.validator.validators.StringLengthFieldValidator;
+import java.math.BigDecimal;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.regex.Pattern;
/**
* Default {@link HtmlConstraintProvider}.
@@ -50,6 +52,11 @@ public class StrutsHtmlConstraintProvider implements
HtmlConstraintProvider {
* The HTML5 boolean attribute; its canonical serialisation repeats the
attribute name as the value.
*/
private static final String REQUIRED = "required";
+ /**
+ * What a validator type may contain to become part of a {@code
data-msg-*} name: no character that
+ * ends or splits an attribute name, and no colon, which an XML parser
reads as a namespace prefix.
+ */
+ private static final Pattern ATTRIBUTE_NAME =
Pattern.compile("[A-Za-z0-9_.-]+");
@Override
public Map<String, String> constraintsFor(List<Validator> validators,
HtmlControlType control, Object action) {
@@ -59,7 +66,9 @@ public class StrutsHtmlConstraintProvider implements
HtmlConstraintProvider {
}
for (Validator validator : validators) {
addConstraints(attributes, validator, control);
- addMessage(attributes, validator, action);
+ if (control != HtmlControlType.UNSUPPORTED) {
+ addMessage(attributes, validator, action);
+ }
}
return attributes;
}
@@ -154,7 +163,10 @@ public class StrutsHtmlConstraintProvider implements
HtmlConstraintProvider {
if (isIntegral(min)) {
putIfPresent(attributes, "min", min);
}
- putIfPresent(attributes, "max", validator.getMax());
+ Object max = validator.getMax();
+ if (isFiniteNumber(max)) {
+ putIfPresent(attributes, "max", max);
+ }
}
protected void addDoubleRange(Map<String, String> attributes,
DoubleRangeFieldValidator validator, HtmlControlType control) {
@@ -170,7 +182,10 @@ public class StrutsHtmlConstraintProvider implements
HtmlConstraintProvider {
if (isIntegral(minInclusive)) {
putIfPresent(attributes, "min", minInclusive);
}
- putIfPresent(attributes, "max", validator.getMaxInclusive());
+ Double maxInclusive = validator.getMaxInclusive();
+ if (isFiniteNumber(maxInclusive)) {
+ putIfPresent(attributes, "max", maxInclusive);
+ }
}
private boolean isNumericRange(HtmlControlType control) {
@@ -183,20 +198,38 @@ public class StrutsHtmlConstraintProvider implements
HtmlConstraintProvider {
* step base, so only {@code min} needs this guard.
*/
private boolean isIntegral(Object value) {
+ if (!isFiniteNumber(value)) {
+ return false;
+ }
+ // decided on the decimal representation, which is also what gets
rendered: a BigDecimal
+ // such as 1.0000000000000000001 rounds to 1.0 as a double yet renders
with its fraction
+ try {
+ return new
BigDecimal(value.toString()).stripTrailingZeros().scale() <= 0;
+ } catch (NumberFormatException e) {
+ return false;
+ }
+ }
+
+ private boolean isFiniteNumber(Object value) {
if (!(value instanceof java.lang.Number number)) {
return false;
}
double asDouble = number.doubleValue();
- return !Double.isNaN(asDouble) && !Double.isInfinite(asDouble) &&
asDouble == Math.floor(asDouble);
+ return !Double.isNaN(asDouble) && !Double.isInfinite(asDouble);
}
protected void addMessage(Map<String, String> attributes, Validator
validator, Object action) {
if (action == null) {
return;
}
+ // the type becomes part of the attribute name, which FreeMarker's
auto-escaping does not cover
+ String type = validator.getValidatorType();
+ if (type == null || !ATTRIBUTE_NAME.matcher(type).matches()) {
+ return;
+ }
String message = validator.getMessage(action);
if (message != null && !message.isEmpty()) {
- attributes.put("data-msg-" + validator.getValidatorType(),
message);
+ attributes.put("data-msg-" + type, message);
}
}
diff --git a/core/src/main/java/org/apache/struts2/components/UIBean.java
b/core/src/main/java/org/apache/struts2/components/UIBean.java
index af466d386..089950fbe 100644
--- a/core/src/main/java/org/apache/struts2/components/UIBean.java
+++ b/core/src/main/java/org/apache/struts2/components/UIBean.java
@@ -50,6 +50,7 @@ import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.function.Function;
import static java.util.Collections.emptyMap;
@@ -959,7 +960,10 @@ public abstract class UIBean extends Component {
return;
}
constraints = new LinkedHashMap<>(constraints);
- constraints.keySet().removeIf(this::isAlreadyRendered);
+ // the template has already written type by the time the map
renders; a second one is a
+ // duplicate attribute, of which the browser keeps the first
+ constraints.keySet().removeIf(attributeName ->
+ "type".equalsIgnoreCase(attributeName) ||
isAlreadyRendered(attributeName));
if (!constraints.isEmpty()) {
addParameter("constraints", constraints);
}
@@ -1039,10 +1043,20 @@ public abstract class UIBean extends Component {
* {@code required} attribute the developer typed by hand as a dynamic
attribute still wins.
*/
private boolean isAlreadyRendered(String attributeName) {
- if (dynamicAttributes.containsKey(attributeName)) {
+ // HTML attribute names are ASCII case-insensitive
+ if (containsIgnoreCase(dynamicAttributes.keySet(), attributeName)) {
return true;
}
- return !"required".equals(attributeName) &&
getAttributes().containsKey(attributeName);
+ return !"required".equalsIgnoreCase(attributeName) &&
containsIgnoreCase(getAttributes().keySet(), attributeName);
+ }
+
+ private static boolean containsIgnoreCase(Set<String> names, String name) {
+ for (String candidate : names) {
+ if (candidate.equalsIgnoreCase(name)) {
+ return true;
+ }
+ }
+ return false;
}
/**
@@ -1110,13 +1124,13 @@ public abstract class UIBean extends Component {
/**
* The kind of HTML control this component renders, used to decide which
HTML5 constraint
- * attributes are legal on it. Defaults to {@link HtmlControlType#OTHER},
which supports no
+ * attributes are legal on it. Defaults to {@link
HtmlControlType#UNSUPPORTED}, which supports no
* constraints — so a component that does not override this emits none.
*
* @since 7.4.0
*/
protected HtmlControlType getControlType() {
- return HtmlControlType.OTHER;
+ return HtmlControlType.UNSUPPORTED;
}
protected void evaluateExtraParams() {
diff --git
a/core/src/test/java/org/apache/struts2/components/ConstraintAttributesTest.java
b/core/src/test/java/org/apache/struts2/components/ConstraintAttributesTest.java
index 6d70cd20e..6c7a59e73 100644
---
a/core/src/test/java/org/apache/struts2/components/ConstraintAttributesTest.java
+++
b/core/src/test/java/org/apache/struts2/components/ConstraintAttributesTest.java
@@ -36,6 +36,8 @@ public class ConstraintAttributesTest extends
AbstractUITagTest {
private FormTag form;
private String theme = "html5";
private String fieldName = "username";
+ private final Map<String, String> dynamicAttributes = new HashMap<>();
+ private String declaredMaxlength;
public void testNoConstraintsWhenTheConstantIsOff() throws Exception {
initDispatcherWith("false");
@@ -86,6 +88,44 @@ public class ConstraintAttributesTest extends
AbstractUITagTest {
assertFalse(constraints.containsKey("data-msg-field-visitor"));
}
+ /**
+ * HTML attribute names are ASCII case-insensitive, and the documented
rule is that the developer's
+ * own value always wins — including one typed as {@code MAXLENGTH}.
+ */
+ public void testADynamicAttributeSuppressesTheDerivedOneRegardlessOfCase()
throws Exception {
+ initDispatcherWith("true");
+ fieldName = "bio";
+ dynamicAttributes.put("MAXLENGTH", "5");
+
+ Map<String, String> constraints =
renderFieldAndReturnConstraints(null);
+
+ assertTrue("expected the derived maxlength to yield to the developer's
MAXLENGTH",
+ constraints == null || !constraints.containsKey("maxlength"));
+ }
+
+ /**
+ * {@code html5/text.ftl} has already emitted a hardcoded {@code type} by
the time the constraint
+ * map renders, so a {@code type} from a provider would be a duplicate
attribute the browser drops.
+ */
+ public void
testATypeOrADeclaredAttributeFromTheProviderIsDiscardedRegardlessOfCase()
throws Exception {
+ initDispatcherWith("true");
+
+ declaredMaxlength = "5";
+ TextFieldTag field = startField(null);
+ ((UIBean) field.getComponent()).setHtmlConstraintProvider((validators,
control, derivedFrom) ->
+ new java.util.LinkedHashMap<>(Map.of("Type", "email", "Maxlength",
"9", "required", "required")));
+ Map<String, Object> attributes = ((UIBean)
field.getComponent()).getAttributes();
+
+ finishField(field);
+
+ @SuppressWarnings("unchecked")
+ Map<String, String> constraints = (Map<String, String>)
attributes.get("constraints");
+ assertNotNull(constraints);
+ assertFalse("type must never reach the template, whatever its case",
constraints.containsKey("Type"));
+ assertFalse("a declared maxlength wins over a provider's Maxlength",
constraints.containsKey("Maxlength"));
+ assertEquals("required", constraints.get("required"));
+ }
+
/**
* Pins the hook to running after {@code evaluateExtraParams()}. A {@code
stringlength} validator on
* a control the browser treats as numeric must not emit {@code minlength}
at all — that attribute
@@ -185,6 +225,12 @@ public class ConstraintAttributesTest extends
AbstractUITagTest {
if (type != null) {
field.setType(type);
}
+ if (declaredMaxlength != null) {
+ field.setMaxlength(declaredMaxlength);
+ }
+ for (Map.Entry<String, String> dynamic : dynamicAttributes.entrySet())
{
+ field.setDynamicAttribute(null, dynamic.getKey(),
dynamic.getValue());
+ }
field.doStartTag();
return field;
}
diff --git
a/core/src/test/java/org/apache/struts2/components/ControlTypeTest.java
b/core/src/test/java/org/apache/struts2/components/ControlTypeTest.java
index 9c819b059..8f4cd026d 100644
--- a/core/src/test/java/org/apache/struts2/components/ControlTypeTest.java
+++ b/core/src/test/java/org/apache/struts2/components/ControlTypeTest.java
@@ -36,7 +36,7 @@ public class ControlTypeTest extends AbstractUITagTest {
public void testTextFieldFallsBackForAnUnknownType() {
TextField textField = new TextField(stack, request, response);
textField.addParameter("type", "supercolor");
- assertEquals(HtmlControlType.OTHER, textField.getControlType());
+ assertEquals(HtmlControlType.UNSUPPORTED, textField.getControlType());
}
public void testPasswordIsAlwaysPassword() {
diff --git
a/core/src/test/java/org/apache/struts2/components/EcmaScriptSafeRegexTest.java
b/core/src/test/java/org/apache/struts2/components/EcmaScriptSafeRegexTest.java
index 743ccc4c3..c76895f0a 100644
---
a/core/src/test/java/org/apache/struts2/components/EcmaScriptSafeRegexTest.java
+++
b/core/src/test/java/org/apache/struts2/components/EcmaScriptSafeRegexTest.java
@@ -43,6 +43,79 @@ public class EcmaScriptSafeRegexTest {
assertThat(EcmaScriptSafeRegex.isSafe("\\h+")).isFalse();
}
+ @Test
+ public void allowsAnEscapedHyphenOnlyInsideACharacterClass() {
+ // HTML compiles pattern with the Unicode flag, under which \- outside
a class is a
+ // SyntaxError and the whole attribute is ignored — silently, so the
allowlist must not admit it
+ assertThat(EcmaScriptSafeRegex.isSafe("[\\w\\-]+")).isTrue();
+ assertThat(EcmaScriptSafeRegex.isSafe("\\d+\\-\\d+")).isFalse();
+ }
+
+ @Test
+ public void rejectsAClosingBracketThatOpensAClass() {
+ // Java reads []a] as a class holding ] and a; the browser's
unicode-mode compiler throws
+ assertThat(EcmaScriptSafeRegex.isSafe("[]a]")).isFalse();
+ assertThat(EcmaScriptSafeRegex.isSafe("[^]a]")).isFalse();
+ }
+
+ @Test
+ public void rejectsALoneClosingBracketOrBraceOutsideAClass() {
+ // literals in Java, "lone quantifier brackets" in the browser
+ assertThat(EcmaScriptSafeRegex.isSafe("a]")).isFalse();
+ assertThat(EcmaScriptSafeRegex.isSafe("x}")).isFalse();
+ }
+
+ @Test
+ public void rejectsUnicodeSetsSyntaxCharactersUnescapedInsideAClass() {
+ // browsers compile pattern with the v flag, under which ( ) { } / |
and a hyphen that is not
+ // a range operator must be escaped inside a class — verified on node
24
+ assertThat(EcmaScriptSafeRegex.isSafe("[a-z/]")).isFalse();
+ assertThat(EcmaScriptSafeRegex.isSafe("^[+-]?\\d+$")).isFalse();
+ assertThat(EcmaScriptSafeRegex.isSafe("[-a]")).isFalse();
+ assertThat(EcmaScriptSafeRegex.isSafe("[a-]")).isFalse();
+ assertThat(EcmaScriptSafeRegex.isSafe("[(]")).isFalse();
+ assertThat(EcmaScriptSafeRegex.isSafe("[|]")).isFalse();
+ assertThat(EcmaScriptSafeRegex.isSafe("[{]")).isFalse();
+
assertThat(EcmaScriptSafeRegex.isSafe("[a-z0-9._%+-]+@[a-z]+")).isFalse();
+ // the negation marker is not a literal a range can start from
+ assertThat(EcmaScriptSafeRegex.isSafe("[^-a]")).isFalse();
+ assertThat(EcmaScriptSafeRegex.isSafe("^[^-,]+$")).isFalse();
+ }
+
+ @Test
+ public void rejectsStackedQuantifiers() {
+ // Java compiles a{2}{3}; the browser throws "nothing to repeat"
+ assertThat(EcmaScriptSafeRegex.isSafe("a{2}{3}")).isFalse();
+ assertThat(EcmaScriptSafeRegex.isSafe("[a-z]{2}{3}")).isFalse();
+ assertThat(EcmaScriptSafeRegex.isSafe("a{2}*")).isFalse();
+ }
+
+ @Test
+ public void acceptsLazyQuantifiersAndNegatedClasses() {
+ assertThat(EcmaScriptSafeRegex.isSafe("a+?")).isTrue();
+ assertThat(EcmaScriptSafeRegex.isSafe("a{2}?")).isTrue();
+ assertThat(EcmaScriptSafeRegex.isSafe("[^a-z]+")).isTrue();
+ assertThat(EcmaScriptSafeRegex.isSafe("[^\\d]")).isTrue();
+ assertThat(EcmaScriptSafeRegex.isSafe("(?:ab){2}")).isTrue();
+ }
+
+ @Test
+ public void rejectsDoubledPunctuatorsInsideAClass() {
+ assertThat(EcmaScriptSafeRegex.isSafe("[a..z]")).isFalse();
+ assertThat(EcmaScriptSafeRegex.isSafe("[!!]")).isFalse();
+ }
+
+ @Test
+ public void acceptsUnicodeSetsSafeClasses() {
+ assertThat(EcmaScriptSafeRegex.isSafe("[a-z]")).isTrue();
+ assertThat(EcmaScriptSafeRegex.isSafe("[a.z]")).isTrue();
+ assertThat(EcmaScriptSafeRegex.isSafe("[a\\-z]")).isTrue();
+ assertThat(EcmaScriptSafeRegex.isSafe("[\\d\\-]")).isTrue();
+
assertThat(EcmaScriptSafeRegex.isSafe("[a-z0-9._%+\\-]+@[a-z]+")).isTrue();
+ assertThat(EcmaScriptSafeRegex.isSafe("a/b")).isTrue();
+ assertThat(EcmaScriptSafeRegex.isSafe("a{2}")).isTrue();
+ }
+
@Test
public void rejectsPossessiveQuantifiers() {
assertThat(EcmaScriptSafeRegex.isSafe("\\d++")).isFalse();
diff --git
a/core/src/test/java/org/apache/struts2/components/HtmlControlTypeTest.java
b/core/src/test/java/org/apache/struts2/components/HtmlControlTypeTest.java
index 9fa862cdd..0adf678d5 100644
--- a/core/src/test/java/org/apache/struts2/components/HtmlControlTypeTest.java
+++ b/core/src/test/java/org/apache/struts2/components/HtmlControlTypeTest.java
@@ -38,17 +38,17 @@ public class HtmlControlTypeTest {
@Test
public void neverThrowsOnUnusableInput() {
-
assertThat(HtmlControlType.from(null)).isEqualTo(HtmlControlType.OTHER);
- assertThat(HtmlControlType.from("")).isEqualTo(HtmlControlType.OTHER);
- assertThat(HtmlControlType.from("
")).isEqualTo(HtmlControlType.OTHER);
-
assertThat(HtmlControlType.from("supercolor")).isEqualTo(HtmlControlType.OTHER);
+
assertThat(HtmlControlType.from(null)).isEqualTo(HtmlControlType.UNSUPPORTED);
+
assertThat(HtmlControlType.from("")).isEqualTo(HtmlControlType.UNSUPPORTED);
+ assertThat(HtmlControlType.from("
")).isEqualTo(HtmlControlType.UNSUPPORTED);
+
assertThat(HtmlControlType.from("supercolor")).isEqualTo(HtmlControlType.UNSUPPORTED);
}
@Test
public void otherSupportsNothing() {
- assertThat(HtmlControlType.OTHER.supportsPattern()).isFalse();
- assertThat(HtmlControlType.OTHER.supportsLength()).isFalse();
- assertThat(HtmlControlType.OTHER.supportsRange()).isFalse();
+ assertThat(HtmlControlType.UNSUPPORTED.supportsPattern()).isFalse();
+ assertThat(HtmlControlType.UNSUPPORTED.supportsLength()).isFalse();
+ assertThat(HtmlControlType.UNSUPPORTED.supportsRange()).isFalse();
}
@Test
diff --git
a/core/src/test/java/org/apache/struts2/components/StrutsHtmlConstraintProviderTest.java
b/core/src/test/java/org/apache/struts2/components/StrutsHtmlConstraintProviderTest.java
index 0cc97eab0..357accbc0 100644
---
a/core/src/test/java/org/apache/struts2/components/StrutsHtmlConstraintProviderTest.java
+++
b/core/src/test/java/org/apache/struts2/components/StrutsHtmlConstraintProviderTest.java
@@ -25,6 +25,7 @@ import
org.apache.struts2.validator.validators.DateRangeFieldValidator;
import org.apache.struts2.validator.validators.DoubleRangeFieldValidator;
import org.apache.struts2.validator.validators.EmailValidator;
import org.apache.struts2.validator.validators.IntRangeFieldValidator;
+import org.apache.struts2.validator.validators.RangeValidatorSupport;
import org.apache.struts2.validator.validators.RegexFieldValidator;
import org.apache.struts2.validator.validators.RequiredFieldValidator;
import org.apache.struts2.validator.validators.RequiredStringValidator;
@@ -32,6 +33,7 @@ import
org.apache.struts2.validator.validators.StringLengthFieldValidator;
import org.junit.Before;
import org.junit.Test;
+import java.math.BigDecimal;
import java.util.Date;
import java.util.List;
import java.util.Map;
@@ -289,7 +291,7 @@ public class StrutsHtmlConstraintProviderTest {
@Test
public void unknownControlGetsNothing() {
- assertThat(constraints(new RequiredFieldValidator(),
HtmlControlType.OTHER)).isEmpty();
+ assertThat(constraints(new RequiredFieldValidator(),
HtmlControlType.UNSUPPORTED)).isEmpty();
}
@Test
@@ -298,6 +300,82 @@ public class StrutsHtmlConstraintProviderTest {
assertThat(provider.constraintsFor(List.of(), HtmlControlType.TEXT,
null)).isEmpty();
}
+ @Test
+ public void messageIsNotEmittedOnAControlThatNeverSubmits() {
+ // s:label and unknown controls resolve to UNSUPPORTED; a message
there decorates an element the
+ // browser never validates and no script has a submitted value to
check it against
+ Validator validator = mock(Validator.class);
+ when(validator.getValidatorType()).thenReturn("requiredstring");
+ when(validator.getMessage(action)).thenReturn("required");
+
+ Map<String, String> result =
+ provider.constraintsFor(singletonList(validator),
HtmlControlType.UNSUPPORTED, action);
+
+ assertThat(result).isEmpty();
+ }
+
+ @Test
+ public void
messageIsNotEmittedForAValidatorTypeThatCannotBeAnAttributeName() {
+ // validator types are free-form in validators.xml; FreeMarker escapes
the value, not the name
+ Validator validator = mock(Validator.class);
+ when(validator.getValidatorType()).thenReturn("my type=\"x\"");
+ when(validator.getMessage(action)).thenReturn("nope");
+
+ Map<String, String> result =
+ provider.constraintsFor(singletonList(validator),
HtmlControlType.TEXT, action);
+
+ assertThat(result).isEmpty();
+ }
+
+ @Test
+ public void messageIsEmittedForADottedValidatorType() {
+ // legal both as a validators.xml type name and as a data-* attribute
suffix
+ Validator validator = mock(Validator.class);
+ when(validator.getValidatorType()).thenReturn("acme.required");
+ when(validator.getMessage(action)).thenReturn("needed");
+
+ Map<String, String> result =
+ provider.constraintsFor(singletonList(validator),
HtmlControlType.TEXT, action);
+
+ assertThat(result).containsEntry("data-msg-acme.required", "needed");
+ }
+
+ @Test
+ public void rangeOmitsAMaxThatIsNotANumber() {
+ // a date range on a control the developer declared numeric: min
already fails the integral
+ // guard, max must not fall through as Date.toString()
+ DateRangeFieldValidator validator = new DateRangeFieldValidator();
+ validator.setMin(new Date(0));
+ validator.setMax(new Date(1_000_000L));
+
+ assertThat(constraints(validator, HtmlControlType.NUMBER)).isEmpty();
+ }
+
+ @Test
+ public void doubleRangeOmitsANonFiniteMax() {
+ DoubleRangeFieldValidator nan = new DoubleRangeFieldValidator();
+ nan.setMaxInclusive(Double.NaN);
+ DoubleRangeFieldValidator infinite = new DoubleRangeFieldValidator();
+ infinite.setMaxInclusive(Double.POSITIVE_INFINITY);
+
+ assertThat(constraints(nan,
HtmlControlType.NUMBER)).doesNotContainKey("max");
+ assertThat(constraints(infinite,
HtmlControlType.NUMBER)).doesNotContainKey("max");
+ }
+
+ @Test
+ public void rangeOmitsAMinThatIsOnlyIntegralAfterRoundingThroughDouble() {
+ // 1.0000000000000000001 collapses to 1.0 as a double but renders with
its fraction, which
+ // would shift the HTML step base exactly like a fractional min
+ RangeValidatorSupport<BigDecimal> validator = new
RangeValidatorSupport<>(BigDecimal.class) {
+ };
+ validator.setMin(new BigDecimal("1.0000000000000000001"));
+ validator.setMax(new BigDecimal("10"));
+
+ assertThat(constraints(validator, HtmlControlType.NUMBER))
+ .doesNotContainKey("min")
+ .containsEntry("max", "10");
+ }
+
@Test
public void messageIsEmittedEvenForAValidatorThatContributesNoConstraint()
{
Validator validator = mock(Validator.class);
