Add system property to cap memory usage in ResponseInputStream#149
Add system property to cap memory usage in ResponseInputStream#149tbelaire wants to merge 1 commit into
Conversation
Right now it will consume an unbounded amount of memory if the remote server sends a response with nonsense all on one line.
| public class ResponseInputStream { | ||
|
|
||
| private static final Integer INPUT_STREAM_BUFFER_MAX_SIZE = | ||
| Integer.parseInt(System.getProperty("org.eclipse.angus.mail.iap.inputStreamBufferMaxSize", "0")); |
There was a problem hiding this comment.
Use primitive.
Use Math.max(x, 0) to clamp value.
Trap numberformatexception and use default. PropUtils has some helper methods.
There was a problem hiding this comment.
It would be preferable to obtain that value from Session#properties instead of a System property, because System will apply to all instances of ResponseInputStream in the whole JVM.
There is only one reference to new ResponseInputStream that is happening in angus-mail/providers/imap/src/main/java/org/eclipse/angus/mail/iap/Protocol.java. It should be easy to add a new constructor in ResponseInputStream passing the Properties as an argument. In this way we can obtain that new parameter, and it opens the possibility to add more in the future.
| // need count-avail more bytes | ||
| ba.grow(Math.max(minIncrement, count + incrementSlop - avail)); | ||
| int amountToGrow = Math.max(minIncrement, count + incrementSlop - avail); | ||
| if (INPUT_STREAM_BUFFER_MAX_SIZE > 0 && buffer.length + amountToGrow > INPUT_STREAM_BUFFER_MAX_SIZE) { |
There was a problem hiding this comment.
Do an unsigned compare.
| if (INPUT_STREAM_BUFFER_MAX_SIZE > 0 && buffer.length + amountToGrow > INPUT_STREAM_BUFFER_MAX_SIZE) { | |
| if((buffer.length + amountToGrow) - INPUT_STREAM_BUFFER_MAX_SIZE > 0) { |
Or use Integer.compareUnsigned.
|
@tbelaire May I ask when this MR can be merged? |
|
@baoxinggit PR has to be updated per feedback. |
jmehrens
left a comment
jmehrens
left a comment
There was a problem hiding this comment.
No new changes received.
4 participants
Right now it will consume an unbounded amount of memory if the remote server sends a response with nonsense all on one line.
It could also use a fixed limit instead of making it configurable if that is preferable.