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

Fix file serving for files with absolute paths #586

Merged
merged 1 commit into from
Feb 22, 2016

Conversation

lukeyeager
Copy link
Member

Bug reported at https://groups.google.com/d/msg/digits-users/MW_kHbOEqNo/y7wXpvlSFAAJ

To reproduce:

  • Create a dataset from textfiles on the server (i.e. enter the path on the server manually)
  • Click on the textfile from the DatasetJob page
  • Get a 404 error because the path is wrong

@lukeyeager lukeyeager added the bug label Feb 18, 2016
@gheinrich
Copy link
Contributor

I don't see this issue on master, is this because I'm using a devserver and the source install?
For example when I click on a path like /home/greg/ws/digits/digits/jobs/20160117-131355-ba71/train.txt I seem to be correctly directed to http://localhost:5000/files/20160117-131355-ba71/train.txt and I can see the contents of the file.

@lukeyeager
Copy link
Member Author

That still works because the file you're using is in the jobs directory. Try one on your desktop, for example.

@gheinrich
Copy link
Contributor

Indeed! Moving the .txt files to /tmp/text/*.txt, now with your fix at 08f73e5 I don't get links to text files any more on the dataset job page. Is it expected?
job-local-file

@lukeyeager
Copy link
Member Author

I don't get links to text files any more on the dataset job page. Is it expected?

That is what I meant to do, yes. But I admit it's a little confusing/disappointing.

The serve_file route only serves up files from within the jobs directory. That's for security and because it integrates with nginx more easily. See this comment:

# https://github.com/NVIDIA/DIGITS/blob/v3.2.0/digits/views.py#L319-L328
def serve_file(path):
    """
    Return a file in the jobs directory
    If you install the nginx.site file, nginx will serve files instead
    and this path will never be used
    """
    jobs_dir = config_value('jobs_dir')
    return flask.send_from_directory(jobs_dir, path)

if relative:
path = os.path.relpath(path, config_value('jobs_dir'))
if relative:
path = os.path.relpath(path, config_value('jobs_dir'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated with this change but that code duplication makes it look like Task and Job classes could share a common ancestor

@gheinrich
Copy link
Contributor

Beside #593 the change looks good to me and is working for me.

lukeyeager added a commit that referenced this pull request Feb 22, 2016
Fix file serving for files with absolute paths
@lukeyeager lukeyeager merged commit 5a2dd2d into NVIDIA:master Feb 22, 2016
@lukeyeager lukeyeager deleted the fix-abspath-serving branch February 22, 2016 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants