This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git
The following commit(s) were added to refs/heads/master by this push:
new fc67697eb Range.readObject() does not re-assert the
comparator/ordering invariant. (#1686)
fc67697eb is described below
commit fc67697ebe7d8c4a6ad5b9344e3216522ec83113
Author: Gary Gregory <[email protected]>
AuthorDate: Wed Jun 3 09:05:25 2026 -0400
Range.readObject() does not re-assert the comparator/ordering invariant.
(#1686)
---
pom.xml | 8 +-
.../spotbugs-exclude-filter-java8-sb-4.8.6.xml | 242 +++++++++++++++++++++
src/main/java/org/apache/commons/lang3/Range.java | 3 +
.../apache/commons/lang3/RangeReadObjectTest.java | 21 ++
4 files changed, 272 insertions(+), 2 deletions(-)
diff --git a/pom.xml b/pom.xml
index 1ec4020d8..6242d65c9 100644
--- a/pom.xml
+++ b/pom.xml
@@ -174,6 +174,7 @@
<commons.jacoco.complexityRatio>0.91</commons.jacoco.complexityRatio>
<commons.text.version>1.15.0</commons.text.version>
<commons.io.version>2.22.0</commons.io.version>
+
<commons.spotbugs.filter>${commons.conf.dir}/spotbugs-exclude-filter.xml</commons.spotbugs.filter>
</properties>
<build>
<defaultGoal>clean verify apache-rat:check checkstyle:check japicmp:cmp
spotbugs:check pmd:check javadoc:javadoc</defaultGoal>
@@ -316,7 +317,7 @@
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<configuration>
-
<excludeFilterFile>${basedir}/src/conf/spotbugs-exclude-filter.xml</excludeFilterFile>
+ <excludeFilterFile>${commons.spotbugs.filter}</excludeFilterFile>
</configuration>
</plugin>
</plugins>
@@ -376,7 +377,7 @@
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<configuration>
-
<excludeFilterFile>${basedir}/src/conf/spotbugs-exclude-filter.xml</excludeFilterFile>
+ <excludeFilterFile>${commons.spotbugs.filter}</excludeFilterFile>
</configuration>
</plugin>
<plugin>
@@ -476,6 +477,9 @@
<activation>
<jdk>1.8</jdk>
</activation>
+ <properties>
+
<commons.spotbugs.filter>${commons.conf.dir}/spotbugs-exclude-filter-java8-sb-4.8.6.xml</commons.spotbugs.filter>
+ </properties>
<build>
<!-- Don't run javadoc:javadoc on Java 8 due to Java's incompatible
expectation of package-list vs element-list on remote links. -->
<defaultGoal>clean verify apache-rat:check checkstyle:check
japicmp:cmp spotbugs:check pmd:check</defaultGoal>
diff --git a/src/conf/spotbugs-exclude-filter-java8-sb-4.8.6.xml
b/src/conf/spotbugs-exclude-filter-java8-sb-4.8.6.xml
new file mode 100644
index 000000000..8c4cb6c3c
--- /dev/null
+++ b/src/conf/spotbugs-exclude-filter-java8-sb-4.8.6.xml
@@ -0,0 +1,242 @@
+<?xml version="1.0"?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ https://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<!--
+ This file contains some false positive bugs detected by findbugs. Their
+ false positive nature has been analyzed individually and they have been
+ put here to instruct findbugs it must ignore them.
+-->
+<FindBugsFilter
+ xmlns="https://github.com/spotbugs/filter/3.0.0";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0
https://raw.githubusercontent.com/spotbugs/spotbugs/3.1.0/spotbugs/etc/findbugsfilter.xsd";>
+
+ <!-- SpotBugs 4.8.6 suffers from
https://github.com/spotbugs/spotbugs/issues/2957 -->
+ <!-- MC_OVERRIDABLE_METHOD_CALL_IN_READ_OBJECT false positive in 4.8.4 -->
+ <Match>
+ <Class name="org.apache.commons.lang3.Range" />
+ <Bug pattern="MC_OVERRIDABLE_METHOD_CALL_IN_READ_OBJECT" />
+ </Match>
+
+ <!-- TODO Can any of these be done without breaking binary compatibility? -->
+ <Match>
+ <Class name="~.*" />
+ <Or>
+ <Bug pattern="EI_EXPOSE_REP" />
+ <Bug pattern="EI_EXPOSE_REP2" />
+ <Bug pattern="MS_EXPOSE_REP" />
+ </Or>
+ </Match>
+
+ <Match>
+ <!-- TODO ? -->
+ <Bug pattern="CT_CONSTRUCTOR_THROW" />
+ </Match>
+
+ <!-- TODO Can any of these be done without breaking binary compatibility? -->
+ <Match>
+ <Class name="org.apache.commons.lang3.reflect.FieldUtils" />
+ <Bug pattern="REFLF_REFLECTION_MAY_INCREASE_ACCESSIBILITY_OF_FIELD" />
+ </Match>
+
+ <!-- TODO Cannot be done without breaking binary compat. -->
+ <Match>
+ <Class name="org.apache.commons.lang3.tuple.MutablePair" />
+ <Bug pattern="PA_PUBLIC_PRIMITIVE_ATTRIBUTE" />
+ </Match>
+
+ <!-- TODO Cannot be done without breaking binary compat. -->
+ <Match>
+ <Class name="org.apache.commons.lang3.tuple.MutableTriple" />
+ <Bug pattern="PA_PUBLIC_PRIMITIVE_ATTRIBUTE" />
+ </Match>
+
+ <!-- https://github.com/spotbugs/spotbugs/issues/2710 -->
+ <Match>
+ <Class name="org.apache.commons.lang3.ThreadUtils$ThreadIdPredicate" />
+ <Bug pattern="CT_CONSTRUCTOR_THROW" />
+ </Match>
+
+ <Match>
+ <Class name="org.apache.commons.lang3.ArrayUtils" />
+ <Method name="addFirst" />
+ <Bug pattern="NP_LOAD_OF_KNOWN_NULL_VALUE" />
+ </Match>
+
+ <!-- Reason: Optimization to use == -->
+ <Match>
+ <Class name="org.apache.commons.lang3.BooleanUtils" />
+ <Or>
+ <Method name="toBoolean" />
+ <Method name="toBooleanObject" />
+ </Or>
+ <Bug pattern="ES_COMPARING_PARAMETER_STRING_WITH_EQ" />
+ </Match>
+ <Match>
+ <Class name="org.apache.commons.lang3.BooleanUtils" />
+ <Method name="toBoolean" />
+ <Bug pattern="RC_REF_COMPARISON_BAD_PRACTICE_BOOLEAN" />
+ </Match>
+
+ <!-- Reason: Behavior documented in javadoc -->
+ <Match>
+ <Class name="org.apache.commons.lang3.BooleanUtils" />
+ <Or>
+ <Method name="negate" />
+ <Method name="toBooleanObject" />
+ </Or>
+ <Bug pattern="NP_BOOLEAN_RETURN_NULL" />
+ </Match>
+
+ <!-- Reason: base class cannot be changed and field is properly checked
against null so behavior is OK -->
+ <Match>
+ <Class name="org.apache.commons.lang3.text.ExtendedMessageFormat" />
+ <Method name="applyPattern" />
+ <Bug pattern="UR_UNINIT_READ_CALLED_FROM_SUPER_CONSTRUCTOR" />
+ </Match>
+
+ <!-- Reason: Optimization to use == -->
+ <Match>
+ <Class name="org.apache.commons.lang3.StringUtils" />
+ <Or>
+ <Method name="indexOfDifference"/>
+ <Method name="compare"
params="java.lang.String,java.lang.String,boolean"/>
+ <Method name="compareIgnoreCase"
params="java.lang.String,java.lang.String,boolean"/>
+ </Or>
+ <Bug pattern="ES_COMPARING_PARAMETER_STRING_WITH_EQ" />
+ </Match>
+ <Match>
+ <Class name="org.apache.commons.lang3.Strings$CiStrings" />
+ <Method name="compare" params="java.lang.String,java.lang.String"/>
+ <Bug pattern="ES_COMPARING_PARAMETER_STRING_WITH_EQ" />
+ </Match>
+ <Match>
+ <Class name="org.apache.commons.lang3.Strings$CsStrings" />
+ <Method name="compare" params="java.lang.String,java.lang.String"/>
+ <Bug pattern="ES_COMPARING_PARAMETER_STRING_WITH_EQ" />
+ </Match>
+
+ <!-- Reason: Intended -->
+ <Match>
+ <Class name="org.apache.commons.lang3.math.NumberUtils" />
+ <Method name="createNumber"/>
+ <Bug pattern="SF_SWITCH_FALLTHROUGH" />
+ </Match>
+ <!-- Reason: Intended -->
+ <Match>
+ <Class name="org.apache.commons.lang3.time.DateUtils" />
+ <Method name="getFragment"/>
+ <Bug pattern="SF_SWITCH_FALLTHROUGH" />
+ </Match>
+ <!-- Reason: False positive -->
+ <Match>
+ <Class name="org.apache.commons.lang3.text.ExtendedMessageFormat" />
+ <Method name="applyPattern"/>
+ <Bug pattern="SF_SWITCH_FALLTHROUGH" />
+ </Match>
+
+ <!-- Reason: toProperString is lazily loaded -->
+ <Match>
+ <Class name="org.apache.commons.lang3.math.Fraction" />
+ <Field name="toProperString" />
+ <Bug pattern="SE_TRANSIENT_FIELD_NOT_RESTORED" />
+ </Match>
+
+ <!-- Reason: It does call super.clone(), but via a subsequent method -->
+ <Match>
+ <Class name="org.apache.commons.lang3.text.StrTokenizer" />
+ <Method name="clone"/>
+ <Bug pattern="CN_IDIOM_NO_SUPER_CALL" />
+ </Match>
+
+ <!-- Reason: FindBugs 2.0.2 used in maven-findbugs-plugin 2.5.2 seems to
have problems with detection of default cases
+ in switch statements. All the excluded methods have switch statements that
contain a default case. -->
+ <Match>
+ <Class name="org.apache.commons.lang3.math.NumberUtils"/>
+ <Method name="createNumber" />
+ <Bug pattern="SF_SWITCH_NO_DEFAULT" />
+ </Match>
+ <!-- Reason: FindBugs does not correctly recognize default branches in
switch statements without break statements.
+ See, e.g., the report at https://sourceforge.net/p/findbugs/bugs/1298 -->
+ <Match>
+ <Class name="org.apache.commons.lang3.time.FastDateParser"/>
+ <Or>
+ <Method name="getStrategy" />
+ <Method name="simpleQuote" params="java.lang.StringBuilder,
java.lang.String"/>
+ </Or>
+ <Bug pattern="SF_SWITCH_NO_DEFAULT" />
+ </Match>
+
+ <!-- Reason: FindBugs cannot correctly recognize default branches in switch
statements without break statements.
+ See, e.g., the report at https://sourceforge.net/p/findbugs/bugs/1298 -->
+ <Match>
+ <Class name="org.apache.commons.lang3.time.FastDatePrinter"/>
+ <Method name="appendFullDigits" params="java.lang.Appendable, int, int"/>
+ <Bug pattern="SF_SWITCH_NO_DEFAULT" />
+ </Match>
+
+ <!-- Reason: The fallthrough on the switch statement is intentional -->
+ <Match>
+ <Class name="org.apache.commons.lang3.time.FastDatePrinter"/>
+ <Method name="appendFullDigits" params="java.lang.Appendable, int, int"/>
+ <Bug pattern="SF_SWITCH_FALLTHROUGH" />
+ </Match>
+
+ <!-- Reason: Internal class that is used only as a key for an internal
FormatCache. For this reason we can
+ be sure, that equals will never be called with null or types other than
MultipartKey.
+ -->
+ <Match>
+ <Class name="org.apache.commons.lang3.time.FormatCache$MultipartKey" />
+ <Method name="equals" />
+ <Bug pattern="BC_EQUALS_METHOD_SHOULD_WORK_FOR_ALL_OBJECTS" />
+ </Match>
+ <Match>
+ <Class name="org.apache.commons.lang3.time.FormatCache$MultipartKey" />
+ <Method name="equals" />
+ <Bug pattern="NP_EQUALS_SHOULD_HANDLE_NULL_ARGUMENT" />
+ </Match>
+
+ <!-- Reason: toString() can return null! -->
+ <Match>
+ <Class name="org.apache.commons.lang3.compare.ObjectToStringComparator" />
+ <Method name="compare" />
+ <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE,
ES_COMPARING_STRINGS_WITH_EQ" />
+ </Match>
+
+ <!-- Reason: requireNonNull is supposed to take a nullable parameter,
+ whatever Spotbugs thinks of it. -->
+ <Match>
+ <Class name="org.apache.commons.lang3.function.Objects" />
+ <Method name="requireNonNull" />
+ <Bug pattern="NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE" />
+ </Match>
+ <!-- False positive https://github.com/spotbugs/spotbugs/issues/2957 to be
fixed in 4.9.0 -->
+ <Match>
+ <Class name="org.apache.commons.lang3.event.EventListenerSupport" />
+ <Method name="readObject" />
+ <Bug pattern="MC_OVERRIDABLE_METHOD_CALL_IN_READ_OBJECT" />
+ </Match>
+ <!-- Fundamental disagreement with SB: It should be OK to have predefined
instances and allow new instances. -->
+ <Match>
+ <Bug pattern="SING_SINGLETON_HAS_NONPRIVATE_CONSTRUCTOR" />
+ </Match>
+ <!-- Fundamental disagreement with SB: It should be OK to have predefined
instances and allow new instances. -->
+ <Match>
+ <Bug pattern="SING_SINGLETON_IMPLEMENTS_SERIALIZABLE" />
+ </Match>
+</FindBugsFilter>
diff --git a/src/main/java/org/apache/commons/lang3/Range.java
b/src/main/java/org/apache/commons/lang3/Range.java
index e0fca54d1..0d2165de8 100644
--- a/src/main/java/org/apache/commons/lang3/Range.java
+++ b/src/main/java/org/apache/commons/lang3/Range.java
@@ -559,6 +559,9 @@ private void readObject(final ObjectInputStream in) throws
IOException, ClassNot
if (comparator == null) {
throw new InvalidObjectException("comparator null");
}
+ if (comparator.compare(minimum, maximum) > 0) {
+ throw new InvalidObjectException("Range minimum is greater than
maximum under the comparator.");
+ }
}
/**
diff --git a/src/test/java/org/apache/commons/lang3/RangeReadObjectTest.java
b/src/test/java/org/apache/commons/lang3/RangeReadObjectTest.java
index be2866ff2..88188f629 100644
--- a/src/test/java/org/apache/commons/lang3/RangeReadObjectTest.java
+++ b/src/test/java/org/apache/commons/lang3/RangeReadObjectTest.java
@@ -20,6 +20,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
@@ -28,6 +29,8 @@
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
+import java.util.Collections;
+import java.util.Comparator;
import java.util.Objects;
import org.apache.commons.lang3.reflect.FieldUtils;
@@ -117,6 +120,23 @@ void testComparatorNullViaForgedStream() throws Exception {
assertThrows(InvalidObjectException.class, () -> deserialize(forged));
}
+ /**
+ * Forged stream: minimum=1, maximum=10, hashCode=hash(1,10) (all
legitimate), but comparator replaced with a reversed ordering. The hashCode
gate passes
+ * (comparator excluded from the hash) and the null gates pass (comparator
is non-null); the ordering invariant is the only one violated. The deserialized
+ * Range still reports endpoints [1,10] but {@code contains(5)} returns
false because it trusts the reversed comparator.
+ */
+ @Test
+ void testForgedReversedComparatorBreaksContains() throws Exception {
+ final Range<Integer> reference = Range.of(Integer.valueOf(1),
Integer.valueOf(10));
+ final Range<Integer> forged = Range.of(Integer.valueOf(1),
Integer.valueOf(10));
+ final Comparator<Integer> reversed = Collections.reverseOrder();
+ FieldUtils.writeDeclaredField(forged, "comparator", reversed, true);
+ assertThrows(InvalidObjectException.class, () ->
deserialize(SerializationUtils.serialize(forged)));
+ assertThrows(SerializationException.class, () ->
SerializationUtils.deserialize(SerializationUtils.serialize(forged)));
+ assertThrows(SerializationException.class, () ->
SerializationUtils.roundtrip(forged));
+ assertTrue(reference.contains(Integer.valueOf(5)));
+ }
+
/**
* Forged stream with {@code maximum == null}; symmetric to F-061b.
*/
@@ -152,4 +172,5 @@ void testRoundTripPreservesCorrectHashCode() throws
Exception {
assertEquals(range.hashCode(), roundtrip.hashCode(), "Round-trip
serialization must preserve the correct hashCode");
assertEquals(range, roundtrip);
}
+
}
