Add a check to StringUtils.repeat() for large length repeat value#362
Add a check to StringUtils.repeat() for large length repeat value#362TuranDev wants to merge 2 commits intoapache:masterfrom TuranDev:master
Conversation
| assertEquals(10000, str.length()); | ||
| assertTrue(StringUtils.containsOnly(str, 'a')); | ||
| try { | ||
| StringUtils.repeat("aaa",Integer.MAX_VALUE); |
There was a problem hiding this comment.
Missing space between parameters
| assertEquals(10000, str.length()); | ||
| assertTrue(StringUtils.containsOnly(str, 'a')); | ||
| try { | ||
| StringUtils.repeat("aaa",Integer.MAX_VALUE); |
There was a problem hiding this comment.
Missing space between parameters
| assertEquals(10000, str.length()); | ||
| assertTrue(StringUtils.containsOnly(str, 'a')); | ||
| try { | ||
| StringUtils.repeat("aaa",Integer.MAX_VALUE); |
There was a problem hiding this comment.
Missing space between parameters
| assertEquals(10000, str.length()); | ||
| assertTrue(StringUtils.containsOnly(str, 'a')); | ||
| try { | ||
| StringUtils.repeat("aaa",Integer.MAX_VALUE); |
There was a problem hiding this comment.
Missing space between parameters
| assertEquals(10000, str.length()); | ||
| assertTrue(StringUtils.containsOnly(str, 'a')); | ||
| try { | ||
| StringUtils.repeat("aaa",Integer.MAX_VALUE); |
There was a problem hiding this comment.
Missing space between parameters
| assertEquals(10000, str.length()); | ||
| assertTrue(StringUtils.containsOnly(str, 'a')); | ||
| try { | ||
| StringUtils.repeat("aaa",Integer.MAX_VALUE); |
There was a problem hiding this comment.
Missing space between parameters
| assertEquals(10000, str.length()); | ||
| assertTrue(StringUtils.containsOnly(str, 'a')); | ||
| try { | ||
| StringUtils.repeat("aaa",Integer.MAX_VALUE); |
There was a problem hiding this comment.
Missing space between parameters
| assertEquals(10000, str.length()); | ||
| assertTrue(StringUtils.containsOnly(str, 'a')); | ||
| try { | ||
| StringUtils.repeat("aaa",Integer.MAX_VALUE); |
There was a problem hiding this comment.
Missing space between parameters
| assertEquals(10000, str.length()); | ||
| assertTrue(StringUtils.containsOnly(str, 'a')); | ||
| try { | ||
| StringUtils.repeat("aaa",Integer.MAX_VALUE); |
There was a problem hiding this comment.
Missing space between parameters
| assertEquals(10000, str.length()); | ||
| assertTrue(StringUtils.containsOnly(str, 'a')); | ||
| try { | ||
| StringUtils.repeat("aaa",Integer.MAX_VALUE); |
|
I'm somewhat wary of simply comparing the values, as it's not always intuitive and doesn't save us much in the way of performance. Perhaps comparing it to Integer.MAX_VALUE is a bit more appropriate? |
What do you mean? |
https://github.com/apache/commons-lang/pull/362/files#diff-3f4c835875ddc8a211c0272e8be51394R6253 I would sooner do something like:
|
|
Other thing of note: I don't think that providing the length in the exception is terribly useful, as the developer would clearly have access to that information. Is there consensus on whether this is a needed feature? Is there a JIRA ticket open for this? I am having a hard time believing that multi-billion character Strings emanating from this method are common enough to warrant the overhead. |
|
It is convention for exception messages to provide the user with information on the cause of failure. In the same way an ArrayIndexOutOfBoundsException would provide the user with the length which caused it even though the "developer would clearly have access to that information." This provides the user with information on the offending value. With regards to overhead it is a single check at the start that doesn't add much overhead and prevents issues with both being too large and failing silently (with regards to the expected string not being what the users expects as a result). |
|
Can you think of any kind of use-case where a String over two-billion characters long is created, such that all other calls to this should check for it? |
|
Validation checks are validation checks, we can't begin using occurrence to dictate their place. As with any utility a set number of validation checks are performed before the actual brunt of the logic. |
4 participants
I have added a check to ensure that the resultant String from the repeat is not larger than the maximum allowed length.