Bug: NullPointerException from AntSSH scp task

[pjdarton]

The Ant "scp" task works from a Windows machine but the Dovetail one does not.

I'm using Windows 7, running Ant 1.9.4, using Java 1.8, with the contents of dovetail-ant-ssh.0.1.1.zip\lib dropped into Ant's lib folder.
I ran the following ant code:
...
<ssh:session
host="localhost" port="10022"
username="peter"
password="something"
trust="true">
<ssh:sftp action="mkdir" dir="/home/peter/jbs" ignoreError="yes" />
<ssh:scp localFile="scripts.tar.gz" remoteToFile="/home/peter/jbs/scripts.tar.new" />
<ssh:sftp action="rename" file="/home/peter/jbs/scripts.tar.new" tofile="/home/peter/jbs/scripts.tar.gz" />
</ssh:session>
...
and got the following error:
[ssh:session] Connected to ssh-agent
[ssh:session] Connecting to localhost:10022
[ssh:sftp] mkdir: "/home/peter/jbs"

BUILD FAILED
C:\Dev\RCommon\JBS\build.xml:39: java.lang.NullPointerException
at com.dovetail.ant.ssh.ScpToMessage.getLocalFileMode(ScpToMessage.java:506)
at com.dovetail.ant.ssh.ScpToMessage.sendFileToRemote(ScpToMessage.java:371)
at com.dovetail.ant.ssh.ScpToMessage.doSingleTransfer(ScpToMessage.java:282)
at com.dovetail.ant.ssh.ScpToMessage.execute(ScpToMessage.java:258)
at com.dovetail.ant.ssh.Scp.upload(Scp.java:401)
at com.dovetail.ant.ssh.Scp.execute(Scp.java:268)
at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:292)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:95)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:55)
at java.lang.reflect.Method.invoke(Method.java:508)
at org.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
at org.apache.tools.ant.Task.perform(Task.java:348)
at com.dovetail.ant.ssh.SSHSession.execute(SSHSession.java:270)
at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:292)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:95)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:55)
at java.lang.reflect.Method.invoke(Method.java:508)
at org.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
at org.apache.tools.ant.Task.perform(Task.java:348)
at org.apache.tools.ant.Target.execute(Target.java:435)
at org.apache.tools.ant.Target.performTasks(Target.java:456)
at org.apache.tools.ant.Project.executeSortedTargets(Project.java:1393)
at org.apache.tools.ant.Project.executeTarget(Project.java:1364)
at org.apache.tools.ant.helper.DefaultExecutor.executeTargets(DefaultExecutor.java:41)
at org.apache.tools.ant.Project.executeTargets(Project.java:1248)
at org.apache.tools.ant.Main.runBuild(Main.java:851)
at org.apache.tools.ant.Main.startAnt(Main.java:235)
at org.apache.tools.ant.launch.Launcher.run(Launcher.java:280)
at org.apache.tools.ant.launch.Launcher.main(Launcher.java:109)

Total time: 1 second

Line 39 on my ant build.xml file is the ssh:scp line.
If I replace the call to ssh:scp with a "normal" scp (and add in host/username/password/port information) ...
<scp localFile="scripts.tar.gz" port="10022" password="something" trust="true" remoteToFile="peter@localhost:/home/peter/jbs/scripts.tar.new" />
...then it works fine.
i.e. the "standard" ant scp task copes when run from a Windows machine but the com.dovetail.ant.ssh version does not.

