LocalFileContent causes browser cache without allowing Cache Control about ktor HOT 8 CLOSED

@gzoritchak Why don't you simply intercept and wrap LocalFileContent with your best max-age settings?

from ktor.

the other alternative is to provide constructor parameters expires and cache control

from ktor.

@cy6erGn0m

Why don't you simply intercept and wrap LocalFileContent with your best max-age settings?

I first tried to put a global interception :

        intercept(ApplicationCallPipeline.Infrastructure) {
            if (call.request.httpMethod == HttpMethod.Get ||
                    call.request.httpMethod == HttpMethod.Head) {
                call.response.cacheControl(CacheControl.MaxAge(300))
            }
        }

It put the Cache-control header to all requests except the ones served by LocalFileContent.

the other alternative is to provide constructor parameters expires and cache control

I didn't see any constructor in LocalFileContent that gives access to cacheControl property.

from ktor.

My bad: the global interception is working.

In fact I have now 2 Cache-Control http header for one request!!

from ktor.

@gzoritchak in a simple case I see no double cache control header

from ktor.

@cy6erGn0m

1. Thanks for your modifications on LocalFileContent to allow CacheControl.
2. In your test, adding cache control to the LocalFileContent doubles the header. It should not be possible (https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2)

Multiple message-header fields with the same field-name MAY be present in a message if and only if the entire field-value for that header field is defined as a comma-separated list [i.e., #(values)]. It MUST be possible to combine the multiple header fields into one "field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field-value to the first, each separated by a comma.

    fun testInterceptCacheControl() {
        withTestApplication {
            application.intercept(ApplicationCallPipeline.Infrastructure) { call ->
                if (call.request.httpMethod == HttpMethod.Get ||
                        call.request.httpMethod == HttpMethod.Head) {
                    call.response.cacheControl(CacheControl.MaxAge(300))
                }
            }

            application.intercept(ApplicationCallPipeline.Call) { call ->
                call.respond(LocalFileContent(
                        File(basedir, "org/jetbrains/ktor/tests/http/StaticContentTest.kt"),
                        cacheControl = CacheControl.MaxAge(200)
                ))
            }

produces the assertion fail:

java.lang.AssertionError: 
Expected :[max-age=300]
Actual   :[max-age=300, max-age=200]

But I don't know if it should be considered as a bug. Is it the responsibility of the developper to be sure of adding only one time the Cache-Control header? In fact it's a little bit more complicated. The Cache-Control can be added more than one time but for different directives.

I close this bug and let you decide if the duplicate header with same directive should be considered as a bug.

from ktor.

Don't think we can eliminate double headers: to achieve this we have to keep and maintain a list of single values only headers that doesn't look good

from ktor.

I'm not sure.

The specification gives a general rule that can be applied. It's not an implementation based like "THIS particular header can be duplicated and THAT one not".

from ktor.