jakartaee / jakartaee/mail-api
Possible ConcurrentModificationException in jakarta.mail.internet.ParameterList
- Dominant language
- Java
- Stars
- 285
- Forks
- 109
- Avg merge
- 15h 19m
- Merged PRs (30d)
- 1
Description
**Describe the bug**
In the method `ParameterList::combineMultisegmentNames(boolean)` method, there is an iteration for the `multisegmentNames` set. There is also a statement on line 433 that removes items from the `multisegmentNames` set within the loop. If line 433 is invoked, then the next call to the next() method of the set iterator will throw a ConcurrentModificationException.
Although there is a comment on line 432 mentions that the set removes method on line 433 should not be called, it is found that there are some invalid inputs that could pass the checking on line 431 and invoke line 433 which causes ConcurrentModificationException from the next Iterator::next() call on line 408.
Here are more details about the possible bug.
In the constructor of ParameterList, there is a call to the `ParameterList::combineMultisegmentNames(boolean)` method on line 309. That parameter takes a random String object to process. If we pass in an invalid string `;'*0=$;*0=Sg';*1*=$`, it can pass all parsing and checking and get to line 309 and invoke the `ParameterList::combineMultisegmentNames(boolean)` method. At that moment, the first item in the `multisegmentNames` set will be an empty string, and the `slist` map will contain three key-value pairs as follows.
KeyValue
'*0$
*0Sg'
*1An object of ParameterList$Value
In the first iteration of the segment on line 417, as `name` is an empty string, thus the `sname` on line 418 will become `*0` which makes the Object v on line 419 becomes a String object of `$`. As the Object v is not an instance of ParameterList$Value, thus the conditional checking on line 424 will be false and jump to 438, this makes the `charset` variable stay null.
Then in the next segment iteration, the `sname` on line 418 will become `*1` which makes the Object v on line 419 becomes an instance of the ParameterList$Value. With the newest Object v, the conditional checking on line 424 is true and gets to the inner logic, but because the segment is already 1 for this iteration, it will jump to another conditional checking on line 431. Because the `charset` variable is still null, the condition is met and thus the `multisegmentNames` is modified and causes ConcurrentModificationException in the next call to Iterator::next() method on line 408 during the next `multisegmentNames` iteration.
**To Reproduce**
Here is a sample proof of concept code.
```java
import jakarta.mail.internet.ParameterList;
public class ProofOfConcept {
public static void main(String...args) throws Exception {
new ParameterList(";'*0=$;*0=Sg';*1*=$");
}
}
```
Just compile and run it will directly cause the ConcurrentModificationException.
**Expected behaviour**
ParseException should be thrown for invalid input instead of just a comment mentioning that it should not happen.
Contributor guide
Research direction
Start with ParameterList::combineMultisegmentNames(boolean), especially the iteration over multisegmentNames and the removal described around lines 408, 431, and 433. Run the provided ProofOfConcept with `;'*0=$;*0=Sg';*1*=$`; done means invalid input produces ParseException rather than ConcurrentModificationException.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- api
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 25/100