nextcloud/android-library

null pointer crash in CreateRemoteFolderOperation

farindk opened this issue · 5 comments

farindk commented

There is a null-pointer crash when using CreateRemoteFolderOperation in this situation:

When the operation is run, the following happens:

  • CreateRemoteFolder.run() calls createFolder() which fails.
  • Because it fails, it will call createParentFolder(), which will try to create "/" and fails again.
  • As it fails again, it will try to create the parent folder of "/".
  • It calls String parentPath = new File(remotePath).getParent() on remotePath="/", which results in `parentPath=null' (FileUtils.java:37)
  • It crashes in the following line.
nextcloud-android-bot commented

GitMate.io thinks a possibly related issue is #201 (Parser crash for activities with null values ).

tobiasKaminsky commented

Thank you for your detailed bug report!

The root cause is that you used, as mentioned, the wrong server url.
But the error results in the fact that only the parent folder is tried to be created.

Trying to use "https://myserver.de/remote.php/webdav" and create /subfolder" would then result in such a folder on server "remote.php/webdav/subfolder".

While this is of course not something you want, I do not see any other option, other to hardcode a check for "remote.php/webdav", but this is not too intrusive.

Other option would be to check base url, but same here: it can be nearly anything, so we cannot do any real assumption.

What do you think?

tobiasKaminsky commented

Edit: this does not work, as we only have "/Subfolder" as remote path, as the other part is from server…

farindk commented

I solved it on my side by filtering away any "remote.php/webdav" from the server URL. However, my point with this bug report is that the library should not crash with a null-pointer exception if such a case happens.

I made the following change in CreateFolderRemoteOperation:

         if (!result.isSuccess() && mCreateFullPath &&
                 RemoteOperationResult.ResultCode.CONFLICT == result.getCode()) {
-            result = createParentFolder(FileUtils.getParentPath(mRemotePath), client);
-            if (result.isSuccess()) {
-                result = createFolder(client);    // second (and last) try
+
+            if (!mRemotePath.equals("/")) {
+                result = createParentFolder(FileUtils.getParentPath(mRemotePath), client);
+                if (result.isSuccess()) {
+                    result = createFolder(client);    // second (and last) try
+                }
             }
         }

I.e. avoid trying to get the parent of the root folder and return an error message instead.
I can send you a PR for this, but since it's only one line to insert, you may want to simply do this yourself.
Or you may want to handle it in another way. Whatever. Just make sure there is no hard crash with wrong user input :-)

  • 👍1
AndyScherzinger commented

Will ship with 1.4.0 and is now available already via the master-SNAPSHOT