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

Add specific application/x-binary mime type for bin extension files to mime type mapping #4846

Closed
wants to merge 2 commits into from

Conversation

AlexAndBear
Copy link

@AlexAndBear AlexAndBear commented Sep 16, 2024

Enhancement: Add specific mime type for bin files

We've added the specific mime type 'application/x-binary' for bin files to the list of mime types.

Copy link

update-docs bot commented Sep 16, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@AlexAndBear AlexAndBear changed the title Add specific application/x-binary mimetype for bin extension files to… Add specific application/x-binary mime type for bin extension files to mime type mapping Sep 16, 2024
@AlexAndBear AlexAndBear requested review from glpatcern and a team as code owners September 16, 2024 10:40
kobergj
kobergj previously approved these changes Sep 16, 2024
Copy link
Contributor

@kobergj kobergj left a comment

Choose a reason for hiding this comment

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

@2403905 will take the review over. We are not sure this fixes it without side effects.

@kobergj kobergj dismissed their stale review September 16, 2024 12:12

needs investigation

Copy link

@2403905 2403905 left a comment

Choose a reason for hiding this comment

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

We map the mime type to files by extension and not only bin has type application/octet-stream, there are bpk, buffer and etc. in a mimeTypes map. We also set the application/octet-stream to any files w/o extension.

I think we should take a look at the ListSupportedMimeTypes

@2403905
Copy link

2403905 commented Sep 16, 2024

I think the problem is here https://github.com/owncloud/ocis/blob/4d5168f6c3ab182db596c996e8b9090f4dedc55e/services/collaboration/pkg/helpers/registration.go#L41

	mimeTypesMap := make(map[string]bool)
	for _, extensions := range appUrls {
		for ext := range extensions {
			m := mime.Detect(false, ext)
			mimeTypesMap[m] = true
		}
	}

The mime.Detect returns the "application/octet-stream" as default. So, we could skip there the "application/octet-stream" mime type.

@AlexAndBear
Copy link
Author

AlexAndBear commented Sep 16, 2024

~~And then the files w/o a mapped extension<>mime type have no mime type anymore? I think the default is there on purpose.

I am wondering why OnlyOffice and Collabora are configured to be able to open especially the application/ocet-stream mime type.

Shouldn't we just avoid that ?~~

NVM, you are totally right, I think that ^ is especially what you are trying to achieve 💪

@2403905
Copy link

2403905 commented Sep 19, 2024

Fixed in a ocis

@2403905 2403905 closed this Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants