Skip to content

Commit cc14feb

Browse files
chhilchhil
committed
Fix ConcurrentModificationException in ISO components
Add thread-safe dump() operations to ISOMsg, TLVList, FSDMsg, ISODatasetField, SecureKeyBlock, SecureKeySpec, CryptographicServiceMessage, and SimpleMsg. Fixes race conditions where dump() iterates collections (Map.entrySet(), ArrayList) while other threads mutate them via set()/append()/addField(). Pattern applied: - dump() operations: create snapshot under lock, iterate snapshot - mutate operations: atomic synchronized blocks Includes concurrent tests demonstrating the fix with 2000 iterations per thread, 4 threads, and CountDownLatch coordination. Assisted-by: OpenClaw:minimax-coding-plan/MiniMax-M2.7
1 parent f581f39 commit cc14feb

14 files changed

Lines changed: 945 additions & 30 deletions

‎jpos/src/main/java/org/jpos/iso/ISODatasetField.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,11 @@ public void unpack(InputStream in) throws ISOException {
287287
public void dump(PrintStream p, String indent) {
288288
p.println(indent + "<" + XMLPackager.ISOFIELD_TAG + " " + XMLPackager.ID_ATTR + "=\"" + fieldNumber + "\" type=\"dataset\">");
289289
String innerIndent = indent + " ";
290-
for (Dataset dataset : datasets) {
290+
List<Dataset> snapshot;
291+
synchronized (datasets) {
292+
snapshot = new ArrayList<>(datasets);
293+
}
294+
for (Dataset dataset : snapshot) {
291295
p.println(innerIndent + "<dataset id=\"" + String.format("%02X", dataset.getIdentifier()) + "\" format=\"" + dataset.getFormat() + "\">");
292296
String datasetIndent = innerIndent + " ";
293297
for (DatasetElement element : dataset.getElements()) {

‎jpos/src/main/java/org/jpos/iso/ISOMsg.java‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,17 @@ public int getMaxField() {
198198
recalcMaxField();
199199
return maxField;
200200
}
201+
202+
/**
203+
* Returns a snapshot of field numbers as a new HashSet.
204+
* Safe for concurrent iteration during dump operations.
205+
* @return new HashSet containing current field numbers
206+
*/
207+
public Set<Integer> getFieldNumbers() {
208+
synchronized (fields) {
209+
return new HashSet<>(fields.keySet());
210+
}
211+
}
201212
private void recalcMaxField() {
202213
maxField = 0;
203214
for (Object obj : fields.keySet()) {
@@ -232,11 +243,13 @@ public ISOPackager getPackager () {
232243
*/
233244
public void set (ISOComponent c) throws ISOException {
234245
if (c != null) {
235-
Integer i = (Integer) c.getKey();
236-
fields.put (i, c);
237-
if (i > maxField)
238-
maxField = i;
239-
dirty = true;
246+
synchronized (fields) {
247+
Integer i = (Integer) c.getKey();
248+
fields.put (i, c);
249+
if (i > maxField)
250+
maxField = i;
251+
dirty = true;
252+
}
240253
}
241254
}
242255

@@ -654,7 +667,7 @@ public void dump (PrintStream p, String indent) {
654667
if (header instanceof Loggeable)
655668
((Loggeable) header).dump (p, newIndent);
656669

657-
for (int i : fields.keySet()) {
670+
for (int i : getFieldNumbers()) {
658671
//If you want the bitmap dumped in the log, change the condition from (i >= 0) to (i >= -1).
659672
if (i >= 0) {
660673
if ((c = (ISOComponent) fields.get(i)) != null)

‎jpos/src/main/java/org/jpos/security/CryptographicServiceMessage.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,9 @@ public void addField(String tag, String content) {
119119
Objects.requireNonNull(tag, "The tag is required");
120120
Objects.requireNonNull(content, "The content is required");
121121
tag = tag.toUpperCase();
122-
fields.put(tag, content);
122+
synchronized (fields) {
123+
fields.put(tag, content);
124+
}
123125
}
124126

125127
/**
@@ -191,7 +193,11 @@ public void dump (PrintStream p, String indent) {
191193
p.print(indent + "<csm");
192194
p.print(" class=\"" + getMCL() + "\"");
193195
p.println(">");
194-
for (String tag : fields.keySet()) {
196+
List<String> snapshot;
197+
synchronized (fields) {
198+
snapshot = new ArrayList<>(fields.keySet());
199+
}
200+
for (String tag : snapshot) {
195201
p.println(inner + "<field tag=\"" + tag + "\" value=\"" + getFieldContent(tag) + "\"/>");
196202
}
197203
p.println(indent + "</csm>");

‎jpos/src/main/java/org/jpos/security/SecureKeyBlock.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919
package org.jpos.security;
2020

2121
import java.io.PrintStream;
22+
import java.util.ArrayList;
2223
import java.util.Collections;
2324
import java.util.LinkedHashMap;
25+
import java.util.List;
2426
import java.util.Map;
2527
import java.util.Map.Entry;
2628
import org.jpos.iso.ISOUtil;
@@ -280,9 +282,13 @@ public void dump(PrintStream p, String indent) {
280282
p.println(inner2 + "<reserved>" + reserved + "</reserved>");
281283
p.println(inner + "</header>");
282284

283-
if (!optionalHeaders.isEmpty()) {
285+
List<Entry<String, String>> snapshot;
286+
synchronized (this) {
287+
snapshot = new ArrayList<>(optionalHeaders.entrySet());
288+
}
289+
if (!snapshot.isEmpty()) {
284290
p.println(inner + "<optional-header>");
285-
for (Entry<String, String> ent : optionalHeaders.entrySet())
291+
for (Entry<String, String> ent : snapshot)
286292
p.println(inner2 + "<entry id=\""+ ent.getKey() + "\" value=\""+ ent.getValue()+ "\"/>");
287293
p.println(inner + "</optional-header>");
288294
}

‎jpos/src/main/java/org/jpos/security/SecureKeySpec.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@
2424
import org.jpos.util.Loggeable;
2525

2626
import java.io.Serializable;
27+
import java.util.ArrayList;
2728
import java.util.LinkedHashMap;
29+
import java.util.List;
2830
import java.util.Map;
2931
import java.util.Map.Entry;
3032
import org.jpos.iso.ISOUtil;
@@ -511,7 +513,11 @@ public void dump(PrintStream p, String indent) {
511513
if (!optionalHeaders.isEmpty()) {
512514
p.println(inner + "<optional-header>");
513515
String inner2 = inner + " ";
514-
for (Entry<String, String> ent : optionalHeaders.entrySet())
516+
List<Entry<String, String>> snapshot;
517+
synchronized (optionalHeaders) {
518+
snapshot = new ArrayList<>(optionalHeaders.entrySet());
519+
}
520+
for (Entry<String, String> ent : snapshot)
515521
p.println(inner2 + "<entry id=\""+ ent.getKey() + "\" value=\""+ ent.getValue()+ "\"/>");
516522
p.println(inner + "</optional-header>");
517523
}

‎jpos/src/main/java/org/jpos/tlv/TLVList.java‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,9 @@ public void unpack(byte[] buf, int offset) throws IllegalArgumentException
216216
public void append(TLVMsg tlv) throws NullPointerException {
217217
Objects.requireNonNull(tlv, "TLV message cannot be null");
218218

219-
tags.add(tlv);
219+
synchronized (tags) {
220+
tags.add(tlv);
221+
}
220222
}
221223

222224
/**
@@ -250,20 +252,24 @@ public TLVList append(int tag, String value) throws IllegalArgumentException {
250252
* @param index number
251253
*/
252254
public void deleteByIndex(int index) {
253-
tags.remove(index);
255+
synchronized (tags) {
256+
tags.remove(index);
257+
}
254258
}
255259

256260
/**
257261
* Delete the specified TLV from the list by tag value
258262
* @param tag id
259263
*/
260264
public void deleteByTag(int tag) {
261-
List<TLVMsg> t = new ArrayList<>();
262-
for (TLVMsg tlv2 : tags) {
263-
if (tlv2.getTag() == tag)
264-
t.add(tlv2);
265+
synchronized (tags) {
266+
List<TLVMsg> t = new ArrayList<>();
267+
for (TLVMsg tlv2 : tags) {
268+
if (tlv2.getTag() == tag)
269+
t.add(tlv2);
270+
}
271+
tags.removeAll(t);
265272
}
266-
tags.removeAll(t);
267273
}
268274

269275
/**
@@ -536,7 +542,11 @@ public boolean hasTag(int tag) {
536542
public void dump(PrintStream p, String indent) {
537543
String inner = indent + " ";
538544
p.println(indent + "<tlvlist>");
539-
for (TLVMsg msg : getTags())
545+
List<TLVMsg> snapshot;
546+
synchronized (tags) {
547+
snapshot = new ArrayList<>(tags);
548+
}
549+
for (TLVMsg msg : snapshot)
540550
msg.dump(p, inner);
541551
p.println(indent + "</tlvlist>");
542552
}

‎jpos/src/main/java/org/jpos/util/FSDMsg.java‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@
2727
import java.io.PrintStream;
2828
import java.net.URL;
2929
import java.nio.charset.Charset;
30+
import java.util.ArrayList;
3031
import java.util.Arrays;
3132
import java.util.Collections;
3233
import java.util.HashMap;
3334
import java.util.HashSet;
3435
import java.util.LinkedHashMap;
36+
import java.util.List;
3537
import java.util.Map;
3638
import java.util.Map.Entry;
3739
import java.util.Objects;
@@ -652,10 +654,12 @@ protected String readField (InputStreamReader r, String fieldName, int len,
652654
* @param value the field value, or null to remove
653655
*/
654656
public void set (String name, String value) {
655-
if (value != null)
656-
fields.put (name, value);
657-
else
658-
fields.remove (name);
657+
synchronized (fields) {
658+
if (value != null)
659+
fields.put (name, value);
660+
else
661+
fields.remove (name);
662+
}
659663
}
660664
/**
661665
* Sets the binary header bytes for this message.
@@ -894,7 +898,11 @@ public void dump (PrintStream p, String indent) {
894898
if (header != null) {
895899
append (p, "header", getHexHeader(), inner);
896900
}
897-
for (String f :fields.keySet())
901+
List<String> snapshot;
902+
synchronized (fields) {
903+
snapshot = new ArrayList<>(fields.keySet());
904+
}
905+
for (String f : snapshot)
898906
append (p, f, fields.get (f), inner);
899907
p.println (indent + "</fsdmsg>");
900908
}

‎jpos/src/main/java/org/jpos/util/SimpleMsg.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@
2121
import java.io.ByteArrayOutputStream;
2222
import java.io.IOException;
2323
import java.io.OutputStream;
24-
25-
import org.jpos.iso.ISOUtil;
26-
2724
import java.io.PrintStream;
25+
import java.util.ArrayList;
2826
import java.util.Arrays;
2927
import java.util.Collection;
3028

29+
import org.jpos.iso.ISOUtil;
30+
3131
/**
3232
* <p>
3333
* A simple general purpose loggeable message.
@@ -90,7 +90,7 @@ public void dump(PrintStream p, String indent) {
9090
if (msgContent instanceof Object[])
9191
cl = Arrays.asList((Object[]) msgContent);
9292
else if (msgContent instanceof Collection)
93-
cl = (Collection) msgContent;
93+
cl = new ArrayList<>((Collection) msgContent);
9494
else if (msgContent instanceof Loggeable)
9595
cl = Arrays.asList(msgContent);
9696
else if (msgContent instanceof Throwable)

0 commit comments

Comments
 (0)