fix #11625: sanitize invalidly named FS paths

[mtbc]

Fixes http://trac.openmicroscopy.org.uk/ome/ticket/11625 by partially reverting mtbc@34748dd. Now Python clients should be able to set FilesetEntry.clientPath to a /-separated but otherwise-hairy path (e.g., "/".join(dir.splitall())) and have the server fix the path at import time so as to sanitize it according to http://www.openmicroscopy.org/site/support/omero5.1-staging/sysadmins/fs-upload-configuration.html#legal-file-names. (ImportLibrary.createImport has ImportContainer.fillData do path sanitization but clients may import by other means.) The FilePathNamingValidator class, though now unused, may yet have further use and is very lovely, so remains in the codebase.
--no-rebase

[mtbc]

I wondered if there was some nice Guava way to transform the elements of a collection (map (map f) ...) but I don't easily see one.

[joshmoore]

Relaunched on 247. In general, patch seems like quite the easy fix. We likely have zero integration tests running on Windows so manual testing will be necessary.

[snoopycrimecop]

Conflicting PR. Removed from build OME-5.1-merge-push#371. See the console output for more details.

[snoopycrimecop]

Conflicting PR. Removed from build OME-5.1-merge-push#372. See the console output for more details.

[mtbc]

If it helps any with testing the sanitization: One could hack ImportLibrary.createImport to construct a ClientFilePathTransformer with a Function<String, String> argument that simply returns the string it was given, that should disable the client-side sanitization so one can see how the server copes with a nasty path.

[bpindelski]

In the ImportLibrary.createImport() method I did

final ClientFilePathTransformer sanitizer = new ClientFilePathTransformer(new Function<String, String>() {
            public String apply(String string) {
                return string;
            }
        });

Then I used Insight and tried importing a file named //**``\\test11_R3D.dv. The import succeeds and the file path in the client is shown without changes, but in ManagedRepository the files is stored as xx;;xx``!!test11_R3D.dv. I presume this is expected behaviour. The same transformation happens with no changes to createImport().

Are there any other test cases I should try out? If not, then I'd presume this is good to merge.

[mtbc]

Looks good, thank you.

[snoopycrimecop]

Conflicting PR. Removed from build OME-5.1-merge-push#375. See the console output for more details.

[snoopycrimecop]

Conflicting PR. Removed from build OME-5.1-merge-push#376. See the console output for more details.

[mtbc]

Please kick travis for me.

[mtbc]

Need to re-test this PR after merging in develop?

[joshmoore]

Patch still looks quite simple and straight-forward, so no worries here. One question though: @bpindelski, did your testing occur on Windows?

[bpindelski]

@joshmoore No, the test was done on OS X.

[joshmoore]

If I can be so rude, I really think this there needs to be some cross testing, though if @pwalczysko prefers to do that via a different build, we an merge and list this elsewhere. We simply don't have anything that does:

V server/client >  | *nix  | Windows
--------------------------------------
      *nix         |       |
--------------------------------------
   Windows         |       |  

/cc @sbesson

[bpindelski]

@joshmoore I can test this PR again tomorrow, as currently I don't have a Windows VM handy. Unless Petr beats me to it.

[mtbc]

Which client/server combinations suffice? (Perhaps just Windows client to Unix server?) Also, should the cross-testing include @bpindelski's hack from #2909 (comment) in the client?

[pwalczysko]

Well, I am not sure what is this about, but in a hope this would help I took a Windows Insight and imported some images into trout. It went fine.

[snoopycrimecop]

Conflicting PR. Removed from build OME-5.1-merge-push#377. See the console output for more details.

[snoopycrimecop]

Conflicting PR. Removed from build OME-5.1-merge-push#378. See the console output for more details.

[snoopycrimecop]

Conflicting PR. Removed from build OME-5.1-merge-push#381. See the console output for more details.

[mtbc]

@joshmoore is mostly worred about how /, \ get converted cross-platform. I would expect forbidden ones to be converted to !: for instance, importing a\b from a Unix client to a Windows server should result in a!b.

[mtbc]

@bpindelski kindly determined that this PR's conflict is with #2884.

[bpindelski]

V server/client >  | *nix  | Windows
------------------------------------
      *nix         |   ✓   |   ✓
------------------------------------
   Windows         |   ✓   |   ✓

Notes:

• Windows server/OS X client - tested using Windows\foo/bar\bizz/buzz.fake. In Windows OMERO.insight the displayed filename is Windows\foo:bar\bizz:buzz.fake and in ManagedRepository - Windows!foo;bar!bizz;buzz.fake
• OS X server/Windows client - impossible to import anything with \ or / in the file name (Windows prohibits such file's existence; Insight prohibits adding such files to the import list).
• Windows server/Windows client - again, NTFS doesn't allow for any funny characters in the file name, so not much testing possible.
• OS X server/OS X client - tested using Windows\foo/bar\bizz/buzz.fake. In OS X OMERO.insight the displayed filename is Windows\foo:bar\bizz:buzz.fake and in ManagedRepository - Windows!foo;bar!bizz;buzz.fake

[joshmoore]

💯 for the testing, @bpindelski!

[bpindelski]

The last commit doesn't change anything on the code-path of file name sanitisation, so this is good to merge.