Warning: Can't synchronize with repository "(default)" (/home/git/ome.git does not appear to be a Git repository.). Look in the Trac log for more information.
Notice: In order to edit this ticket you need to be either: a Product Owner, The owner or the reporter of the ticket, or, in case of a Task not yet assigned, a team_member"

Task #11625 (closed)

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

File-path sanitization needed for Python

Reported by: jamoore Owned by: mtbcarroll
Priority: critical Milestone: 5.1.0-m1
Component: OmeroFs Version: 5.0.0-beta1
Keywords: n.a. Cc:
Resources: n.a. Referenced By: n.a.
References: n.a. Remaining Time: 0.0d
Sprint: n.a.

Description

The Python import code, even just for testing the import-set logic, will require client path rewriting in order to be cross-platform.

Change History (13)

comment:1 Changed 10 years ago by mtbcarroll

Isn't it already being rewritten? (This is through import.py, right?)

comment:2 Changed 10 years ago by jamoore

No, this is a complete, Python-based import. No Java, and therefore nothing done client-side.

comment:3 Changed 10 years ago by mtbcarroll

Ah, so you'd probably like a port of ClientFilePathTransformer to Python, okay. (More efficient than asking the server to do it.)

(I've added #11636 as a related concern.)

comment:4 Changed 10 years ago by jamoore

If you think it's possibly to confidently allow the server to make these changes (by passing "/" or "\" for example) then I'm fine with that.

comment:5 Changed 10 years ago by mtbcarroll

The code to do that was removed in https://github.com/mtbc/openmicroscopy/commit/34748dda2620ce1e5d4b2c5111a9804d85367bd2 but unfortunately I don't remember why we decided to do that. I'm happy to provide Python code to do the sanitization, or to go back to having importFileset do it, or to provide a separate server API call to sanitize paths, whichever you think best.

comment:6 Changed 10 years ago by mtbcarroll

  • Milestone changed from 5.0.0-beta2 to 5.0.0-beta3

comment:7 Changed 10 years ago by mtbcarroll

  • Milestone changed from 5.1.0 to 5.1.0-m1

Perhaps better resolved sooner rather than later.

comment:8 Changed 10 years ago by mtbcarroll

At present ImportLibrary.createImport has ImportContainer.fillData do sanitization in setting up the fileset entries. If faced with unsanitary entries from other sources, the server throws FilePathNamingException (extends ValidationException) with details of the problem.

comment:9 Changed 10 years ago by mtbcarroll

I think I had misunderstood http://trac.openmicroscopy.org.uk/ome/ticket/11625#comment:4 -- I believe that, while various sanitization can be and was done server-side, the breaking up of the path into its components delimited by the separators was always done client-side.

comment:10 Changed 10 years ago by mtbcarroll

Summary of discussion: have server do sanitization again, with API adjusted to fit needing to know client path separator.

comment:11 Changed 10 years ago by mtbcarroll

  • Status changed from new to accepted

comment:12 Changed 10 years ago by mtbcarroll

  • Resolution set to fixed
  • Status changed from accepted to closed

Fixed by https://github.com/openmicroscopy/openmicroscopy/pull/2909: do "/".join(dir.splitall()) Python-side and, with /-separated paths, the server now does remaining sanitization automatically.

Open to further changes / helpers in the longer term for clients who want more control over sanitization: FilePathNamingValidator code is thus retained as a possible helper for helpers.

comment:13 Changed 10 years ago by jean-marie burel <j.burel@…>

  • Remaining Time set to 0

(In [8af67ea9025e1d8ca11c43f3ce60b50e061f958f/ome.git] on branch develop) Merge pull request #2909 from mtbc/trac-11625-sanitize-file-paths

fix #11625: sanitize invalidly named FS paths

Note: See TracTickets for help on using tickets. You may also have a look at Agilo extensions to the ticket.

1.3.13-PRO © 2008-2011 Agilo Software all rights reserved (this page was served in: 0.66480 sec.)

We're Hiring!