Skip to content

Commit 9db44f5

Browse files
author
TLS-Attacker Developer
committed
Fix equals() symmetry issue in ModifiableLengthField
- Modified equals() method to ensure symmetry with ModifiableInteger - When comparing with another ModifiableLengthField, check both value and reference - When comparing with other ModifiableInteger subclasses, only check value - Added comprehensive tests to verify symmetry is maintained - Fixes SpotBugs issue EQ_OVERRIDING_EQUALS_NOT_SYMMETRIC
1 parent 2dca7f4 commit 9db44f5

File tree

2 files changed

+190
-7
lines changed

2 files changed

+190
-7
lines changed

src/main/java/de/rub/nds/modifiablevariable/length/ModifiableLengthField.java

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,15 +127,30 @@ public boolean equals(Object obj) {
127127
if (this == obj) {
128128
return true;
129129
}
130-
if (!(obj instanceof ModifiableLengthField that)) {
130+
if (obj == null) {
131131
return false;
132132
}
133-
// First check if the values are equal
134-
boolean valuesEqual =
135-
getValue() == null ? that.getValue() == null : getValue().equals(that.getValue());
136-
// Then check if they reference the same byte array
137-
boolean refsEqual = ref.equals(that.ref);
138-
return valuesEqual && refsEqual;
133+
134+
// For ModifiableLengthField instances, check both value and reference
135+
if (obj instanceof ModifiableLengthField that) {
136+
// First check if the values are equal
137+
boolean valuesEqual =
138+
getValue() == null
139+
? that.getValue() == null
140+
: getValue().equals(that.getValue());
141+
// Then check if they reference the same byte array
142+
boolean refsEqual = ref.equals(that.ref);
143+
return valuesEqual && refsEqual;
144+
}
145+
146+
// For other ModifiableInteger subclasses, only compare values for symmetry
147+
if (obj instanceof ModifiableInteger that) {
148+
return getValue() == null
149+
? that.getValue() == null
150+
: getValue().equals(that.getValue());
151+
}
152+
153+
return false;
139154
}
140155

141156
/**
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
/*
2+
* ModifiableVariable - A Variable Concept for Runtime Modifications
3+
*
4+
* Ruhr University Bochum, Paderborn University, Technology Innovation Institute, and Hackmanit GmbH
5+
*
6+
* Licensed under Apache License 2.0 http://www.apache.org/licenses/LICENSE-2.0
7+
*/
8+
package de.rub.nds.modifiablevariable.length;
9+
10+
import static org.junit.jupiter.api.Assertions.*;
11+
12+
import de.rub.nds.modifiablevariable.bytearray.ModifiableByteArray;
13+
import de.rub.nds.modifiablevariable.integer.ModifiableInteger;
14+
import org.junit.jupiter.api.Test;
15+
16+
class ModifiableLengthFieldSymmetryTest {
17+
18+
/**
19+
* Test that equals() is symmetric between ModifiableLengthField and ModifiableInteger. If
20+
* a.equals(b) is true, then b.equals(a) must also be true.
21+
*/
22+
@Test
23+
void testEqualsSymmetryWithModifiableInteger() {
24+
// Create a ModifiableByteArray with 4 bytes
25+
ModifiableByteArray array = new ModifiableByteArray();
26+
array.setOriginalValue(new byte[] {1, 2, 3, 4});
27+
28+
// Create a ModifiableLengthField that references this array
29+
ModifiableLengthField lengthField = new ModifiableLengthField(array);
30+
31+
// Create a ModifiableInteger with the same value (4)
32+
ModifiableInteger integerField = new ModifiableInteger(4);
33+
34+
// Test symmetry: both directions should be equal
35+
assertTrue(
36+
lengthField.equals(integerField),
37+
"ModifiableLengthField should equal ModifiableInteger with same value");
38+
assertTrue(
39+
integerField.equals(lengthField),
40+
"ModifiableInteger should equal ModifiableLengthField with same value");
41+
42+
// Test with different values
43+
ModifiableInteger differentInteger = new ModifiableInteger(5);
44+
assertFalse(
45+
lengthField.equals(differentInteger),
46+
"ModifiableLengthField should not equal ModifiableInteger with different value");
47+
assertFalse(
48+
differentInteger.equals(lengthField),
49+
"ModifiableInteger should not equal ModifiableLengthField with different value");
50+
}
51+
52+
/** Test that equals() is symmetric with null values */
53+
@Test
54+
void testEqualsSymmetryWithNullValues() {
55+
// Create a ModifiableByteArray without setting original value
56+
ModifiableByteArray array = new ModifiableByteArray();
57+
ModifiableLengthField lengthFieldNull = new ModifiableLengthField(array);
58+
59+
// Create a ModifiableInteger with null value
60+
ModifiableInteger integerNull = new ModifiableInteger((Integer) null);
61+
62+
// Both should have null values
63+
assertNull(lengthFieldNull.getValue());
64+
assertNull(integerNull.getValue());
65+
66+
// Test symmetry with null values
67+
assertTrue(
68+
lengthFieldNull.equals(integerNull),
69+
"ModifiableLengthField with null should equal ModifiableInteger with null");
70+
assertTrue(
71+
integerNull.equals(lengthFieldNull),
72+
"ModifiableInteger with null should equal ModifiableLengthField with null");
73+
}
74+
75+
/**
76+
* Test that ModifiableLengthField still checks reference equality when comparing with another
77+
* ModifiableLengthField
78+
*/
79+
@Test
80+
void testReferenceEqualityBetweenModifiableLengthFields() {
81+
// Create two different arrays with same length
82+
ModifiableByteArray array1 = new ModifiableByteArray();
83+
array1.setOriginalValue(new byte[] {1, 2, 3, 4});
84+
ModifiableByteArray array2 = new ModifiableByteArray();
85+
array2.setOriginalValue(new byte[] {5, 6, 7, 8});
86+
87+
ModifiableLengthField field1 = new ModifiableLengthField(array1);
88+
ModifiableLengthField field2 = new ModifiableLengthField(array2);
89+
90+
// Fields should NOT be equal because they reference different arrays
91+
assertFalse(
92+
field1.equals(field2),
93+
"ModifiableLengthFields with different references should not be equal");
94+
95+
// Create two fields with the same reference
96+
ModifiableLengthField field3 = new ModifiableLengthField(array1);
97+
ModifiableLengthField field4 = new ModifiableLengthField(array1);
98+
99+
// Fields should be equal because they reference the same array and have same value
100+
assertTrue(
101+
field3.equals(field4),
102+
"ModifiableLengthFields with same reference and value should be equal");
103+
}
104+
105+
/** Test transitivity: if a.equals(b) and b.equals(c), then a.equals(c) */
106+
@Test
107+
void testEqualsTransitivity() {
108+
// Create a ModifiableByteArray with 5 bytes
109+
ModifiableByteArray array = new ModifiableByteArray();
110+
array.setOriginalValue(new byte[] {1, 2, 3, 4, 5});
111+
112+
ModifiableLengthField lengthField = new ModifiableLengthField(array);
113+
ModifiableInteger integer1 = new ModifiableInteger(5);
114+
ModifiableInteger integer2 = new ModifiableInteger(5);
115+
116+
// Check all three are equal to each other
117+
assertTrue(lengthField.equals(integer1));
118+
assertTrue(integer1.equals(integer2));
119+
assertTrue(lengthField.equals(integer2));
120+
121+
// Check symmetry
122+
assertTrue(integer1.equals(lengthField));
123+
assertTrue(integer2.equals(integer1));
124+
assertTrue(integer2.equals(lengthField));
125+
}
126+
127+
/** Test that equals() correctly handles inheritance hierarchy */
128+
@Test
129+
void testEqualsWithSubclasses() {
130+
// Create a custom subclass of ModifiableInteger
131+
class CustomModifiableInteger extends ModifiableInteger {
132+
CustomModifiableInteger(Integer value) {
133+
super(value);
134+
}
135+
}
136+
137+
ModifiableByteArray array = new ModifiableByteArray();
138+
array.setOriginalValue(new byte[] {1, 2, 3});
139+
140+
ModifiableLengthField lengthField = new ModifiableLengthField(array);
141+
CustomModifiableInteger customInteger = new CustomModifiableInteger(3);
142+
143+
// Should still maintain symmetry with subclasses
144+
assertTrue(
145+
lengthField.equals(customInteger),
146+
"ModifiableLengthField should equal ModifiableInteger subclass with same value");
147+
assertTrue(
148+
customInteger.equals(lengthField),
149+
"ModifiableInteger subclass should equal ModifiableLengthField with same value");
150+
}
151+
152+
/** Test edge cases for equals() */
153+
@Test
154+
void testEqualsEdgeCases() {
155+
ModifiableByteArray array = new ModifiableByteArray();
156+
array.setOriginalValue(new byte[] {1, 2});
157+
ModifiableLengthField lengthField = new ModifiableLengthField(array);
158+
159+
// Test with non-ModifiableInteger objects
160+
assertFalse(lengthField.equals("2"), "Should not equal String");
161+
assertFalse(lengthField.equals(2), "Should not equal raw Integer");
162+
assertFalse(lengthField.equals(new Object()), "Should not equal Object");
163+
assertFalse(lengthField.equals(null), "Should not equal null");
164+
165+
// Test self-equality
166+
assertTrue(lengthField.equals(lengthField), "Should equal itself");
167+
}
168+
}

0 commit comments

Comments
 (0)