yuzutech / kroki

Creates diagrams from textual descriptions!
https://kroki.io
MIT License
2.83k stars 211 forks source link

HTTP date-related headers wrong #1064

Closed MarcelWaldvogel closed 2 years ago

MarcelWaldvogel commented 2 years ago

The date-related headers seen by the web browser (through an Nginx proxy) are as follows:

Date: Fri, 14 Jan 2022 21:50:44 GMT
Expires: 1642629044863
Last-Modified: 1567581178724

https://github.com/yuzutech/kroki/blob/c4e0bb386f4742bd788d5fa6b53c2d49100f9e4c/server/src/main/java/io/kroki/server/response/Caching.java#L27

I'll be preparing a pull request, but can only really test it after #1063.

ggrossetie commented 2 years ago

You are correct, for MDN:

The Date was probably fixed by Nginx, because it is also generated as the number of milliseconds by Caching.java

That's highly likely, I get a wrong Date when using Kroki without NGINX:

$ http :8000/graphviz/svg/eNpLyUwvSizIUHBXqPZIzcnJ17ULzy_KSanlAgB1EAjQ
HTTP/1.1 200 OK
cache-control: public, max-age=432000
content-length: 1362
content-type: image/svg+xml
date: 1642611876174
etag: 2.40.1X-P4LW6vNuh1dPqzkVpE3G00
expires: 1643043876174
last-modified: 1567581178724
vary: origin
MarcelWaldvogel commented 2 years ago

For the header-related tests (continuing the discussion here, as suggested, instead of the PR #1065), I had trouble creating a request where I could later check the headers against. I was trying something along the following lines in server/src/test/java/io/kroki/server/service/DiagramHandlerTest.java:

  @Test
  void should_return_date_headers() {
    DiagramService mockDiagramService = mockDiagramService(Lists.newArrayList(FileFormat.SVG), null);
    DiagramHandler diagramHandler = new DiagramHandler(mockDiagramService);
    MockDiagramRequest mockDiagramRequest = new MockDiagramRequest();
    mockDiagramRequest.setMethod(HttpMethod.POST);
    mockDiagramRequest.addParam("output_format", "svg");
    mockDiagramRequest.setContentType("text/plain");
    mockDiagramRequest.setBodyAsString("Bob -> Alice : hello");
    mockDiagramRequest.setPath("/plantuml/svg");

    RoutingContext routingContext = mockDiagramRequest.getRoutingContext();
    HttpServerResponse httpServerResponseMock = mock(HttpServerResponse.class);
    when(routingContext.response()).thenReturn(httpServerResponseMock);
    when(httpServerResponseMock.putHeader(any(CharSequence.class), any(CharSequence.class))).thenReturn(httpServerResponseMock);

    diagramHandler.createPost("plantuml").handle(routingContext);
    Mockito.verify(httpServerResponseMock).putHeader(HttpHeaders.CONTENT_TYPE, "image/svg+xml");
  }

thenReturn(httpServerResponseMock) is not being used, and therefore Mockito.verify() at the end fails. Kroki is my first use of Mockito, so I hope you can help.

ggrossetie commented 2 years ago

I was suggesting to test the Caching class directly:

HttpServerResponse httpServerResponseMock = mock(HttpServerResponse.class);
Caching caching = new Caching("1.2.3");
LocalDateTime date = LocalDateTime.of(2022, 1, 2, 3, 4, 5);
long today = date.toInstant(ZoneOffset.ofTotalSeconds(0)).toEpochMilli();
caching.addHeaderForCache(httpServerResponseMock, "data", today);

// assert
MarcelWaldvogel commented 2 years ago

This has been implemented now.