Skip to content

Commit 87c230d

Browse files
author
Developer
committed
Fix ICAST_QUESTIONABLE_UNSIGNED_RIGHT_SHIFT in DataConverter
Fixed SpotBugs issue where unsigned right shift operations were being cast directly to byte without proper masking. Added 0xFF mask to ensure only the lower 8 bits are preserved when casting from int/long to byte. - Modified intToBytes() to mask with 0xFF after right shift - Modified longToBytes() to mask with 0xFF after right shift - Added comprehensive tests for negative values to verify fix This resolves potential issues with sign extension when casting the result of unsigned right shift operations to smaller types.
1 parent 2dca7f4 commit 87c230d

File tree

2 files changed

+73
-2
lines changed

2 files changed

+73
-2
lines changed

src/main/java/de/rub/nds/modifiablevariable/util/DataConverter.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ public static byte[] intToBytes(int value, int size) {
169169
int shift = 0;
170170
int finalPosition = size > Integer.BYTES ? size - Integer.BYTES : 0;
171171
for (int i = size - 1; i >= finalPosition; i--) {
172-
result[i] = (byte) (value >>> shift);
172+
result[i] = (byte) ((value >>> shift) & 0xFF);
173173
shift += 8;
174174
}
175175

@@ -192,7 +192,7 @@ public static byte[] longToBytes(long value, int size) {
192192
int shift = 0;
193193
int finalPosition = size > Long.BYTES ? size - Long.BYTES : 0;
194194
for (int i = size - 1; i >= finalPosition; i--) {
195-
result[i] = (byte) (value >>> shift);
195+
result[i] = (byte) ((value >>> shift) & 0xFF);
196196
shift += 8;
197197
}
198198

src/test/java/de/rub/nds/modifiablevariable/util/DataConverterTest.java

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,6 +1173,77 @@ void testIntegerReconversion() {
11731173
}
11741174
}
11751175

1176+
@Test
1177+
void testIntToBytesWithNegativeValues() {
1178+
// Test negative int values to ensure unsigned right shift is handled correctly
1179+
int negativeValue = -1;
1180+
byte[] result = DataConverter.intToBytes(negativeValue, 4);
1181+
assertArrayEquals(
1182+
new byte[] {(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF},
1183+
result,
1184+
"Negative -1 should produce all FF bytes");
1185+
1186+
// Test another negative value
1187+
negativeValue = -256; // 0xFFFFFF00
1188+
result = DataConverter.intToBytes(negativeValue, 4);
1189+
assertArrayEquals(
1190+
new byte[] {(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0x00},
1191+
result,
1192+
"Negative -256 should produce correct bytes");
1193+
1194+
// Test with size larger than int
1195+
negativeValue = -1;
1196+
result = DataConverter.intToBytes(negativeValue, 6);
1197+
assertArrayEquals(
1198+
new byte[] {0x00, 0x00, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF},
1199+
result,
1200+
"Negative -1 with size 6 should be padded with zeros");
1201+
}
1202+
1203+
@Test
1204+
void testLongToBytesWithNegativeValues() {
1205+
// Test negative long values to ensure unsigned right shift is handled correctly
1206+
long negativeValue = -1L;
1207+
byte[] result = DataConverter.longToBytes(negativeValue, 8);
1208+
assertArrayEquals(
1209+
new byte[] {
1210+
(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF,
1211+
(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF
1212+
},
1213+
result,
1214+
"Negative -1L should produce all FF bytes");
1215+
1216+
// Test another negative value
1217+
negativeValue = -256L; // 0xFFFFFFFFFFFFFF00
1218+
result = DataConverter.longToBytes(negativeValue, 8);
1219+
assertArrayEquals(
1220+
new byte[] {
1221+
(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF,
1222+
(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0x00
1223+
},
1224+
result,
1225+
"Negative -256L should produce correct bytes");
1226+
1227+
// Test with size larger than long
1228+
negativeValue = -1L;
1229+
result = DataConverter.longToBytes(negativeValue, 10);
1230+
assertArrayEquals(
1231+
new byte[] {
1232+
0x00,
1233+
0x00,
1234+
(byte) 0xFF,
1235+
(byte) 0xFF,
1236+
(byte) 0xFF,
1237+
(byte) 0xFF,
1238+
(byte) 0xFF,
1239+
(byte) 0xFF,
1240+
(byte) 0xFF,
1241+
(byte) 0xFF
1242+
},
1243+
result,
1244+
"Negative -1L with size 10 should be padded with zeros");
1245+
}
1246+
11761247
/** Test of indexOf method, of class DataConverter. */
11771248
@Test
11781249
void testIndexOf() {

0 commit comments

Comments
 (0)