a-schild/nextcloud-java-api

downloadFolder not working with special charater

flelayo opened this issue · 9 comments

We are using the org.aarboard.nextcloud.api.NextcloudConnector.downloadFolder function and had an issue with the special character. For example for the filename "MobaXterm backup.zip" sardine.exists gave us a malformated URI exception because space was not escaped.

We tried to use the buildWebdavPath but it seems complex to use as the function is called previously on the root path.
I will try to make the pull request work this time if not will edit the issue with the code we used to correct it.

Thanks

Yes, the handling of special characters is not consistent at the moment.
Would be glad for help in this.
We need to define at which place encoding (and decoding) of the file/foldernames is done, so it does not happen twice (or none at all)

For the encoding part as buildWebdavPath is using a URIbuilder I think it is better to always use this function before sending something in sardine.
My fix when tested on a folder with space was not working. I only encoded the filename (because it was my real case scenario). So I will try to make more change in downloadFolder to use buildWebdavPath before each download call.

Here are the changes only for downloadFolder using buildWebdavPath before sardine call (both directory and file can have a space now):

public void downloadFolder(String remotePath, String rootDownloadDirPath) throws IOException {
        int depth=1;
        String[] segments = remotePath.split("/");
        String folderName = segments[segments.length - 1];
        String newDownloadDir = rootDownloadDirPath + "/" + folderName;
        File nefile1 = new File(newDownloadDir);
        if(!nefile1.exists()) {
            LOG.info("Creating new download directory: "+newDownloadDir);
            nefile1.mkdir();
        }

        String listPathURL = buildWebdavPath(remotePath);
        int count = 0;
        String filePath;
        List<String> retVal= new LinkedList<>();
        List<DavResource> resources;
        Sardine sardine = buildAuthSardine();
        try
        {
            try {
                resources = sardine.list(listPathURL, depth);
            } catch (IOException e) {
                throw new NextcloudApiException(e);
            }

            for (DavResource res : resources)
            {
                System.out.println(res.getName());
                //Skip the Documents folder which is listed as default as first by the sardine output
                if(count != 0) {
                    if(res.isDirectory()) {
                        String fileName = res.getName();
                        String pathtosend = remotePath + "/" + fileName;
                        downloadFolder(pathtosend,newDownloadDir);
                    }
                    else {
                            String fileName = res.getName();
                            filePath = buildWebdavPath(remotePath + "/" + fileName);
                            retVal.add(res.getName());

                            InputStream in = null;
                            if (sardine.exists(filePath)) {
                                in = sardine.get(filePath);

screenshot of the git change to see where are the changes:
image

Any chance you can send mit a diff file?
Care to write a unit test for this?

Here is the diff Folder.java.txt. there is some noise due to my IDE reformating

Unit test were not working (I added the setting.xml but still, wrong plugin version ?) so I disabled them and It will be hard to check if they work.

Looks like it fixes the problem, thanks

Can you test with the 11.7.0-SNAPSHOT release?

Yes version 11.7.0-SNAPSHOT solves this issue, thanks.

It also solve the issue we had from 11.6.0 it seems.

Yes, it should also solve the problem with login name not identical to the NC internal user name in issue #68

The drawback is, that we have to ask the server what the user name of that user is, before we can work with files in NC, but there is no way to know this for sure, so that's the only real working solution.

12.0.0 works fine, thanks