redirect from invalid file url

[ashton22305]

Fixes #4360

This error only occurred when attempting to access a nonexistent directory directly from the URL, not when using the "Change Directory" button. It also brought the user to a table that appeared to have navigated to the nonexistent directory, with the buttons to create files and directories still present but causing errors when clicked.

Instead, the user is redirected to the home directory when trying to access a nonexistent directory by URL.

[Bubballoo3]

This looks good and does fix the double error message, but I wonder if we should bump them to the first valid directory instead of straight home? Maybe some brainstorming around how you get into this situation would help, but say I have a file /HOME/somedirectory/form.yml, and I manually type the url (maybe I am already viewing a different file and this is easier than navigating back?) but I type from.yml instead of form, and run into this error. If the rest of the path is good, it would be more convenient to get dumped into /HOME/somedirectory/, where I can click on the correct file, than go all the way back to /HOME/, which will require extra navigation to get to where I wanted.

Not sure if this scenario is realistic, it would require iteratively validating the path, but depending on how we do this it might be ok performance wise. Also happy to hear if there are any use cases that you had in mind for this error.

[ashton22305]

I think that's a good idea! Originally, I was concerned about if I were to type in HOME/baddirectory/otherbaddirectory that there would be a bunch of redirects at the same time, and similarly if I were to go to HOME/baddirectory/gooddirectory/form.yml, it would iterate all the way back to HOME. Similarly, a user could end up going down many directories that that they do not have permission to access, ending in /. In these two cases, maybe it would be slightly confusing to get redirected to those pages?

Still, I think it makes sense to attempt to go back at least one directory, and either fallback to home when none can be found or just redirect to home if the immediate previous directory is valid. If we validate directories because actually redirecting, I don't think performance will be an issue, I'm just not sure which is better for the user.

[Bubballoo3]

Yeah the question is whether we have a good way of checking a path exists without redirecting. Since we always pass the absolute path in the url, it could be as simple as

path = false
while !path
  begin
     File.new(url_path) # Errors if path doesn't exist
     path = url_path
  rescue
     url_path = url_path.split('/')[0...-1].join('/')
  end
end

but using some features from the PathName class that I don't know off the top of my head

[robinkar]

I just realized the original issue is somewhat of a self-inflicted issue.
At CSC, we provide direct links to the log file for the interactive app sessions instead of just the links to the output directory, to guide the users towards making good support requests by requesting them to include the linked log file for the session. The important detail here is that I've made the link visible without checking that the file actually exists, which got me into the situation with double error messages in the first place.

I agree that defaulting to the closest existing directory would make the most sense, with a single redirect and some potential extra checks at the end (e.g. in our case, / would not be in the allowlist -> throws another error).

Yeah the question is whether we have a good way of checking a path exists without redirecting. Since we always pass the absolute path in the url, it could be as simple as

path = false
while !path
  begin
     File.new(url_path) # Errors if path doesn't exist
     path = url_path
  rescue
     url_path = url_path.split('/')[0...-1].join('/')
  end
end

but using some features from the PathName class that I don't know off the top of my head

I believe the easiest way would be something like:

irb(main):007> Pathname.new("/home/rkarlsso/ondemand/bad_directory/bad_file").ascend.find(&:exist?)
=> #<Pathname:/home/rkarlsso/ondemand>

[Bubballoo3]

This looks good!

[johrstrom]

For remote directories, this redirects back to the root - / - of the local file system.

If you enter the URL there is no referrer, so that's unfortunate. Seems like we need some sort of toggle for remote and local paths?

[ashton22305]

Would this not attempt to reach each possible remote path before redirecting to the local filesystem? My understanding was that next_valid is only nil if there were no directories readable by traversing the entire path.

[johrstrom]

I'd have to look at the implementation of remote_file.rb to figure it out. I'm not sure what @path.path actually is out of remote_file and if ascend on that object actually traverses the remote file system at all.

[johrstrom]

OK so @path is an actual Pathname but since it's a remote file system it won't actually traverse the remote, instead traversing the local filesystem and the root / is the only common location on both.

ondemand/apps/dashboard/app/models/remote_file.rb

Line 14 in c2bd154

@path = Pathname.new(path)

[johrstrom]

I guess by toggle I meant more like if it's a local path, redirect back to the top most readable directory, but if it's a remote directory ... I guess just redirect back to home as we can't know where else to go.

[johrstrom]

The last commit has this error, but even when I use @filesystem I run into the error where RcloneUtil doens't have readable? because basically everything on the remote is assumed to be readable.