Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Script paths to basename 11464 #39

Merged
merged 1 commit into from
Sep 19, 2013

Conversation

will-moore
Copy link
Member

This fixes https://trac.openmicroscopy.org.uk/ome/ticket/11464 where several scripts use a filename coming from a script parameter without checking whether the filename is actually a path.

For each of the scripts in this PR, we validate that the file name is only a name using os.path.basename().

To test:

  • Run E.g. Movie Figure script in Insight, using an image that is named like: path/to/the/image.dv
    The path will be removed and the figure should become named "image.jpg".
  • You can try this with a few of the different scripts (include Batch_Image_Export), but probably not worth doing all of them. I've already tested them all and they all have exactly the same commit applied.

@joshmoore
Copy link
Member

Is there a reason that we use the actual requested filename in the script, rather than just uploading to that name. The one place where it's required looks to be the folder_name of the zip in batch exporting.

In terms of a more central solution, the only thing I have thought of so far is a "Filename" parameter type which is automatically sent through os.path.basename.

@bpindelski
Copy link

Tested with howe and insight - all looks good. Path elements are stripped out.

@joshmoore
Copy link
Member

Only side question, @will-moore, is if you also looked at omero-user-scripts@examples for any of these issues. Merging.

joshmoore added a commit that referenced this pull request Sep 19, 2013
@joshmoore joshmoore merged commit 924f650 into ome:dev_4_4 Sep 19, 2013
@will-moore
Copy link
Member Author

Doh - I have just done as @joshmoore suggested and only used the supplied file name for naming the uploaded original file (not for writing locally).

I'll open another PR

@joshmoore
Copy link
Member

Sorry @will-moore!

@sbesson
Copy link
Member

sbesson commented Sep 23, 2013

--rebased-to #42

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants