Add file editor limit #4256
Add file editor limit #4256
Conversation
| # Default for OOD_FILE_EDITOR_MAX is 12*1024*1024 bytes. | ||
| # @return [Integer] | ||
| def file_editor_max | ||
| ENV['OOD_FILE_EDITOR_MAX']&.to_i || 12582912 |
There was a problem hiding this comment.
Could the environment variable mention "SIZE" as well? I feel like "MAX" is a bit unclear. Even if it mirrors the naming scheme of OOD_DOWNLOAD_DIR_MAX, I don't see that as a good reason to repeat previous mistakes.
I.e., I'd suggest OOD_FILE_EDITOR_MAX_SIZE as the name.
12 MiB sounds like a good default!
|
The system test is failing to edit a remote file. I guess because you can't determine a remote file's size through In any case - here are the logs from the failing test. You go to edit it, then it's redirected back with a 302. |
|
I could probably comment here on the RemoteFile parts as I've contributed those. And for Then, you should be able to use |
| @@ -175,16 +175,13 @@ def edit | |||
| parse_path(filepath, fs) | |||
| validate_path! | |||
|
|
|||
| if @path.stat.size > ::Configuration.file_editor_max_size | |||
| if !@path.editable? | |||
| redirect_back(fallback_location: root_path, alert: "<strong>#{@path}</strong> is not an editable file".html_safe) | |||
There was a problem hiding this comment.
Try to avoid html_safe. What if the @path is in the allowlist (most centers don't even use it) but it is in fact <script>window.alert('gotcha');</script>.txt with 000 permissions?
There was a problem hiding this comment.
I went ahead and removed the bolding completely. Probably not a good idea to handle that in the controller in hindsight.
Add file size limit for editing both remote and local files.
backport Add file editor limit (#4256) to 4.0
Fixes #495