Skip to content

Conversation

@XenoAmess
Copy link
Contributor

No description provided.

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.

For the first file change, Attributes class contains a getValue() method that will always return a String, so no cast is needed.

Second file, I've not approved the changes as it seems that the function signature allows an Object to be passed in for the value parameter, therefore the (String) cast here seems reasonable. Unless we wanted to change the interface itself, which would require more validation, I'd think this change might not make sense unless we can prove that this function should always expect a String for the value parameter. Again, further validation needed here, not confident on this change right now.

For the third file, lastModified is already defined as a long in the function signature, therefore no cast to long is needed.

For the last file, https://docs.oracle.com/javase/7/docs/api/javax/imageio/ImageIO.html#getImageWritersByFormatName(java.lang.String) returns a iterator of type ImageWriter and we can see that the next() function of this class, as defined at https://docs.oracle.com/javase/7/docs/api/java/util/Iterator.html#next(), will return the next ImageWriter element within the iterator, so there is no need to cast this to a ImageWriter type. If anything the only concern is that this code isn't checking that getImageWritersByFormatName("jpeg") isn't throwing a NoSuchElementException, but thats really not related to the issue at hand here.

Overall all changes look good minus the change on the java/meterpreter/meterpreter/src/main/java/com/metasploit/meterpreter/TLVPacket.java file.

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

@gwillcox-r7 done. please re-review. thanks.

@gwillcox-r7
Copy link
Contributor

@XenoAmess reviewed and approved, will merge this in now :)

@gwillcox-r7 gwillcox-r7 changed the title remove useless cast Remove useless casts in Java Meterpreter Feb 19, 2021
@gwillcox-r7 gwillcox-r7 merged commit 5370cba into rapid7:master Feb 19, 2021
@XenoAmess XenoAmess deleted the remove_useless_cast 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