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

The list of supported mime-types is too limited #28

Open
svok opened this issue Feb 26, 2020 · 10 comments
Open

The list of supported mime-types is too limited #28

svok opened this issue Feb 26, 2020 · 10 comments
Labels
enhancement New feature or request

Comments

@svok
Copy link
Contributor

svok commented Feb 26, 2020

Now I see the list of supported mime-types:

enum class MimeType(val mimeText: String, val isBinary: Boolean, val extension: String) {
    PLAIN("text/plain", false, "txt"),
    HTML("text/html", false, "html"),
    CSS("text/css", false, "css"),

    PNG("image/png", true, "png"),
    APNG("image/apng", true, "apng"),
    GIF("image/gif", true, "gif"),
    JPEG("image/jpeg", true, "jpeg"),
    BMP("image/bmp", true, "bmp"),
    WEBP("image/webp", true, "webp"),

    JS("application/javascript", false, "js"),
    JSON("application/json", false, "json"),
    XML("application/xml", false, "xml"),
    ZIP("application/zip", true, "zip"),
    GZIP("application/gzip", true, "gzip");
}

And they are too few. In practice many more file types are used including ttf, md, etc. They are hundreds. So, the enum here looks inappropriate.

@TanVD TanVD added the enhancement New feature or request label Feb 26, 2020
svok added a commit to svok/kotless that referenced this issue Mar 2, 2020
@svok
Copy link
Contributor Author

svok commented Mar 2, 2020

I suggest a pull request with 3 extra mime-types. It is not at all a solution, but I am afraid the solution requires more time.

The origin of the problem is MimeType enum. We cannot use enums for extendable mime-type list. But if we use other object we cannot use it in the annotation @StaticGet.
On the other hand, why do we need MimeType in @StaticGet? The extension can be resolved from the file provided. Only mime-type is required together with isBinary flag.
I think the semantics of @StaticGet should be:

@Target(AnnotationTarget.FIELD)
annotation class StaticGet(val path: String, val mime: String, val isBinary: Boolean)

But this semantics breaks the compatibility

TanVD added a commit that referenced this issue Mar 2, 2020
#28 extra mime-types are added: md, map, ttf
@TanVD
Copy link
Member

TanVD commented Mar 2, 2020

Well, in general the idea was to provide useful list of mime types with correct binary flags (in some cases, those flags are not really obvious). But still, we can add one more annotation that gives you ability to creates static files with custom mime and isBinary, like in your example. Am I right, that it would solve a problem?

@svok
Copy link
Contributor Author

svok commented Mar 3, 2020

I consider the annotation only from the point of compatibility. In my project I prefer KTOR version.
Ktor has a rather advanced mime type classes based on ContentType and I would prefer you to use them instead of enum MimeType. But there are following disadvantages of such descision.

  1. These classes are a part of external huge project with huge dependencies and I understand your will to avoid their usage at least for kotless-* modules.
  2. ContentType does not operate with isBinary

Concerning isBinray. I see that it is required for base64 encoding of a response, but I failed to understand why it is required. At least Ktor successfully functions without that. But anyway I admit that isBinary is a useful property and worth being included into ContentType classes. It is easy:

private val rawMimes: String
    get() = """
.123,application/vnd.lotus-1-2-3
.3dmf,x-world/x-3dmf

So, I see that now you are trying to make the same functionality for mime-types that has been realized in Ktor. And this leads to a code duplication.
I would suggest to separate the mime-type functionality from Ktor into a separate library and extend it with isBinary functionality. I guess this will help to both Kotless and Ktor and they both will get extra functionality.

There is some other case I want to discuss. In my project I have a webpack-compiled distribution that includes files like LICENSE - with no extension. It is evident that this file is normal 'text/plain' but there is no way to detect that from the lacking extension. Ktor uses by default

val OctetStream = ContentType("application", "octet-stream")

but Kotless just throws an Exception.

And the last question concerns compatibility. I think that now the state of Kotless is experimental alpha. It has not got wide popularity, there are no projects in production with it and compatibility break will not bring a big pain to the community.
But I am convinced that enum MimeType in particular in @StaticGet will bring more problems in the future due to a lack of flexibility.

@TanVD
Copy link
Member

TanVD commented Mar 8, 2020

Well, actually MimeTypes are necessary only for static resources. The main reason is that static resources are served right from S3 via API Gateway integration. Because of it Kotless should be able to understand what is the MimeType of static resource during deployment. And isBinary is used also for static resources -- API Gateway should be configured with binary_media_types during deployment. Otherwise, you'll get static resource svg as a Plain Text, even if mime type is correct

@TanVD
Copy link
Member

TanVD commented Mar 8, 2020

In case of LICENSE most probably Kotless throws an exception cause it tries to detect MimeType by extension. We can use octet-stream by default, but not sure it will fit everyone

@TanVD
Copy link
Member

TanVD commented Mar 8, 2020

I agree about enum, but I would remain some object with most used MimeTypes, to help users. We have few projects in production in JetBrains with Kotless, but breaking compatibility in this part will not be very painful, if it will be obvious -- how to fix it :)

@TanVD
Copy link
Member

TanVD commented Mar 8, 2020

I'll think about new interface for MimeTypes. Probably will create PR and will ask you to review it, if it is possible :)

@svok
Copy link
Contributor Author

svok commented Mar 13, 2020

Probably will create PR and will ask you to review it, if it is possible :)

You are welcome. I will be glad to help the project

svok added a commit to svok/kotless that referenced this issue Apr 5, 2020
@AarjavP
Copy link

AarjavP commented Mar 17, 2021

We have a use case of building + deploying a static website folder and would like to make all the files in it available. Unfortunately any file not matching one of the expected extensions above is invalid, and fails for files like LICENSE as stated.
I was going to look into how we can make use of the annotation solution proposed above but there hasn't been any changes made yet right? Does the annotation solution also work for static 'folders' as well or do we have to specify each file individually?

This has become one of the major pain points in building / deploying using kotless.

@salamanders
Copy link

Ran into this one lately, a mime-type wasn't in the list (mjs JavaScript module script), and Chrome keels over with "Failed to load module script: Expected a JavaScript module script but the server responded with a MIME type of "application/octet-stream". Strict MIME type checking is enforced for module scripts per HTML spec."

Is there a way to inject custom mime type mappings into what Static can handle?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants