Altinn / app-frontend-react

Altinn application React frontend
BSD 3-Clause "New" or "Revised" License
16 stars 24 forks source link

FileUpload - Attachment box gets highlighted with red indicating error for allowed content type #2012

Open CelineTrammi opened 1 month ago

CelineTrammi commented 1 month ago

Description of the bug

This was discovered by one of the testers, when opening the app in tt02. When uploading a .zip folder by dragging the file to the component, the box turns red even though the content type is allowed. As seen in the video, the zip is actually uploaded. However, the red box causes confusion for those who associate it to not being able to upload.

https://github.com/Altinn/app-frontend-react/assets/61122289/61dc5db3-6fa4-4ce6-863f-7725d5d195d0

Steps To Reproduce

I have not been able to reproduce the error myself. There might be something connected to different mime's of zip, but i'm not sure.

Additional Information

in applicationmetadata:

   {
      "id": "3-1-7-FileUpload",
      "allowedContentTypes": [
        "application/pdf",
        "application/vnd.ms-excel",
        "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
        "application/msword",
        "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
        "application/rtf",
        "image/jpg",
        "image/jpeg",
        "text/plain",
        "application/vnd.openxmlformats-officedocument.presentationml.presentation",
        "application/vnd.ms-powerpoint",
        "application/zip",
        "application/x-zip",
        "application/x-compressed",
        "application/x-zip-compressed",
        "application/vnd.oasis.opendocument.presentation",
        "application/vnd.oasis.opendocument.spreadsheet",
        "application/vnd.oasis.opendocument.text"
      ],
      "taskId": "Task_1",
      "maxSize": 200,
      "maxCount": 25,
      "minCount": 0,
      "grouping": "3.1.7-FileUpload.grouping-title",
      "enablePdfCreation": true,
      "enableFileScan": true,
      "validationErrorOnPendingFileScan": false,
      "enabledFileAnalysers": [],
      "enabledFileValidators": []
    },
olemartinorg commented 1 month ago

The video is not visible for me, so I re-encoded it:

https://github.com/Altinn/app-frontend-react/assets/700139/1f97369e-843b-4967-ba98-c79d1f418131

By 'highlighted with red' I assumed you meant a validation message the user sees? The only red I can see in the video is the indicator for DevTools. DevTools is never shown to the user in production mode, and if everything works apart from an error there, there's nothing to worry about.

Let me know if I'm mistaken here, but unless there's other validations (or this only happens for specific file types), I would think this is a duplicate of #1938

CelineTrammi commented 1 month ago

image This is this box I meant. When the user hovers a file that is not allowed, the border of the box goes from blue to red. This user uploads the files by opening the desired folder in a nearby window and drags it to the box. I think it differs from the issue you mentioned, because this occurs before the user uploads the file.

olemartinorg commented 1 month ago

I see. It seems to me the problem here is the inverse; We're allowing uploads when some of our code (specifically the mapExtensionToAcceptMime(...) call) tells us the attachment should not be possible to upload.

Btw, as far as I can tell we map .zip and .ZIP to the application/zip mime type. The next problem then apparently is to figure out why in this case the file was marked as not allowed to upload. You say you haven't been able to reproduce the issue yourself, but could you check with the user what the full file name is (including file extension) for the file that failed the test?

CelineTrammi commented 1 month ago

That makes sense! I was considering to request adding more mime types when I saw that Altinn Studio was removing the ones I added automatically. The ones I added were:

"application/x-compressed",
"application/x-zip-compressed"

These are not on the list I found here at least, and probably excluded from the map-function you mentioned.

It makes me suspect that the user had one of these files, but I can double check.

olemartinorg commented 1 month ago

I don't think this is about mime type, but about file extension. We don't actually check the mime type (i.e. app-frontend does not read actual file contents trying to find the actual mime type using magic numbers and known mime type signatures), we only look at the file name to make a rough decision. If both .zip and .ZIP maps to application/zip, another file with the same extension (but different content) will give you the exact same result.

So when you said you were unable to reproduce the issue yourself (assuming you weren't able to reproduce it with a .zip), that leads me to believe the user that first saw this problem did not use a .zip (rather, something else). If you're in contact with them, try to get a hold of that file name (or at least the file extension). If we're unable to reproduce the issue, that hampers our ability to properly fix it as well.

olemartinorg commented 1 month ago

I got to see a more detailed video of a reproduction now, and it looks quite strange indeed. The same file was uploaded multiple times, and only the first of those upload attempts showed the red border. Subsequent uploads showed a blue border, and all uploads worked. I think we might want to either remove this red border (if it's not really doing/preventing anything) or fix this problem and also make sure the file type check actually gives the user an error as well.

CelineTrammi commented 1 month ago

I just took a look at the file uploaded in the tt02. Maybe this information helps in the future.

Inspecting the data elements for the latest instance created in tt02, the data element that gave a red border on the box was this one:

{
            "id": "d438aa9a-8b7e-4a1f-9def-105f49b87514",
            "instanceGuid": "ba406335-e739-4869-ae41-38038a82f59c",
            "dataType": "3-2-8-FileUpload",
            "filename": "KRT-3000 v1 infoside til skjemakatalogen.zip",
            "contentType": "application/x-zip-compressed",
            "blobStoragePath": "krt/krt-3002a-1/ba406335-e739-4869-ae41-38038a82f59c/data/d438aa9a-8b7e-4a1f-9def-105f49b87514",
            "selfLinks": {
                "apps": "https://krt.apps.tt02.altinn.no/krt/krt-1157a-1/instances/50906771/ba406335-e739-4869-ae41-38038a82f59c/data/d438aa9a-8b7e-4a1f-9def-105f49b87514",
                "platform": "https://platform.tt02.altinn.no/storage/api/v1/instances/50906771/ba406335-e739-4869-ae41-38038a82f59c/data/d438aa9a-8b7e-4a1f-9def-105f49b87514"
            },
            "size": 31039,
            "contentHash": null,
            "locked": true,
            "refs": null,
            "isRead": true,
            "tags": [],
            "deleteStatus": null,
            "fileScanResult": "Clean",
            "references": null,
            "created": "2024-04-11T11:33:18.5261534Z",
            "createdBy": "341745",
            "lastChanged": "2024-04-11T11:33:18.526153Z",
            "lastChangedBy": "341745"
},

As seen, the contentType is x-zip-compressed, which was already included in applicationmetadata. So I don't understand how the temporary "fix" came by when adding application/octet-stream.

olemartinorg commented 1 month ago

That contentType appears when the backend looks into the file content when the file is uploaded. It is not really relevant to this problem here, because the checks that shows you the red border is caused by code in frontend that only analyzes the file name ending. We don't start uploading the file as soon as you hover it over the drop zone, that starts happening after you drop it, and only when the file is fully uploaded do we get to know what the backend considers the contentType to be.

CelineTrammi commented 1 month ago

Ah I see. Good news is that the file is uploaded at least 😄 . I wasn't too sure myself so this was initially to verify it being uploaded despite the red box.