Looking at the source code, dovetail-ant-ssh.0.1.1.zip\lib\dovetail-ant-ssh.0.1.1.src.zip\com\dovetail\ant\ssh\ScpToMessage.java, line 506 is in method getLocalFileMode, which is calling Files.getFileAttributeView, passing the file's Path and PosixFileAttributeView.class, and then immediately de-referencing the result without first checking that the result isn't null.
According to the Javadocs ( https://docs.oracle.com/javase/7/docs/a ... Option...) ) the result from getFileAttributeView can be null, so the caller must check for this (and getLocalFileMode should return null in this case) before calling readAttributes() - this code doesn't, which is why it can crash like this.

Either the code should check for null, or perhaps use getPosixFilePermissions(...) instead of getFileAttributeView(...).
Note: The way the code looks (and the suspicious trailing whitespace on that line), I do wonder if it was originally meant to call getPosixFilePermissions and then changed to call getPosixFilePermissions instead - I'd suggest that, before changing it to call getPosixFilePermissions, you check the source code history as it's possible there was a good reason why it's calling getFileAttributeView instead of getPosixFilePermissions.

Regards,
Peter

PS. If you fix this, I'd be willing to test it (pre-release) if that'd help. Until that time, I have a workaround (to use scp instead of ssh:scp).

[dovetail]

Thank you for reporting this bug.
Here's a corrected version that you can test: https://dovetail.com/downloads/antssh/d ... .0.1.2.zip

Please let us know if this fixes your problems, or if you have others.

[pjdarton]

Test failed - no improvement, and no change in error.

I took a look at the modified code and I can see why - the null check that was added in this version is too late - the NullPointerException happens just before the null check that was added, on the very first line here (which is line 506 from ScpToMessage.java):

Code: Select all

			Set<PosixFilePermission> spfp = Files.getFileAttributeView(fp, PosixFileAttributeView.class).readAttributes().permissions();
			if (spfp == null) {
				return null;  // Posix file attributes are not available.
			}

The reason this didn't fix the bug is because it's not spfp that is potentially null - it's the result from calling Files.getFileAttributeView(fp, PosixFileAttributeView.class) that can be null.
When Files.getFileAttributeView(...) returns null then you can't call the readAttributes() method on the result (as the result is null), so you get a NullPointerException from the ".readAttributes()" code.
We need to call Files.getFileAttributeView(fp, PosixFileAttributeView.class), store that in a local variable and check that before we go on to call .readAttributes().permissions();
e.g.

Code: Select all

			PosixFileAttributeView pfav = Files.getFileAttributeView(fp, PosixFileAttributeView.class);
			if (pfav == null) {
				return null; // Posix file attributes are not available.
			}
			Set<PosixFilePermission> spfp = pfav.readAttributes().permissions();
			for (PosixFilePermission pfp : spfp) {
				...

FYI readAttributes() permissions() are not documented as ever returning null so we should simply need the null check on the results of Files.getFileAttributeView(...) (which says that it returns "a file attribute view of the specified type, or null if the attribute view type is not available"). i.e. we shouldn't need to check if spfp is null, we should get away with only checking if pfav is null.

[dovetail]

I apologize for not paying attention in my first attempt.
Try the updated download at the same link.
If this doesn't correct it, I'll bit the bullet and fire up Windoze and try to reproduce the problem myself

[pjdarton]

Well, that's fixed the original exception ... but it's now blowing up later on with another NullPointerException:

Code: Select all

java.lang.NullPointerException
        at com.dovetail.ant.ssh.ScpToMessage.sendFileToRemote(ScpToMessage.java:371)
        at com.dovetail.ant.ssh.ScpToMessage.doSingleTransfer(ScpToMessage.java:282)
        at com.dovetail.ant.ssh.ScpToMessage.execute(ScpToMessage.java:258)
        at com.dovetail.ant.ssh.Scp.upload(Scp.java:401)
        at com.dovetail.ant.ssh.Scp.execute(Scp.java:268)
        ...

This is now failing in the following conditional expression (ScpToMessage.java lines 369 to 371):

Code: Select all

        Integer permissions = hasExplicitFileMode()
        					? getFileMode()
        					: getLocalFileMode(localFile);

It's failing because there's some int<->Integer boxing going on and Java's trying to evaluate boolean:int?int (and then turn the resulting int into a non-null Integer) because getFileMode() returns int. I'd guess that what was intended was to have it turn getFileMode()'s answer from an int into an Integer and then evaluate boolean:Integer?Integer, but the Java compiler's choice of type was dictated by the first half of the conditional rather than the second.
I'd suggest that you try changing that code to:

Code: Select all

        Integer permissions = hasExplicitFileMode()
        					? Integer.valueOf(getFileMode())
        					: getLocalFileMode(localFile);

then Java would realize that it's meant to be dealing with (potentially null) Integers on both sides and it won't turn anything into an int.

I would also recommend that you tell your IDE to warn you about boxing (where Java automatically swaps between primitive types and their nullable Object basic types) and then take a look at everywhere in the code that then shows up, as there may be more potential NPEs from that. Personally, I'm not a big fan of automatic boxing/unboxing because the NullPointerExceptions you get are non-obvious (the code looks like it can't throw a NPE) and I prefer explicit calls to e.g. Integer.valueOf(int) and Integer's .intValue() method. However I learned Java long before automatic boxing/unboxing was introduced so I view it all as "dark sorcery" and not to be trusted

I think you're right that it's now time to fire up a Windows machine and try this out yourself - I'd guess that this code hasn't been run on Windows before, so it's quite likely that this won't be the last issue that'll stop it working.

[dovetail]

I reproduced the problem on Windows.
It only applies to scp task, which I'll confess that we rarely use since our motivation for doing this was to have sftp ant tasks that work well with Co:Z SFTP on z/OS.

In your example for instance, I think that it is would be better to use an sftp put, since you already have an SFTP session started.

In any case, I fixed problem and your test runs now for me on Windows. Please download the same link:

https://dovetail.com/downloads/antssh/d ... .0.1.2.zip

[pjdarton]

Well, the ssh:scp step worked ... but it seems to have closed down the ssh:session so that the subsequent sftp rename operation failed:

Code: Select all

[ssh:session] Connected to ssh-agent
[ssh:session] Connecting to somehostname:22
 [ssh:sftp] mkdir: "sometargetdir"
  [ssh:scp]   Sending: scripts.tar.gz : 45331606
 [ssh:sftp] rename: "sometargetdir/myremotefile.tar.gz.new", "sometargetdir/myremotefile.tar.gz"

BUILD FAILED
C:\Dev\JBS\build.xml:53: The following error occurred while executing this line:
C:\Dev\JBS\build.xml:116: com.jcraft.jsch.JSchException: Could not rename "sometargetdir/myremotefile.tar.gz.new", "sometargetdir/myremotefile.tar.gz" - com.jcraft.jsch.JSchException: session is down
        at com.dovetail.ant.ssh.Sftp.handleSftpException(Sftp.java:592)
        at com.dovetail.ant.ssh.Sftp.executeSftpCommand(Sftp.java:534)
        at com.dovetail.ant.ssh.Sftp.executeSftpCommand(Sftp.java:506)
        at com.dovetail.ant.ssh.Sftp.rename(Sftp.java:244)
        at ...

where the ant code being run was of the form:

Code: Select all

<ssh:session
	host="somehostname" port="22"
	username="myusername"
	password="somepassword"
	trust="true">
	<ssh:sftp action="mkdir" dir="sometargetdir" ignoreError="yes" />
	<ssh:scp localFile="mylocalfile.tar.gz" remoteToFile="sometargetdir/myremotefile.tar.gz.new" />
	<ssh:sftp action="rename" file="sometargetdir/myremotefile.tar.gz.new" tofile="sometargetdir/myremotefile.tar.gz" />
</ssh:session>

where line 116 (which encountered the exception) is the ssh:sftp rename line.

i.e. it's getting past the scp line, but failing immediately after. What I would *expect* to happen is that the connection would be re-used by ssh:scp and that the connection would remain open for the duration of the ssh:session construct.

As for using a sftp "put", if I replace <ssh:scp localFile="mylocalfile.tar.gz" remoteToFile="sometargetdir/myremotefile.tar.gz.new" /> with <ssh:sftp action="put" file="mylocalfile.tar.gz" tofile="sometargetdir/myremotefile.tar.gz.new" /> as you suggested then it all works ... but only with version 0.1.2 of the library (I got a NPE with version 0.1.1).

[pjdarton]

Any progress on this?

At present, the official version is 0.1.1, which throws a NullPointerException during <ssh:sftp action="put" ...>
The unofficial 0.1.2 is better, in that <ssh:sftp action="put" ...> works, but that isn't available as an official download yet (https://dovetail.com/downloads/antssh/ only has 0.1.1).

Would it be possible to get the 0.1.2 code released officially?

[dovetail]

As mentioned earlier, the recommended workaround and preferred usage is to use the sftp task.

We are waiting to publish a new release while we work on a few other minor bugs.

[pjdarton]

OK, understood.
I will await the official release

[pjdarton]

A couple of weeks ago (Jan 17th) I received an email announcing that there was a new (official) release ... but https://dovetail.com/downloads/antssh/ is still showing version 0.1.1
Was the updated version of antssh not included in the release?

[dovetail]

The announcement was for a new release of Co:Z Toolkit (5.0.0).

Thanks for reminding us though. We just posted a new version of the ant-ssh package (0.1.3)
https://dovetail.com/downloads/antssh/

[pjdarton]

Thank you