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

Multiple issues with validateFileUpload function in src/patient/fileUpload.ts #7458

Closed
1 task
kabirrajsingh opened this issue Mar 22, 2024 · 10 comments · Fixed by #8419
Closed
1 task

Multiple issues with validateFileUpload function in src/patient/fileUpload.ts #7458

kabirrajsingh opened this issue Mar 22, 2024 · 10 comments · Fixed by #8419
Assignees

Comments

@kabirrajsingh
Copy link
Contributor

kabirrajsingh commented Mar 22, 2024

Describe the bug
The validation function could be rewritten to ensure more reliability.
To Reproduce
Steps to reproduce the behavior:

  1. Go to 'facility/:facilityId/patient/patientId/files/'
  2. Click on 'Choose File'
  3. Issue1-
    a. Try to upload a file with some extension like .exe
    b. Notice that the validateFileUpload function actually returns true in this case and later on while actually making the api call to /api/v1/files we get the error that the filetype is inaccurate, the expected behavior here should be to reject the file when it is getting uploaded in the first place instead of sending it over to the fileupload api to reduce unneccesary api calls.
    Issue2-
    a. Try changing the file extension of some .exe to .pdf
    b. It will get uploaded normally, this should not be the case, the filetype only gets validated via the extension whereas a more reliable mechanism should be used to verify the filetype.
    Issue3-
    a. After uploading an unsupported file type and getting the error when we click on the cross button, the form fields get grayed out, it should get reset instead

Expected Behavior

image

  • When an invalid format file is selected, the upload button should be grayed out, and an error message should be shown in the frontend indicating that it is an invalid format.

make sure this same behavior is applicable in patient consultation page and patient files as well

steps to enter patient consultation file

  1. go to the patient tab
  2. select any patient with active consultation
  3. now from the nav bar under-diagnosis card, select the files
  4. try to upload the .exe file and see the error

steps to enter patient files

  1. after the above steps, in the top right corner, select the patient details button
  2. then select the upload patient files
  3. try to upload the .exe file and see the error

Screenshots

Screenshot_12

@kabirrajsingh
Copy link
Contributor Author

@vigneshhari @rithviknishad please check the issue and let me know if I can work on it

@kabirrajsingh
Copy link
Contributor Author

I was thinking of using file signature checks via the file header to resolve the unsupported file type upload problem.
For this I needed to know what are the file types that we are expecting to return a positive validation?
I was thinking of including .png, .jpeg, .jpg, .mp3, .midi, .wav, .pdf formats. Any other format which I need to consider?

@kabirrajsingh
Copy link
Contributor Author

@khavinshankar @sainak could you please check it once

Copy link

Hi, @coronasafe/care-frontend-maintainers, This issue has been automatically marked as stale because it has not had any recent activity.

@gigincg
Copy link
Member

gigincg commented Aug 5, 2024

@nihal467 Can you verify if this is still an issue?

@nihal467 nihal467 removed their assignment Aug 5, 2024
@nihal467
Copy link
Member

nihal467 commented Aug 5, 2024

@gigincg the issue still exists, added some comment related to the expected behavior as well

@github-actions github-actions bot added the stale label Aug 21, 2024
@gigincg
Copy link
Member

gigincg commented Aug 25, 2024

@nihal467 @shivankacker Are these being solved in the File Upload Refactor?

@shivankacker
Copy link
Member

@gigincg

image
Issue 1 : The new mechanism checks for the file type before making an API call.

Issue 2: Masking a non valid file as a valid file works, but do we need to implement a check for this? I don't know if it is really required.

Issue 3: The new component does not have this issue.

@gigincg
Copy link
Member

gigincg commented Aug 26, 2024

@shivankacker Are we using the accept prop for the input?

Refer: https://stackoverflow.com/questions/4328947/limit-file-format-when-using-input-type-file

@shivankacker
Copy link
Member

@gigincg we are, but it looks like it got broken somewhere during the refactor
#8419 this should fix it

@shivankacker shivankacker moved this from Triage to Review required in Care Sep 2, 2024
@github-project-automation github-project-automation bot moved this from Review required to Done in Care Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
5 participants