Skip to content

MSC2701: Clarifying Content-Type usage in the media repo #2701

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 30, 2024
Merged

MSC2701: Clarifying Content-Type usage in the media repo #2701

merged 3 commits into from
Jan 30, 2024

Conversation

@turt2live
Copy link
Member

@turt2live turt2live commented Jul 28, 2020 •
edited by tulir
Loading

Rendered

Implementations: None required - see "Unstable Prefix" section for details.

FCP tickyboxes

@turt2live turt2live added proposal-in-review proposal A matrix spec change proposal kind:maintenance MSC which clarifies/updates existing spec labels Jul 28, 2020
@turt2live turt2live changed the title Proposal to clarify how Content-Type works with media MSC2701: Clarifying Content-Type usage in the media repo Jul 28, 2020
@aaronraimist aaronraimist mentioned this pull request Nov 29, 2020
Closed
@turt2live turt2live mentioned this pull request Dec 9, 2020
Closed
@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jun 8, 2021
@richvdh
Copy link
Member

richvdh commented Aug 3, 2021 •
edited
Loading

@turt2live: Given that this seems to be largely a statement of the status quo, why is it needs-implementation?

@richvdh
Copy link
Member

richvdh commented Aug 3, 2021

this looks ready to go to me, and I think we should FCP it.

@turt2live
Copy link
Member Author

turt2live commented Aug 3, 2021

@turt2live: Given that this seems to be largely a statement of the status quo, why is it needs-implementation?

Largely because of the criteria when applying that label: Nothing in the PR description to say that it needs an implementation. This translates to it being on my todo list to figure out the status of it.

@richvdh
Copy link
Member

richvdh commented Aug 3, 2021

I assert that it already has an implementation, and I propose FCP.

@mscbot fcp merge.

@mscbot
Copy link
Collaborator

mscbot commented Aug 3, 2021

Unknown disposition 'merge.'.

@richvdh richvdh removed the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Aug 3, 2021
@richvdh
Copy link
Member

richvdh commented Aug 3, 2021

ohh wait, I seem to have jumped the gun here. Sorry!

@richvdh richvdh added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Aug 3, 2021
richvdh
richvdh reviewed Aug 3, 2021
View reviewed changes
@aaronraimist aaronraimist mentioned this pull request Oct 11, 2021
Closed
turt2live and others added 2 commits December 15, 2023 22:54
@turt2live @richvdh
Apply suggestions from code review
184e54f 
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
@turt2live
Modernize wording
630a352
@turt2live turt2live removed the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Dec 16, 2023
@turt2live
Copy link
Member Author

turt2live commented Dec 16, 2023

I've updated this to match current behaviour, which also means no specific implementation is required. Sending it off for review:

@mscbot fcp merge

@mscbot
Copy link
Collaborator

mscbot commented Dec 16, 2023 •
edited by ara4n
Loading

Team member @turt2live has proposed to merge this. The next step is review by the rest of the tagged people:

Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for information about what commands tagged team members can give me.

@mscbot mscbot added proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. disposition-merge labels Dec 16, 2023
@mscbot
Copy link
Collaborator

mscbot commented Jan 25, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@mscbot mscbot added final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. and removed proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Jan 25, 2024
@mscbot
Copy link
Collaborator

mscbot commented Jan 30, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

@mscbot mscbot added finished-final-comment-period and removed disposition-merge final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Jan 30, 2024
@turt2live turt2live merged commit 3fa9417 into old_master Jan 30, 2024
turt2live added a commit that referenced this pull request Jan 30, 2024
@turt2live @richvdh
MSC2701: Clarifying Content-Type usage in the media repo (#2701)
c015c5b 
* Proposal to clarify how Content-Type works with media

* Apply suggestions from code review

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* Modernize wording

---------

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
@turt2live turt2live deleted the travis/msc/media-contenttype branch January 30, 2024 00:43
@turt2live turt2live added spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec and removed finished-final-comment-period labels Jan 30, 2024
@turt2live turt2live mentioned this pull request Mar 12, 2024
Closed
23 tasks
@turt2live turt2live mentioned this pull request Mar 19, 2024
Closed
6 tasks
@turt2live
Copy link
Member Author

turt2live commented Mar 19, 2024

Spec PR: matrix-org/matrix-spec#1758

@turt2live turt2live self-assigned this Mar 19, 2024
Kladki
Kladki reviewed May 31, 2024
View reviewed changes
proposals/2701-media-content-type.md
Comment on lines +24 to +25
For `GET /download`, the server MUST return a `Content-Type` which is either exactly the same as the
original upload, or reasonably close. The bounds of "reasonable" are:
Copy link
Contributor

@Kladki Kladki May 31, 2024

Choose a reason for hiding this comment

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

I know that this MSC was merged a while ago, but would it have made sense to also mandate a X-Content-Type-Options nosniff header as part of the response?

Copy link
Member

@anoadragon453 anoadragon453 Jun 25, 2024

Choose a reason for hiding this comment

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

@Kladki Could you open this question as a new issue on https://github.com/matrix-org/matrix-spec/issues? Then we can discuss and track it.

@zecakeh zecakeh mentioned this pull request Sep 5, 2024
Closed
41 tasks
@turt2live
Copy link
Member Author

turt2live commented Sep 30, 2024

Spec PR: matrix-org/matrix-spec#1935

@turt2live turt2live added merged A proposal whose PR has merged into the spec! and removed spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec labels Sep 30, 2024
@turt2live
Copy link
Member Author

turt2live commented Sep 30, 2024

Merged 🎉

This was referenced Oct 8, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anoadragon453 anoadragon453 anoadragon453 left review comments

@richvdh richvdh richvdh left review comments

+1 more reviewer

@Kladki Kladki Kladki left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@turt2live turt2live

Labels

kind:maintenance MSC which clarifies/updates existing spec merged A proposal whose PR has merged into the spec! proposal A matrix spec change proposal

Projects

Spec Core Team Workflow
Status: Merged

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@turt2live @richvdh @mscbot @anoadragon453 @Kladki