hv-0001/0002: DefaultValidationOrder groupList O(N²) 50x + addInheritedGroups diamond O(2^D) 52428x; count 654→656
This commit is contained in:
parent
31db695abc
commit
347cb57f09
2 changed files with 160 additions and 0 deletions
|
|
@ -0,0 +1,91 @@
|
|||
# UNDF: (pending)
|
||||
# hv-0001: DefaultValidationOrder.insertGroup — groupList.contains() O(N) list scan
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: O(N) List.contains() dedup in validation group ordering
|
||||
|
||||
| Field | Value |
|
||||
|--------------|-------|
|
||||
| ID | hv-0001 |
|
||||
| Severity | MEDIUM |
|
||||
| Ecosystem | hibernate-validator |
|
||||
| Package | hibernate-validator |
|
||||
| File | `engine/src/main/java/org/hibernate/validator/internal/engine/groups/DefaultValidationOrder.java` |
|
||||
| Lines | 57–65 |
|
||||
| Complexity | O(N) per call; O(N²) for N groups |
|
||||
| Hot path | Called per validation group during `ValidatorImpl.validate()` |
|
||||
|
||||
## Defect
|
||||
|
||||
`DefaultValidationOrder.insertGroup` uses `List<Group>.contains()` to deduplicate groups:
|
||||
|
||||
```java
|
||||
// DefaultValidationOrder.java:57-65 (DEFECT)
|
||||
private List<Group> groupList; // initialized with new ArrayList<>(5)
|
||||
|
||||
public void insertGroup(Group group) {
|
||||
if ( groupList == null ) {
|
||||
groupList = new ArrayList<>( 5 );
|
||||
}
|
||||
if ( !groupList.contains( group ) ) { // O(N) linear scan
|
||||
groupList.add( group );
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Also in `ensureDefaultGroupSequenceIsExpandable` (line 119):
|
||||
```java
|
||||
int index = groupList.indexOf( group ); // O(N) linear scan
|
||||
```
|
||||
|
||||
And `ValidationOrderGenerator.addGroups` (lines 166-167):
|
||||
```java
|
||||
if ( resolvedGroupSequence.contains( tmpGroup ) // O(N) scan #1
|
||||
&& resolvedGroupSequence.indexOf( tmpGroup ) < resolvedGroupSequence.size() - 1 ) { // O(N) scan #2
|
||||
```
|
||||
|
||||
Two O(N) scans per group in `addGroups` — O(N²) total.
|
||||
|
||||
## Fix
|
||||
|
||||
Replace `List<Group>` with a `LinkedHashSet<Group>` for O(1) membership and preserved insertion order:
|
||||
|
||||
```java
|
||||
// AFTER — O(1) per operation
|
||||
private Set<Group> groupList; // LinkedHashSet preserves insertion order
|
||||
|
||||
public void insertGroup(Group group) {
|
||||
if ( groupList == null ) {
|
||||
groupList = new LinkedHashSet<>( 5 );
|
||||
}
|
||||
groupList.add( group ); // O(1) — Set.add() deduplicates automatically
|
||||
}
|
||||
```
|
||||
|
||||
For `ensureDefaultGroupSequenceIsExpandable`, build an index map:
|
||||
```java
|
||||
Map<Group, Integer> groupIndex = new LinkedHashMap<>();
|
||||
for (int i = 0; i < sequenceGroups.size(); i++) groupIndex.put(sequenceGroups.get(i), i);
|
||||
int index = groupIndex.getOrDefault(group, -1); // O(1)
|
||||
```
|
||||
|
||||
For `addGroups`, build a position index before the loop:
|
||||
```java
|
||||
Map<Group, Integer> positionMap = new LinkedHashMap<>();
|
||||
for (int i = 0; i < resolvedGroupSequence.size(); i++) positionMap.put(resolvedGroupSequence.get(i), i);
|
||||
for (Group tmpGroup : groups) {
|
||||
Integer pos = positionMap.get(tmpGroup);
|
||||
if (pos != null && pos < resolvedGroupSequence.size() - 1) throw LOG.getUnableToExpandGroupSequenceException();
|
||||
resolvedGroupSequence.add(tmpGroup);
|
||||
positionMap.put(tmpGroup, resolvedGroupSequence.size() - 1);
|
||||
}
|
||||
```
|
||||
|
||||
## Speedup
|
||||
|
||||
| Groups (N) | Before (scans) | After (scans) | Speedup |
|
||||
|------------|----------------|---------------|---------|
|
||||
| 10 | 55 | 10 | 5.5× |
|
||||
| 50 | 1,275 | 50 | 25.5× |
|
||||
| 100 | 5,050 | 100 | 50.5× |
|
||||
|
||||
Growth before: O(N²). Growth after: O(N).
|
||||
|
|
@ -0,0 +1,69 @@
|
|||
# UNDF: (pending)
|
||||
# hv-0002: Sequence.addInheritedGroups — O(2^D) diamond re-traversal in group inheritance expansion
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: O(2^D) recursive diamond re-traversal in validation group hierarchy
|
||||
|
||||
| Field | Value |
|
||||
|--------------|-------|
|
||||
| ID | hv-0002 |
|
||||
| Severity | HIGH |
|
||||
| Ecosystem | hibernate-validator |
|
||||
| Package | hibernate-validator |
|
||||
| File | `engine/src/main/java/org/hibernate/validator/internal/engine/groups/Sequence.java` |
|
||||
| Lines | 130–139 |
|
||||
| Complexity | O(2^D) on diamond group inheritance hierarchies |
|
||||
| Hot path | Called during validation order initialization for @GroupSequence |
|
||||
|
||||
## Defect
|
||||
|
||||
`Sequence.addInheritedGroups` recursively expands group interface hierarchies. Although it uses
|
||||
`Set<Group> expandedGroups` to track visited groups, the recursion guard is MISSING — the
|
||||
method recurses unconditionally even after `expandedGroups.add(g)` returns `false` (indicating
|
||||
the group was already visited):
|
||||
|
||||
```java
|
||||
// Sequence.java:130-139 (DEFECT)
|
||||
private void addInheritedGroups(Group group, Set<Group> expandedGroups) {
|
||||
for ( Class<?> inheritedGroup : group.getDefiningClass().getInterfaces() ) {
|
||||
if ( isGroupSequence( inheritedGroup ) ) { throw LOG.getSequenceDefinitionsNotAllowedException(); }
|
||||
Group g = new Group( inheritedGroup );
|
||||
expandedGroups.add( g ); // adds to set — returns false if already present
|
||||
addInheritedGroups( g, expandedGroups ); // DEFECT: recurses unconditionally, ignores add() return value
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
On a diamond group hierarchy (interface D extends A and B; interface A extends BaseGroup; interface
|
||||
B extends BaseGroup; class C implements D), BaseGroup is traversed twice:
|
||||
- Via D→A→BaseGroup: `expandedGroups.add(BaseGroup)` succeeds → recurse into BaseGroup
|
||||
- Via D→B→BaseGroup: `expandedGroups.add(BaseGroup)` fails (already present) → BUT STILL RECURSES into BaseGroup!
|
||||
|
||||
At diamond depth D, BaseGroup is visited 2^D times.
|
||||
|
||||
## Fix
|
||||
|
||||
Guard the recursion with the return value of `expandedGroups.add(g)`:
|
||||
|
||||
```java
|
||||
// AFTER — O(N+E) where N=groups, E=inheritance edges
|
||||
private void addInheritedGroups(Group group, Set<Group> expandedGroups) {
|
||||
for ( Class<?> inheritedGroup : group.getDefiningClass().getInterfaces() ) {
|
||||
if ( isGroupSequence( inheritedGroup ) ) { throw LOG.getSequenceDefinitionsNotAllowedException(); }
|
||||
Group g = new Group( inheritedGroup );
|
||||
if ( expandedGroups.add( g ) ) { // only recurse if newly added
|
||||
addInheritedGroups( g, expandedGroups );
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Speedup
|
||||
|
||||
| Diamond depth (D) | Before (visits) | After (visits) | Speedup |
|
||||
|------------------|----------------|----------------|---------|
|
||||
| 5 | 31 | 5 | 6× |
|
||||
| 10 | 1,023 | 10 | 102× |
|
||||
| 15 | 32,767 | 15 | 2,184× |
|
||||
| 20 | 1,048,575 | 20 | 52,428× |
|
||||
|
||||
Growth before: O(2^D). Growth after: O(D).
|
||||
Loading…
Add table
Add a link
Reference in a new issue