This PR fixes a few alerts flagged up by LGTM.com. These are ones where I could quickly tell what was going on and what to do, but with your knowledge you would be able to make so much more of them.
The first commit is where we are constructing and executing a command line by building up a string. What will happen is that the Runtime class will then split it back up based on the whitespace. This is a bit dangerous if metadataDir contains any spaces, and it could be very dangerous if an attacker is able to set that variable because they could then execute arbitrary code by naming the file something like dirname; rm -rf /. Building the command line by passing an array of strings is much safer and means they will be passed to mkdir as-is.
The second commit fixes a place where we were comparing the boxed Long type using == which can sometimes do the right thing but often not. Using .equals or in this case Objects.equals will compare the values instead of whether they are the exact same java object.
The third commit closes a FileOutputStream after it has been written to. This means we don't leak a file handle and if will definitely be flushed before the file is read from in BFHDFSHandler.writeHDFS.
Most of the other alerts look correct and worth fixing, but I think it would be a lot easier for you to address these as you know what's going on in the codebase. Full disclosure, I do work at the company that makes LGTM.com. What this means is that I've raised an issue internally for the "potential input resource leak" which seems to have a few false positives on your project, so in the future the results will hopefully be even better.
This PR fixes a few alerts flagged up by LGTM.com. These are ones where I could quickly tell what was going on and what to do, but with your knowledge you would be able to make so much more of them.
The first commit is where we are constructing and executing a command line by building up a string. What will happen is that the
Runtimeclass will then split it back up based on the whitespace. This is a bit dangerous ifmetadataDircontains any spaces, and it could be very dangerous if an attacker is able to set that variable because they could then execute arbitrary code by naming the file something likedirname; rm -rf /. Building the command line by passing an array of strings is much safer and means they will be passed tomkdiras-is.The second commit fixes a place where we were comparing the boxed
Longtype using==which can sometimes do the right thing but often not. Using.equalsor in this caseObjects.equalswill compare the values instead of whether they are the exact same java object.The third commit closes a
FileOutputStreamafter it has been written to. This means we don't leak a file handle and if will definitely be flushed before the file is read from inBFHDFSHandler.writeHDFS.Most of the other alerts look correct and worth fixing, but I think it would be a lot easier for you to address these as you know what's going on in the codebase. Full disclosure, I do work at the company that makes LGTM.com. What this means is that I've raised an issue internally for the "potential input resource leak" which seems to have a few false positives on your project, so in the future the results will hopefully be even better.