Knotx / knotx

Knot.x is a highly-efficient and scalable integration framework designed to build backend APIs
https://knotx.io
Apache License 2.0
126 stars 26 forks source link

Template modification date determines value of Last-Modified header #471

Open jwadolowski opened 6 years ago

jwadolowski commented 6 years ago

Bug description

I recently discovered that knot.x copies Last-Modified header it receives from template repository and sends it back to the HTTP client. Such behavior can cause caching issues, especially when rather static templates are combined with dynamic data and the response goes through shared cache (CDN, forward proxy server, etc).

knot.x version: 1.4.0

Steps to reproduce

I captured 2 scenarios that outline the problem.

1st example

knotx-304

2nd example

Expected behavior

My first thought was that knot.x should do one of the following things:

After a bit of thinking I realized that in some cases Last-Modified header presence could be needed (i.e. template that renders single object off of some identifier that doesn't change often). At the same time re-using whatever came from template repository is definitely not expected.

As a rule of thumb I'd say that whenever knot.x injects data into Handlebars template it should remove Last-Modified header and possibly re-set its value. Ideally the 2nd part should be somehow configurable.

Screenshots

N/A

Additional context

N/A

malaskowski commented 6 years ago

Hi @jwadolowski , you may configure what headers are returned by Knot.x Server instance and what are not by overriding allowedResponseHeaders in the conf/includes/server.conf. With that you can achieve the expected behavior you mentioned:

stop sending Last-Modified header completely (just remove Last-Modified entry in your Knot.x instance configuration).

The default behavior you suggested:

re-set Last-Modified if Handlebars markup got evaluated and placeholders were filled in with dynamic data

looks good to me. It could quite nice improvement.

Let us know it that helped.

jwadolowski commented 6 years ago

Thanks @Skejven. allowedResponseHeaders will do the trick, however there are 2 downsides of this approach:

Workaround I used was implemented directly at CDN level.

Regarding re-setting Last-Modified - I started thinking about that and it seems to be more complex than I originally assumed. Here's a few use cases:

In general there's probably no universal solution.

malaskowski commented 5 years ago

@jwadolowski , there might be a solution that Knot.x can technically support. It is a bit simplified scenario in comparison to your proposition:

As you wrote, this is not universal solution. It is custom behaviour, not everyone might want to use it. In my opinion this is good candidate for Knot.x 2.0 when it will be easy to insert some custom handler with that logic.