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

[BUG] Unable to delete timelapse from UI when filename contains parenthesis #724

Closed
jneilliii opened this issue Jan 17, 2015 · 5 comments
Closed

Comments

@jneilliii
Copy link
Contributor

  1. What were you doing?
    Trying to delete timelapse mpg within web UI.
  2. What did you expect to happen?
    For the file to delete and remove from list in UI.
  3. What happened instead?
    File remained on filesystem and in web UI.
  4. Branch & Commit or Version:
    Version: 1.2.0-dev-404-gb0805a9 (devel branch)
  5. Printer model & used firmware incl. version
    N/A
  6. Link to octoprint.log on gist.github.com or pastebin.com:
    No log entries added to octoprint.log
  7. Link to contents of terminal tab on gist.github.com or pastebin.com:
    N/A

After investigating the issue it appears to be related to parenthesis in the filename. If there aren't any parenthesis in the filename the operation works as expected. Running dev branch on raspbian OS wheezy version 7.

I love cookies

@jneilliii
Copy link
Contributor Author

Just verified that after renaming the file to exclude parenthesis via the console the file deleted as expected.

@jneilliii
Copy link
Contributor Author

Issue does not exist on the filemanager on the main front page where the stl and gcode files live.

@jneilliii jneilliii changed the title [BUG] Unable to delete timelapse from UI when contains parenthesis [BUG] Unable to delete timelapse from UI when filename contains parenthesis Jan 17, 2015
@derpicknicker1
Copy link
Contributor

I can confirm this bug. I tested it with my Version: 1.2.0-dev-405-gf52afb8-dirty (devel branch).
It seems like werkzeug.utils.secure_filename causes this.
You find it in server/api/timelapse.py on line 53:

secure = os.path.join(settings().getBaseFolder("timelapse"), secure_filename(filename))

Updating the line to

secure = os.path.join(settings().getBaseFolder("timelapse"), filename)

will make it work. But i'm not sure if it's the best idea to leave out the secure_filename(). Otherwise, the filenames for timelapses are taken from the *.gcode files. So i think they will allready be safe?!

@foosel
Copy link
Member

foosel commented Jan 19, 2015

They will, but input validation is still necessary here. I'll fix it to properly check that no directory traversal happens here, but that should be enough.

foosel added a commit that referenced this issue Jan 19, 2015
Was broken after switching to new file name sanitizing with the new file management layer, still stripped everything but ascii from the filename although timelapse file names may actually contain more than that character set since the gcode file names they are based off may too.

Closes #724
foosel added a commit that referenced this issue Jan 19, 2015
@foosel
Copy link
Member

foosel commented Jan 19, 2015

Fixed in devel

@foosel foosel closed this as completed Jan 19, 2015
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants