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

template-import: Add format check to the import file upload #447

Open
wants to merge 4 commits into
base: release
Choose a base branch
from

Conversation

FrederikSchlemmer
Copy link
Collaborator

This pull request adds a check before accessing the uploaded import file. This verifies that the uploaded file matches the needed file format to perform an import. This behavior is verified by an implemented test.

Closes #355

@FrederikSchlemmer FrederikSchlemmer force-pushed the fix-invalid-fileUpload-produces-serverErrors branch from d8a92d4 to befd791 Compare November 27, 2020 10:13
Copy link
Member

@czarnecki czarnecki left a comment

Choose a reason for hiding this comment

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

Checking if the filename ends with "xls" or "xlsx" isn't enough to ensure that the file is a valid Spreadsheet file. I could just upload an empty file with the name "xlsx" and it would trigger either the same or a similar exception as before. So we should either verify that it's a valid file beforehand, or somehow inform the Page that it's an invalid file.

@FrederikSchlemmer FrederikSchlemmer force-pushed the fix-invalid-fileUpload-produces-serverErrors branch 2 times, most recently from 906201a to d9c1eec Compare December 1, 2020 10:03
Copy link
Member

@tinne tinne left a comment

Choose a reason for hiding this comment

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

When I now start an import to an already existing template, the template is duplicated under the same name instead of (as happened before) the file in the already existing template being swapped. Also, the same template name should be allowed only once per project.

Regarding file recognition:

  • .xslt is accepted, which is ok, but not being recognized by error message (may be an undocumented feature, so all right), but
  • .xslm is sometimes accepted and sometimes rejected, and I do not understand why.
  • .xls is rejected when it is in fact CSV, which is ok, but possibly deserves an error of its own
  • uploading real old .xls files lead a new error (Caused by: java.lang.ClassCastException: org.apache.poi.hssf.usermodel.HSSFWorkbook cannot be cast to org.apache.poi.xssf.usermodel.XSSFWorkbook), so either handle this or remove .xls support

Everything else seems to be alright.

As I accidently already tested the ticket, you might consider moving the ticket to project 2 as well.

@FrederikSchlemmer FrederikSchlemmer force-pushed the fix-invalid-fileUpload-produces-serverErrors branch 3 times, most recently from bedef99 to ce8e9db Compare December 9, 2020 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid template - upload produces internal server errors
3 participants