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 bookmark fileinput #1370

Merged
merged 5 commits into from Sep 13, 2016
Merged

Fix bookmark fileinput #1370

merged 5 commits into from Sep 13, 2016

Conversation

wch
Copy link
Collaborator

@wch wch commented Sep 13, 2016

Fixes #1368. The problem was that a restored fileInput wasn't marked with the correct serializer because it uses a different code path from a file that was uploaded the normal way.

This PR also:

  • Copies the restored file to a temp dir. Previosuly, it let the restored session access the bookmarked file directly, which could be a problem if the user's code modifies the file.
  • Has a better check that the restored file doesn't contain a path, so that a malicious user can't access any other parts of the filesystem.
  • Uses createUniqueId in the FileUploadContext, for consistency.

@wch wch added the review label Sep 13, 2016
@@ -94,7 +94,7 @@ FileUploadContext <- R6Class(
},
createUploadOperation = function(fileInfos) {
while (TRUE) {
id <- paste(as.raw(p_runif(12, min=0, max=0xFF)), collapse='')
id <- createUniqueId(12)
private$ids <- c(private$ids, id)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a separate issue, but it seems to me that the id shouldn't be recorded here. Instead, it should be recorded after the dir.create() succeeds. Although realistically, there's no need to attempt the dir.create and then loop if it fails, because the probability of a name collision is so low. It seems more likely that the dir.create would fail due to a permission problem, and then this code would get stuck in an infinite loop.

@wch wch merged commit 9613c58 into master Sep 13, 2016
@wch wch deleted the fix-bookmark-fileinput branch September 13, 2016 20:38
@wch wch removed the review label Sep 13, 2016
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

1 participant