diff --git a/corere/apps/file_datatable/static/file_datatable/file_datatable.js b/corere/apps/file_datatable/static/file_datatable/file_datatable.js index 9e084c9..cc80640 100644 --- a/corere/apps/file_datatable/static/file_datatable/file_datatable.js +++ b/corere/apps/file_datatable/static/file_datatable/file_datatable.js @@ -39,7 +39,6 @@ function create_file_table_config(table_path, readonly, is_submission, file_url_ processing: true, stateSave: true, paging: true, - select: 'single', autoWidth: false, // dom: 'Bftlp', dom: 'Bfrtpl', @@ -60,13 +59,19 @@ function create_file_table_config(table_path, readonly, is_submission, file_url_ { data: 'path', render: function(data,type,row,meta){ - return row[0]; + if(readonly) { return row[0]; } + + //TODO: Stale, doing name first + return row[0] + ''; + } }, { data: 'name', render: function(data,type,row,meta){ - return row[1]; + if(readonly) { return row[1]; } + + return row[1] + ''; } }, ], @@ -78,4 +83,90 @@ function create_file_table_config(table_path, readonly, is_submission, file_url_ buttons: top_buttons } return config +} + +//TODO: When we show these modals, we need to look at the length of the path/name (that is hidden) to determine the length of the input field + +function show_edit_name_modal(file_url_base, file_path, old_name){ + name_length = 260 - decodeURIComponent(file_path).length; + $('#name_modal_file_name_new').attr('maxlength', name_length); + $('#name_modal_file_name_old').val(decodeURIComponent(old_name)); + $('#name_modal_file_path').val(file_path); + $('#name_modal_file_url_base').val(file_url_base); + $('#name_modal').modal('show'); +} + +function show_edit_path_modal(file_url_base, file_name, old_path){ + path_length = 260 - decodeURIComponent(file_name).length; + $('#path_modal_file_path_new').attr('maxlength', path_length); + $('#path_modal_file_path_old').val(decodeURIComponent(old_path)); + $('#path_modal_file_name').val(file_name); + $('#path_modal_file_url_base').val(file_url_base); + $('#path_modal').modal('show'); +} + +//TODO: Why are we only encoding one of the two text fields in both? +function submit_edit_name_modal_and_reload(){ + file_name_old = encodeURIComponent($('#name_modal_file_name_old').val()) + file_name_new = $('#name_modal_file_name_new').val() + file_url_base = $('#name_modal_file_url_base').val() + file_path = $('#name_modal_file_path').val() + + var regex1 = /[*?"<>|;#:\\\/]/; + var regex2 = /\.\./; + if($.trim(file_name_new).length === 0 || regex1.test(file_name_new) || regex2.test(file_name_new)) { + $('#name_modal_sanitize_error').removeAttr('hidden'); + return; + } + + old_full_path = file_path + file_name_old + new_full_path = file_path + file_name_new + + rename_url = file_url_base+'renamefile/?old_path='+old_full_path+'&new_path='+new_full_path + rename_and_refresh(rename_url, clear_name_modal, show_name_modal_generic_error) +} + +function show_name_modal_generic_error() { + $('#name_modal_unexpected_error').removeAttr('hidden') +} + +function clear_name_modal() { + $('#name_modal_sanitize_error').attr("hidden",true); + $('#name_modal_unexpected_error').attr("hidden",true); + $('#name_modal_file_name_new').val('') + $('#name_modal').modal('hide'); +} + +function submit_edit_path_modal_and_reload(){ + file_path_old = encodeURIComponent($('#path_modal_file_path_old').val()) + file_path_new = $('#path_modal_file_path_new').val() + file_url_base = $('#path_modal_file_url_base').val() + file_name = $('#path_modal_file_name').val() + + //TODO: We need to ensure path begins and ends with / + var regex1 = /[^a-zA-Z0-9 /_\-\.]/; + var regex2 = /\.\./; + var regex3 = /\/\s*\// + if($.trim(file_path_new).length === 0 || regex1.test(file_path_new) || regex2.test(file_path_new) || regex3.test(file_path_new) || !file_path_new.startsWith("/") || !file_path_new.endsWith("/")) { + $('#path_modal_sanitize_error').removeAttr('hidden'); + return; + } + + old_full_path = file_path_old + file_name + new_full_path = file_path_new + file_name + + rename_url = file_url_base+'renamefile/?old_path='+old_full_path+'&new_path='+new_full_path + rename_and_refresh(rename_url, clear_path_modal, show_path_modal_generic_error) + +} + +function show_path_modal_generic_error() { + $('#path_modal_unexpected_error').removeAttr('hidden') +} + +function clear_path_modal() { + $('#path_modal_sanitize_error').attr("hidden",true); + $('#path_modal_unexpected_error').attr("hidden",true); + $('#path_modal_file_path_new').val('') + $('#path_modal').modal('hide'); } \ No newline at end of file diff --git a/corere/apps/file_datatable/templates/file_datatable/file_datatable.html b/corere/apps/file_datatable/templates/file_datatable/file_datatable.html index 3bdbc52..8fd7300 100644 --- a/corere/apps/file_datatable/templates/file_datatable/file_datatable.html +++ b/corere/apps/file_datatable/templates/file_datatable/file_datatable.html @@ -11,6 +11,68 @@ + + + + +{% comment %}If we want this library to truly be portable, all this CORE2 specific stuff should not be here? {% endcomment %} +{% comment %}Also file_datatable.js references delete_and_refresh which exists in the main app {% endcomment %} {% load static %} {% endif %} \ No newline at end of file diff --git a/corere/main/templates/main/form_upload_files.html b/corere/main/templates/main/form_upload_files.html index 0d70032..f671c3b 100644 --- a/corere/main/templates/main/form_upload_files.html +++ b/corere/main/templates/main/form_upload_files.html @@ -35,7 +35,7 @@

{% csrf_token %} diff --git a/corere/main/templates/main/head.html b/corere/main/templates/main/head.html index 5e0b616..9491fd4 100644 --- a/corere/main/templates/main/head.html +++ b/corere/main/templates/main/head.html @@ -3,7 +3,7 @@ - + diff --git a/corere/main/tests/selenium_access_utils.py b/corere/main/tests/selenium_access_utils.py index ab8ee15..0ca628b 100644 --- a/corere/main/tests/selenium_access_utils.py +++ b/corere/main/tests/selenium_access_utils.py @@ -173,7 +173,7 @@ def call_request_method(browser, method, endpoint, debug=False): g_dict_editor_access = g_dict_normal_access.copy() g_dict_editor_access.update( { - "manuscript/create/": {"GET": 200, "POST": 500}, + "manuscript/create/": {"GET": 200, "POST": 500}, #TODO: Why 500? } ) @@ -347,7 +347,7 @@ def call_request_method(browser, method, endpoint, debug=False): "newfilecheck/": {"GET": 404, "POST": 405}, # Need a query string to not 404 # 'wtstream/': {'GET': 200}, # 'wtdownloadall/': {'GET': 200}, - "file_table/": {"GET": 200, "POST": 405}, # TODO: Should this 404 instead and hit the access restriction first? + "file_table/": {"GET": 200, "POST": 405}, } s_dict_admin_access__completed = s_dict_admin_access.copy() @@ -356,11 +356,11 @@ def call_request_method(browser, method, endpoint, debug=False): s_dict_yes_access_curator__out_of_phase = s_dict_admin_access.copy() s_dict_yes_access_curator__out_of_phase.update( { - "view/": {"GET": 404}, #, "POST": 404 Post disabled because ajax error, probably due to changing the file_table#TODO: Why does this 404? Because view is disabled when you have edit if you are non admin? + "view/": {"GET": 404}, #, "POST": 404 Post disabled because ajax error, probably due to changing the file_table #TODO: Why does this 404? Because view is disabled when you have edit if you are non admin? "viewfiles/": {"GET": 404, "POST": 404}, #TODO: Why does this 404? Because view is disabled when you have edit if you are non admin? "downloadall/": {"GET": 404, "POST": 404}, #TODO: I really don't understand why this is 404ing "newfilecheck/": {"GET": 404, "POST": 404}, # Need a query string to not 404 - "file_table/": {"GET": 200, "POST": 405}, # TODO: Should this 404 instead and hit the access restriction first? + "file_table/": {"GET": 200, "POST": 405}, "confirmfiles/": {"GET": 404 , "POST": 404}, #TODO: Disabled because it causes an ajax error / 500s. Probably due to changing file_table perms check. "uploader/": {"GET": 405, "POST": 404}, # Test with files to get actual 200 "fileslist/": {"GET": 200, "POST": 405}, @@ -393,7 +393,7 @@ def call_request_method(browser, method, endpoint, debug=False): "newfilecheck/": {"GET": 404, "POST": 404}, # 'wtstream/': {'GET': 404}, # 'wtdownloadall/': {'GET': 404}, - "file_table/": {"GET": 404, "POST": 405}, # TODO: Should this 404 instead and hit the access restriction first? + "file_table/": {"GET": 404, "POST": 405}, } #TODO: This is really a fix for a broken piece of code. file_table shouldn't 200. Then we can delete this and just use s_dict_no_access diff --git a/corere/main/tests/test_unit.py b/corere/main/tests/test_unit.py new file mode 100644 index 0000000..efcf0ce --- /dev/null +++ b/corere/main/tests/test_unit.py @@ -0,0 +1,57 @@ +# import unittest +from django.test import SimpleTestCase +from corere.main.views import classes as cl + +class HelperTestCase(SimpleTestCase): + # def setUp(self): + # pass + + #file_check returns "" if no issue with path+name, otherwise returns a string with the issue + def test_helper_sanitary_file_check(self): + self.assertFalse(cl._helper_sanitary_file_check("/good")) + self.assertFalse(cl._helper_sanitary_file_check("/good/good")) + self.assertFalse(cl._helper_sanitary_file_check("/good/good/good")) + + self.assertFalse(cl._helper_sanitary_file_check("/good.")) + self.assertFalse(cl._helper_sanitary_file_check("/good/good.")) + self.assertFalse(cl._helper_sanitary_file_check("/good/good/good.")) + + self.assertFalse(cl._helper_sanitary_file_check("/g /good")) + self.assertFalse(cl._helper_sanitary_file_check("/G/good")) + self.assertFalse(cl._helper_sanitary_file_check("/1/good")) + self.assertFalse(cl._helper_sanitary_file_check("/_/good")) + self.assertFalse(cl._helper_sanitary_file_check("/-/good")) + self.assertFalse(cl._helper_sanitary_file_check("/./good")) + + self.assertTrue(cl._helper_sanitary_file_check("")) + self.assertTrue(cl._helper_sanitary_file_check("bad")) + self.assertTrue(cl._helper_sanitary_file_check("bad/")) + self.assertTrue(cl._helper_sanitary_file_check("/bad/")) + self.assertTrue(cl._helper_sanitary_file_check("/bad/bad/")) + self.assertTrue(cl._helper_sanitary_file_check("/bad/bad/bad/")) + self.assertTrue(cl._helper_sanitary_file_check("//bad/bad")) + self.assertTrue(cl._helper_sanitary_file_check("/ /bad/bad")) + + self.assertTrue(cl._helper_sanitary_file_check("/bad/..")) + self.assertTrue(cl._helper_sanitary_file_check("/bad/..bad")) + self.assertTrue(cl._helper_sanitary_file_check("/bad/....bad")) + self.assertTrue(cl._helper_sanitary_file_check("/bad../")) + self.assertTrue(cl._helper_sanitary_file_check("/..bad/")) + self.assertTrue(cl._helper_sanitary_file_check("/..bad../")) + self.assertTrue(cl._helper_sanitary_file_check("../bad/")) + + self.assertTrue(cl._helper_sanitary_file_check("/*")) + self.assertTrue(cl._helper_sanitary_file_check("/?")) + self.assertTrue(cl._helper_sanitary_file_check("/\"")) + self.assertTrue(cl._helper_sanitary_file_check("/<")) + self.assertTrue(cl._helper_sanitary_file_check("/>")) + self.assertTrue(cl._helper_sanitary_file_check("/|")) + self.assertTrue(cl._helper_sanitary_file_check("/;")) + self.assertTrue(cl._helper_sanitary_file_check("/#")) + self.assertTrue(cl._helper_sanitary_file_check("/:")) + self.assertTrue(cl._helper_sanitary_file_check("//")) + self.assertTrue(cl._helper_sanitary_file_check("/\\")) + self.assertTrue(cl._helper_sanitary_file_check("/ * ? \" < > | ; # : \ ")) + + self.assertTrue(cl._helper_sanitary_file_check("/bad!/bad")) + self.assertTrue(cl._helper_sanitary_file_check("/!/bad")) \ No newline at end of file diff --git a/corere/main/urls.py b/corere/main/urls.py index 6e6737d..3750c2b 100644 --- a/corere/main/urls.py +++ b/corere/main/urls.py @@ -28,6 +28,7 @@ path("manuscript//unassignverifier//", users.unassign_verifier, name="manuscript_unassignverifier"), # path('manuscript//createsubmission/', classes.SubmissionCreateView.as_view(), name="manuscript_createsubmission"), path("manuscript//deletefile/", classes.ManuscriptDeleteFileView.as_view(), name="manuscript_deletefile"), + path("manuscript//renamefile/", classes.ManuscriptRenameFileView.as_view(), name="manuscript_renamefile"), path("manuscript//downloadfile/", classes.ManuscriptDownloadFileView.as_view(), name="manuscript_downloadfile"), path("manuscript//downloadall/", classes.ManuscriptDownloadAllFilesView.as_view(), name="manuscript_downloadall"), # path('manuscript//notebook/', main.open_notebook, name="manuscript_notebook"), #This is disabled, but we used it for internal container mode so we should look at it again when needed @@ -57,6 +58,7 @@ path("submission//viewfiles/", classes.SubmissionReadFilesView.as_view(), name="submission_readfiles"), path("submission//deletefile/", classes.SubmissionDeleteFileView.as_view(), name="submission_deletefile"), path("submission//deleteallfiles/", classes.SubmissionDeleteAllFilesView.as_view(), name="submission_deleteallfiles"), + path("submission//renamefile/", classes.SubmissionRenameFileView.as_view(), name="submission_renamefile"), path("submission//downloadfile/", classes.SubmissionDownloadFileView.as_view(), name="submission_downloadfile"), path("submission//downloadall/", classes.SubmissionDownloadAllFilesView.as_view(), name="submission_downloadall"), # path('submission//progress/', classes.SubmissionProgressView.as_view(), name="submission_progress"), diff --git a/corere/main/views/classes.py b/corere/main/views/classes.py index 7600559..ce7c128 100644 --- a/corere/main/views/classes.py +++ b/corere/main/views/classes.py @@ -1,4 +1,4 @@ -import logging, os, io, requests, urllib, time, git, sseclient, threading, base64, json, tempfile +import logging, os, io, re, requests, urllib, time, git, sseclient, threading, base64, json, tempfile from django.conf import settings from django.shortcuts import render, redirect, get_object_or_404 from django.contrib import messages @@ -588,6 +588,20 @@ def post(self, request, *args, **kwargs): try: file = request.FILES.get("file") fullRelPath = request.POST.get("fullPath", "") + + validation_error = _helper_sanitary_file_check("/" + fullRelPath + file.name) + if(validation_error): + logger.warning( + "While uploading the file " + + fullRelPath + + " on manuscript " + + str(self.object.id) + + ", the sanitization check on the new path failed : " + + validation_error + ) + return HttpResponse(status=400) + + path = "/" + fullRelPath.rsplit(file.name)[0] # returns '' if fullPath is blank, e.g. file is on root if gitfiles := m.GitFile.objects.filter(parent_manuscript=self.object, path=path, name=file.name): @@ -622,10 +636,22 @@ class ManuscriptDownloadFileView(LoginRequiredMixin, GetOrGenerateObjectMixin, T def get(self, request, *args, **kwargs): self.general(request, *args, **kwargs) - file_path = request.GET.get("file_path") - if not file_path: + file_path_escaped = request.GET.get("file_path") + if not file_path_escaped: raise Http404() - file_path = unescape(file_path) + file_path = unescape(file_path_escaped) + + validation_error = _helper_sanitary_file_check(file_path) + if(validation_error): + logger.warning( + "While downloading the file " + + file_path_escaped + + " on manuscript " + + str(self.object.id) + + ", the sanitization check on the new path failed : " + + validation_error + ) + return HttpResponse(status=400) return g.get_manuscript_file(self.object, file_path, True) @@ -637,10 +663,23 @@ class ManuscriptDeleteFileView(LoginRequiredMixin, GetOrGenerateObjectMixin, Tra def post(self, request, *args, **kwargs): self.general(request, *args, **kwargs) - file_path = request.GET.get("file_path") - if not file_path: + file_path_escaped = request.GET.get("file_path") + if not file_path_escaped: raise Http404() - file_path = unescape(file_path) + file_path = unescape(file_path_escaped) + + validation_error = _helper_sanitary_file_check(file_path) + if(validation_error): + logger.warning( + "While deleting the file " + + file_path_escaped + + " on manuscript " + + str(self.object.id) + + ", the sanitization check on the new path failed : " + + validation_error + ) + return HttpResponse(status=400) + g.delete_manuscript_file(self.object, file_path) folder_path, file_name = file_path.rsplit("/", 1) @@ -658,6 +697,80 @@ def post(self, request, *args, **kwargs): return HttpResponse(status=200) +#TODO: implement (switch from delete code). See submission version. Include sanitization +# ... this should probably be ONE endpoint for renaming file and path, as we have to pass both anyways and I think its the same in the backend +class ManuscriptRenameFileView(LoginRequiredMixin, GetOrGenerateObjectMixin, TransitionPermissionMixin, GenericManuscriptView): + http_method_names = ["post"] + transition_method_name = "edit_files_noop" + + def post(self, request, *args, **kwargs): + self.general(request, *args, **kwargs) + + #Note: This code assumes the name+path are combined + old_file_path_escaped = request.GET.get("old_path") + new_file_path_escaped = request.GET.get("new_path") + if not (old_file_path_escaped and new_file_path_escaped): + raise Http404() + + old_file_path = unescape(old_file_path_escaped) + new_file_path = unescape(new_file_path_escaped) + + old_result = _helper_sanitary_file_check(old_file_path) + if(old_result): + logger.warning( + "While renaming the supposed file " + + old_file_path_escaped + + " on manuscript " + + str(self.object.id) + + ", the sanitization check on the old path failed : " + + old_result + ) + return HttpResponse(status=400) + + validation_error = _helper_sanitary_file_check(old_file_path) + if(validation_error): + logger.warning( + "While renaming the file " + + old_file_path_escaped + + " to " + + new_file_path_escaped + + " on manuscript " + + str(self.object.id) + + ", the sanitization check on the new path failed : " + + validation_error + ) + return HttpResponse(status=400) + + #First rename actual file + rename_for_git = [{"old":old_file_path, "new":new_file_path}] + if not g.rename_manuscript_files(self.object, rename_for_git): + return HttpResponse(status=400) + + #Then rename GitFile in database + old_folder_path, old_file_name = old_file_path.rsplit("/", 1) + old_folder_path = old_folder_path + "/" + new_folder_path, new_file_name = new_file_path.rsplit("/", 1) + new_folder_path = new_folder_path + "/" + + try: + model_file = m.GitFile.objects.get(parent_manuscript=self.object, path=old_folder_path, name=old_file_name) + model_file.path = new_folder_path + model_file.name = new_file_name + model_file.save() + except m.GitFile.DoesNotExist: + logger.warning( + "While renaming file " + + old_file_path + + " on manuscript " + + str(self.object.id) + + ", the associated GitFile was not found. This could be due to a previous error during upload." + ) + + self.object.files_changed = True + self.object.save() + + return HttpResponse(status=200) + # TODO: Pass less parameters, especially token stuff. Could combine with ManuscriptUploadFilesView, but how to handle parameters with that... class ManuscriptReadFilesView(LoginRequiredMixin, GetOrGenerateObjectMixin, TransitionPermissionMixin, GitFilesMixin, GenericManuscriptView): @@ -1786,7 +1899,7 @@ class SubmissionUploaderView(LoginRequiredMixin, GetOrGenerateObjectMixin, Trans object_friendly_name = "submission" http_method_names = ["post"] - # TODO: Should we making sure these files are safe? + # TODO: Should we making sure these files are safe? We are escaping in rename def post(self, request, *args, **kwargs): self.general(request, *args, **kwargs) @@ -1794,8 +1907,19 @@ def post(self, request, *args, **kwargs): try: file = request.FILES.get("file") fullRelPath = request.POST.get("fullPath", "") - # print(file) - # print(fullRelPath) + + validation_error = _helper_sanitary_file_check("/" + fullRelPath + file.name) + if(validation_error): + logger.warning( + "While uploading the file " + + fullRelPath + + " on submission " + + str(self.object.id) + + ", the sanitization check on the new path failed : " + + validation_error + ) + return HttpResponse(status=400) + path = "/" + fullRelPath.rsplit(file.name)[0] # returns '' if fullPath is blank, e.g. file is on root # print(path) if gitfiles := m.GitFile.objects.filter(parent_submission=self.object, path=path, name=file.name): @@ -1833,10 +1957,22 @@ class SubmissionDownloadFileView(LoginRequiredMixin, GetOrGenerateObjectMixin, T def get(self, request, *args, **kwargs): self.general(request, *args, **kwargs) - file_path = request.GET.get("file_path") - if not file_path: + file_path_escaped = request.GET.get("file_path") + if not file_path_escaped: raise Http404() - file_path = unescape(file_path) + file_path = unescape(file_path_escaped) + + validation_error = _helper_sanitary_file_check(file_path) + if(validation_error): + logger.warning( + "While downloading the file " + + file_path_escaped + + " on submission " + + str(self.object.id) + + ", the sanitization check on the new path failed : " + + validation_error + ) + return HttpResponse(status=400) return g.get_submission_file(self.object, file_path, True) @@ -1864,10 +2000,23 @@ class SubmissionDeleteFileView(LoginRequiredMixin, GetOrGenerateObjectMixin, Tra def post(self, request, *args, **kwargs): self.general(request, *args, **kwargs) - file_path = request.GET.get("file_path") - if not file_path: + file_path_escaped = request.GET.get("file_path") + if not file_path_escaped: raise Http404() - file_path = unescape(file_path) + file_path = unescape(file_path_escaped) + + validation_error = _helper_sanitary_file_check(file_path) + if(validation_error): + logger.warning( + "While deleting the file " + + file_path_escaped + + " on submission " + + str(self.object.id) + + ", the sanitization check on the new path failed : " + + validation_error + ) + return HttpResponse(status=400) + g.delete_submission_file(self.object.manuscript, file_path) folder_path, file_name = file_path.rsplit("/", 1) @@ -1918,6 +2067,81 @@ def post(self, request, *args, **kwargs): return HttpResponse(status=200) +class SubmissionRenameFileView(LoginRequiredMixin, GetOrGenerateObjectMixin, TransitionPermissionMixin, GenericCorereObjectView): + http_method_names = ["post"] + transition_method_name = "edit_noop" + model = m.Submission + parent_model = m.Manuscript + object_friendly_name = "submission" + + def post(self, request, *args, **kwargs): + self.general(request, *args, **kwargs) + + #Note: This code assumes the name+path are combined + old_file_path_escaped = request.GET.get("old_path") + new_file_path_escaped = request.GET.get("new_path") + if not (old_file_path_escaped and new_file_path_escaped): + raise Http404() + + old_file_path = unescape(old_file_path_escaped) + new_file_path = unescape(new_file_path_escaped) + + old_result = _helper_sanitary_file_check(old_file_path) + if(old_result): + logger.warning( + "While renaming the supposed file " + + old_file_path_escaped + + " on submission " + + str(self.object.id) + + ", the sanitization check on the old path failed : " + + old_result + ) + return HttpResponse(status=400) + + validation_error = _helper_sanitary_file_check(old_file_path) + if(validation_error): + logger.warning( + "While renaming the file " + + old_file_path_escaped + + " to " + + new_file_path_escaped + + " on submission " + + str(self.object.id) + + ", the sanitization check on the new path failed : " + + validation_error + ) + return HttpResponse(status=400) + + #First rename actual file + rename_for_git = [{"old":old_file_path, "new":new_file_path}] + if not g.rename_submission_files(self.object.manuscript, rename_for_git): + return HttpResponse(status=400) + + #Then rename GitFile in database + old_folder_path, old_file_name = old_file_path.rsplit("/", 1) + old_folder_path = old_folder_path + "/" + new_folder_path, new_file_name = new_file_path.rsplit("/", 1) + new_folder_path = new_folder_path + "/" + + try: + model_file = m.GitFile.objects.get(parent_submission=self.object, path=old_folder_path, name=old_file_name) + model_file.path = new_folder_path + model_file.name = new_file_name + model_file.save() + except m.GitFile.DoesNotExist: + logger.warning( + "While renaming file " + + old_file_path + + " on submission " + + str(self.object.id) + + ", the associated GitFile was not found. This could be due to a previous error during upload." + ) + + self.object.files_changed = True + self.object.save() + + return HttpResponse(status=200) + # Used for ajax refreshing in EditFiles # TODO: Probably no longer be needed with list rewrite @@ -2692,12 +2916,27 @@ def _helper_submit_submission_and_redirect(request, submission): # messages.add_message(request, messages.SUCCESS, self.msg) return redirect("manuscript_landing", id=submission.manuscript.id) -# TODO: The error validation calling this could use refinement. It'll bail out after the first error and doesn't attach errors to file names. +# TODO: This is a good candidate for a unit test +# TODO: This could maybe use additional python-based sanitization https://pathvalidate.readthedocs.io/en/latest/pages/examples/sanitize.html +# NOTE: We don't actually show these messages anywhere other than logs currently. Browser-side JS also enforces this sanitization, this is just a back-up to prevent malice.\ def _helper_sanitary_file_check(path): + try: + folders_name, file_name = path.rsplit("/", 1) + except ValueError: + return "File must have a name and a path: " + path + folders_name = folders_name + "/" + if path.find("..") != -1: return "File name with .. not allowed." - if path.strip() == "" or (path.rsplit("/", 1) and path.rsplit("/", 1)[1].strip() == ""): - return "File name must include a character other than just spaces." if len(path) > 260: - return "File paths + cannot be longer than 260 characters." - # TODO: Maybe include slugify check. Not using right now because that'll remove spaces among other things. + return "File paths + names cannot be longer than 260 characters." + if file_name.strip() == "": + return "File name must include a character other than just spaces." + if re.search('\/\s*\/', folders_name): + return "Folder names cannot be blank or just spaces" + if re.search("[*?\"<>|;#:\\\/]", file_name): + return "File name cannot include these characters: * ? \" < > | ; # : \ /" + if re.search("[^a-zA-Z0-9\s/_\-\.]", folders_name): + return "File folder can only contain the alphanumerics, _ - . / and whitespace" + + return ""