Copilot commented on code in PR #817:
URL: https://github.com/apache/maven-wagon/pull/817#discussion_r2651731998
##########
wagon-providers/wagon-ssh-external/src/main/java/org/apache/maven/wagon/providers/ssh/external/ScpExternalWagon.java:
##########
@@ -229,7 +229,18 @@ private void executeScpCommand(Resource resource, File
localFile, boolean put)
}
Commandline cl = createBaseCommandLine(putty, scpExecutable,
privateKey);
- cl.setWorkingDirectory(localFile.getParentFile().getAbsolutePath());
+ File parentFile = localFile.getParentFile();
+ if (null == parentFile) {
+ try {
+ File abs = localFile.getAbsoluteFile();
+ parentFile = abs.getParentFile();
+ } catch (SecurityException e) {
+ fireTransferError(resource, e, put ? TransferEvent.REQUEST_PUT
: TransferEvent.REQUEST_GET);
+
+ throw new TransferFailedException("Error accessing absolute
path of " + localFile, e);
+ }
+ }
Review Comment:
This change introduces new behavior for handling relative file paths without
test coverage. Since the repository already has test files for
ScpExternalWagon, consider adding a test case that verifies the behavior when
localFile.getParentFile() returns null (e.g., when using a relative path like
"gluegen-2.6.0-sources.jar"). This would help ensure the fix works as intended
and prevent regressions.
##########
wagon-providers/wagon-ssh-external/src/main/java/org/apache/maven/wagon/providers/ssh/external/ScpExternalWagon.java:
##########
@@ -229,7 +229,18 @@ private void executeScpCommand(Resource resource, File
localFile, boolean put)
}
Commandline cl = createBaseCommandLine(putty, scpExecutable,
privateKey);
- cl.setWorkingDirectory(localFile.getParentFile().getAbsolutePath());
+ File parentFile = localFile.getParentFile();
+ if (null == parentFile) {
+ try {
+ File abs = localFile.getAbsoluteFile();
+ parentFile = abs.getParentFile();
+ } catch (SecurityException e) {
+ fireTransferError(resource, e, put ? TransferEvent.REQUEST_PUT
: TransferEvent.REQUEST_GET);
+
+ throw new TransferFailedException("Error accessing absolute
path of " + localFile, e);
+ }
Review Comment:
After obtaining the absolute file, there's still no null check on the second
call to getParentFile(). According to Java documentation,
getAbsoluteFile().getParentFile() can still return null for filesystem roots
(e.g., "/" on Unix or "C:\" on Windows). This would cause a
NullPointerException on line 243 when calling parentFile.getAbsolutePath().
Consider adding a null check after line 236 and throwing a
TransferFailedException with a descriptive message if parentFile is still null.
```suggestion
}
if (parentFile == null) {
TransferFailedException e =
new TransferFailedException("Cannot determine parent
directory of " + localFile);
fireTransferError(resource, e, put ?
TransferEvent.REQUEST_PUT : TransferEvent.REQUEST_GET);
throw e;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
