This replaces a bunch of methods attempting to read file bytes with one method.#727
This replaces a bunch of methods attempting to read file bytes with one method.#727
Conversation
method. They were not robust, especially in a Windows OS environment.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Check notice
Code scanning / CodeQL
Unread local variable Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix the problem, remove the unused local variable while preserving the side effect of calling getFileBytes. This means we should keep the method invocation but drop the assignment to resultBytes.
The best minimal change is within testGetFileBytes_NotRegular_NotReadable in src/test/java/org/apache/datasketches/common/TestUtilTest.java: replace byte[] resultBytes = getFileBytes(resPath, ""); with just getFileBytes(resPath, "");. This does not alter any test logic (the test still triggers the call that may throw a RuntimeException), but eliminates the unread local variable. No new imports, methods, or other definitions are required.
| @@ -57,7 +57,7 @@ | ||
| @Test | ||
| public void testGetFileBytes_NotRegular_NotReadable() throws IOException { | ||
| try { | ||
| byte[] resultBytes = getFileBytes(resPath, ""); | ||
| getFileBytes(resPath, ""); | ||
| } catch (RuntimeException e) { | ||
| System.out.println(e.toString()); | ||
| } |
| filePath = basePath.resolve(fileName); | ||
| Files.write(filePath, bytes); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException("System IO Error writing file: " + filePath.toString() + " " + e); |
Check warning
Code scanning / CodeQL
Dereferenced variable may be null Warning test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 12 hours ago
To fix the problem, we must ensure that the catch block does not dereference filePath when it might still be null. The behavior we want to preserve is: throw a RuntimeException that includes as much path information as is safely available, but never trigger a NullPointerException in the error handling itself.
The best minimal change is to avoid preinitializing filePath to null, move its declaration down so that it is definitely assigned before use when possible, and in the catch block handle the case where filePath was never set. Concretely:
- Replace
Path filePath = null;withPath filePath;and move it inside thetryblock, immediately before theresolvecall, so that the variable is only in scope within thetry/catchwhere it’s needed. - In the
catchblock, construct the error message defensively: iffilePathis non-null, usefilePath.toString(), otherwise fall back to a generic"unknown"or something based onbasePathandfileName(both are guaranteed non-null byObjects.requireNonNull).
Because filePath is used only in this method and no external contracts depend on its exact error message format, this change preserves functionality while removing the null-dereference risk. No new imports or helper methods are required.
| @@ -127,13 +127,13 @@ | ||
| Objects.requireNonNull(basePath, "input parameter 'Path basePath' cannot be null."); | ||
| Objects.requireNonNull(fileName, "input parameter 'String fileName' cannot be null."); | ||
| Objects.requireNonNull(bytes, "input parameter 'byte[] bytes' cannot be null."); | ||
| Path filePath = null; | ||
| try { | ||
| Files.createDirectories(basePath); //create the directory if it doesn't exist. | ||
| filePath = basePath.resolve(fileName); | ||
| final Path filePath = basePath.resolve(fileName); | ||
| Files.write(filePath, bytes); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException("System IO Error writing file: " + filePath.toString() + " " + e); | ||
| final String baseInfo = "basePath=" + basePath + ", fileName=" + fileName; | ||
| throw new RuntimeException("System IO Error writing file (" + baseInfo + "): " + e); | ||
| } | ||
| } | ||
| } |
1 participant
This replaces a bunch of methods attempting to read file bytes that were not robust, especially in a Windows OS environment.
This is a far simpler and more robust and modern solution that is only one method.