JetBrains / kotless

Kotlin Serverless Framework
Apache License 2.0
1.15k stars 57 forks source link

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

Open svok opened 4 years ago

svok commented 4 years ago

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.

svok commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

svok commented 4 years ago

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

AarjavP commented 3 years ago

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 commented 2 years ago

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?