Skip to content

Conversation

@XenoAmess
Copy link
Contributor

No description provided.

@gwillcox-r7 gwillcox-r7 self-assigned this Feb 15, 2021
Copy link
Contributor

@gwillcox-r7 gwillcox-r7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First two files adjustments look good, they are just implementations of https://docs.oracle.com/javase/7/docs/api/java/io/ByteArrayOutputStream.html#toString() which we should have been using anyway. The way we were doing it before was roundabout by using https://docs.oracle.com/javase/7/docs/api/java/io/ByteArrayOutputStream.html#toByteArray() and then calling the toString() method on that which is just excessive and unnecessary.

The next change is a case of us using https://docs.oracle.com/javase/7/docs/api/java/lang/StringBuffer.html#append(String%20str) to build up a StringBuffer of strings, and then just calling StringBuffer.toString() instead of just appending the strings to one another. I'm not a huge fan of doing multiple dangling lines which all make up one command of input, however in this case the change seems worthwhile as it removes a bunch of extra unnecessary function calls and overhead in the code.

As for a last change, there is a subtle difference between the two, which is noted at https://www.java67.com/2014/08/difference-between-string-literal-and-new-String-object-Java.html. Specifically the change would mean that these strings are no longer being created in heap memory, but are now using the Java string pool, which is a cache of commonly used strings. This saves memory if a string is repeated multiple times across an application. In the event it is not repeated or is changed, the effect is the same as new String() as strings in Java are unmutable, therefore even though string literals like the ones proposed in this change will all point to the same object, only at the time when they change new objects be created.

So in a way this last change is actually a bit of a memory saver as well in addition to being more appropriate syntax.

Overall this looks good to land in my view.

@gwillcox-r7 gwillcox-r7 changed the title refine string handlings refine string handlings in Java Meterpreter Feb 15, 2021
@gwillcox-r7 gwillcox-r7 merged commit 85be5fd into rapid7:master Feb 15, 2021
@XenoAmess XenoAmess deleted the refine_string_handlings branch February 19, 2021 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants