Skip to content

Conversation

tslocum
Copy link

@tslocum tslocum commented Apr 6, 2021

Resolves #8152.

This is an alternative implementation to the existing PR for this issue.

@silverwind
Copy link
Member

I think this should be optional and default off for the sake of security. See #8152 (comment).

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 6, 2021
@tslocum
Copy link
Author

tslocum commented Apr 6, 2021

Thanks @silverwind. PR updated.

@silverwind silverwind added the topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! label Apr 7, 2021
Resolves go-gitea#8152.

Signed-off-by: Trevor Slocum <trevor@rocketnine.space>
@silverwind
Copy link
Member

See #8152 (comment) for a better suggestion.

@tslocum
Copy link
Author

tslocum commented Apr 14, 2021

While I can see merit in your suggestion, the changes in this PR could be explicitly enabled by users via a configuration option (as you originally suggested), with a warning stating the potential security implications. It would also coexist with custom content types when they are implemented (i.e., check custom content types before detecting a file's content type).

Closing because the scope of the solution has grown beyond my interest.

@tslocum tslocum closed this Apr 14, 2021
@go-gitea go-gitea locked and limited conversation to collaborators Jun 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[API] Provide correct MIME type when getting a raw text file
3 participants