Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.

Consider as optional the mimetype in the file description #50

Closed
giomfo opened this issue Aug 5, 2021 · 3 comments · Fixed by #51
Closed

Consider as optional the mimetype in the file description #50

giomfo opened this issue Aug 5, 2021 · 3 comments · Fixed by #51

Comments

@giomfo
Copy link
Member

giomfo commented Aug 5, 2021

Hi, a recent change in the attachment description at the Element client level "Remove Redundant mimetype Fields" broke the use of the content scanner in encrypted rooms.
Any scan request fails with the following error because the mimetype is missing

{
  "info" : "Client error: File type not supported",
  "reason" : "MCS_MEDIA_NOT_CLEAN"
}

The mimetype field has been removed from the content.file description, because it was already mentioned in the content.info(see here.

Suggestion: consider as optional the mimetype in the file description at the content scanner level here. The mimetype is currently checked from the decrypted content too

@anoadragon453
Copy link
Member

It's worth noting that the client will independently send the content.file dictionary from the m.room.message event. Thus the content scanner never gets access to the specced content.info.mimetype field, only content.file.mimetype.

We could only optionally consider content.file.mimetype, yet I'd actually go as far to say that we should just completely ignore content.file.mimetype altogether, as it's not specced.

As @giomfo says, the mimetype of the actual file is eventually checked via magic bytes, regardless of what the client claims.

@anoadragon453
Copy link
Member

#51 addresses this issue, and now ignores the field altogether.

@anoadragon453
Copy link
Member

As noted in element-hq/element-web#17145 (comment), this field actually is currently specced (or at least intended to be), but that there's an open MSC (matrix-org/matrix-spec-proposals#2582) to remove it, and clients have been steadily doing so.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants