Knotx / Knotx/knotx

Template modification date determines value of Last-Modified header

Open
#471 3 comments 1 reaction 0 assignees View on GitHub
discussion
Dominant language
No language data
Stars
126
Forks
26
PR merge metrics
No merged PRs in 30d

Description

# 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](https://user-images.githubusercontent.com/6334715/47743052-597aa500-dc7e-11e8-8290-779e240c5808.png)

* the flow looks as on the diagram I attached above
* Handlebars template changes few times a year, so template repository caches HTML markup on disk
* data stored in the API (the one knot.x fetches dynamic data from) is super dynamic and produces personalized results
* upon first request knot.x fetches the template and receives the following response
```
HTTP/1.1 200 OK
Date: Tue, 30 Oct 2018 18:52:41 GMT
Server: Apache
X-Content-Type-Options: nosniff
Last-Modified: Tue, 30 Oct 2018 12:59:26 GMT
Accept-Ranges: bytes
Vary: Accept-Encoding
Content-Encoding: gzip
X-Request-ID: W9ioeeae7@ELj-qnzR2fcAAAAkA
X-Frame-Options: SAMEORIGIN
X-XSS-Protection: 1; mode=block
Referrer-Policy: no-referrer-when-downgrade
Cache-Control: private
Content-Length: 2772
Connection: close
Content-Type: text/html; charset=UTF-8
```
* knot.x sends a call to the API to obtain data required to populate template placeholders
* as soon as the response is ready knot.x returns:
```
HTTP/1.1 200 OK
Cache-Control: private
Content-Type: text/html; charset=UTF-8
Date: Tue, 30 Oct 2018 18:55:39 GMT
Last-Modified: Tue, 30 Oct 2018 12:59:26 GMT
Vary: Accept-Encoding
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
X-Request-ID: W9ipK@-2Z@DZlr@S@yRslAAAAQs
X-XSS-Protection: 1; mode=block
Connection: close
Content-Length: 7080
```
* please note that response is not cacheable (because of `Cache-Control: private`) and contains the same `Last-Modified` header knot.x got from template repository
* response goes to the CDN, but it won't cache it as the origin said it's not cacheable
* 2nd request comes in, this time this is a conditional GET with `If-Modified-Since` header (to be honest I have no idea why Chrome decided to send such request, perhaps `Last-Modified` presence was enough and `Cache-Control: private` doesn't have any meaning in this context)
* CDN passes it through to the origin, the same steps are executed as in the first case (template fetch, API call, page assembly by knot.x)
* the response reaches CDN which sees that it contains `Last-Modified` header. At the same time it remembered that the request had `If-Modified-Since` header. Simple date comparison takes place and it turns out that nothing has changed, so CDN thinks it's ok to reply with 304 to save some bandwidth (304 doesn't have the body)

## 2nd example

* as in the 1st example, static template gets combined with dynamic data, but this time responses produced by knot.x are cacheable
* as soon as the response reaches CDN it'll be cached according the the value of `Cache-Control`, `Surrogate-Control`, `Edge-Control` or any other header that tells CDN how long it can keep it in the cache
* at some point in time TTL expires and given resource has to be fetched from the origin one more time
* to optimize round trips CDNs very often send conditional GET whenever someone asked for an object which just expired in the cache. In other words CDN knows original `Last-Modified` header value and sends GET with `If-Modified-Since: `. knot.x does its magic and returns `Last-Modified` it got from the template call.
* if the template hasn't changed, but the dynamic dataset has, we will end up with 304 from the origin, so updates won't be cached in CDN, hence end users won't see any changes

# Expected behavior

My first thought was that knot.x should do one of the following things:
* stop relying on `Last-Modified` it gets from template repository
* re-set `Last-Modified` if Handlebars markup got evaluated and placeholders were filled in with dynamic data
* stop sending `Last-Modified` header completely

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

Contributor guide

Open the contributing guide

Research direction

Start by tracing how knot.x forwards template-repository response headers through Handlebars evaluation and assembled HTTP responses. Reproduce the conditional GET scenarios with Last-Modified and If-Modified-Since, then verify that template-derived Last-Modified is not reused after dynamic data injection and that the chosen replacement behavior is covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
handlebars, java
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.