setTemplateProcessor() causes incorrect ETag and Last-Modified headers to be returned for static files

[m-z-b]

Platform

ESP32

IDE / Tooling

PlatformIO

What happened?

When using ASyncWebServer::serveStatic() with SetTemplateProcessor ETag and Last-Modified headers are returned based on the file system date.

This is incorrect if the template processor produces any dynamic content (e.g. the current time).

The browser will receives 304 Not Modified response on subsequent requests. In addition, if the Last-Modified header is suppressed, most browsers will not re-render the page if the ETag is unchanged.

The suggested resolution is that ASyncStaticWebHandler::handleRequest assumes not_modified is false if a _callback exists, and does not include ETag or Last-Modified headers in the response.

Stack Trace

No stack trace

Minimal Reproductible Example (MRE)

Snippets - assume that LittleFS contains a /status/index.html file with a date from 2024.

AsyncWebServer webServer(80);
...
  webServer.serveStatic("/", LittleFS, "/status/", "no-cache")
    .setDefaultFile("index.html")
    .setTemplateProcessor(StatusProcessor)
      ;

$ curl -i http://esp32-hrv.arl/
HTTP/1.1 200 OK
content-disposition: inline
etag: 1734923401
last-modified: Mon, 23 Dec 2024 02:52:34 GMT
cache-control: no-cache
connection: close
accept-ranges: none
transfer-encoding: chunked
content-type: text/html

...

I confirm that:

• I have read the documentation.
• I have searched for similar discussions.
• I have searched for similar issues.
• I have looked at the examples.
• I have upgraded to the lasted version of ESPAsyncWebServer (and AsyncTCP for ESP32).

[mathieucarbou]

Thanks! That’s true, any template should cancel caching

[mathieucarbou]

@JosePineiro: did you consider the template feature in your refactoring ? Wondering if this might has caused any impact.

@m-z-b : did you test with previous version to determine if this is a recent bug introduced by a refactoring ?

Thank you both of you !

[JosePineiro]

In principle, I only use etag when the files are pre-compressed. In this case, templates are never processed.
Even so, I don't rule out the possibility of a bug.
Please give me a couple of days to review and find out where the buffer is coming from.

[m-z-b]

Broken as far back as I could find a release online.... Suspect ever since LastModified and ETag headers were introduced.

Tried the earliest 3.x release I could find and it had the same issue.

Worked correctly in the (discontinued) 1.2.4 release I was using previously since it produced neither header:-)

Logic of void AsyncStaticWebHandler::handleRequest(AsyncWebServerRequest *request) suggest 304 response can be generated and both headers will be added even if a template processor callback exists.

Note: The default gzip logic prints a warning message on every GET request if the .gz file does not exist unless SetTryGzipFirst(false). Not sure if this verbosity was intentional.

[mathieucarbou]

thanks! definitely a new bug to fix then!

[JosePineiro]

I have been able to reproduce the described behavior.
I've located it in the function: void AsyncStaticWebHandler::handleRequest(AsyncWebServerRequest *request)
This function always sets an Etag based on the file size or the last modification date. It doesn't check whether there is a template processor or not.
I don't know the original intent: serveStatic implies that these are static files and therefore should be cached.
But on the other hand, it doesn't seem to make sense to add a TemplateProcessor to a static file (logically, it's no longer static).

Possible solutions:

• Understand that serveStatic always refers to static files and therefore disable the TemplateProcessor. This solution will significantly change the current behavior and may confuse some users.
• Check if the _tempFile file has templates. If so, disable eTag. This would slow down the handleRequest function, which I believe defeats the purpose of serveStatic.
• Check if _callback != null. In that case, disable eTag. I think this is the best solution.

@mathieucarbou
Please choose one of these options, and I'll try to implement it.

Once the issue is resolved, we can open a discussion to calmly decide what would be the best way to improve the performance of this library.

[mathieucarbou]

@me-no-dev : your input is required.

But on the other hand, it doesn't seem to make sense to add a TemplateProcessor to a static file (logically, it's no longer static).

I completely agree... serveStatic should not be used with templates since, as its name implies, it serves static content which does not change and can be cached. A template is dynamic this cannot be cached, or, if it can, it has to be by the user because it will depend on some application logic to set the expiration hat only the user can know about.

Since keeping API compatibility is required, I also think that the third option is best for the moment...

Once the issue is resolved, we can open a discussion to calmly decide what would be the best way to improve the performance of this library.

👍

@JosePineiro @me-no-dev : fyi I am in the process this summer of moving from a house to another so my availability in coding will be really limited - that's why I can only have time to review and do releases for now.

[JosePineiro]

I'm currently traveling. But I hope to have some free time tomorrow and try out the fix for this bug.

[m-z-b]

I think the third option is the best one... but I'm concerned that a key use case for using a template processor on a micro-controller is being forgotten.

I have an ESP32 used in an IoT context to display / update configuration and sensor data. Each of these pages is static content except for a few values from the current configuration and the current settings. i.e. there is a set of static templates to which values are filled in.

serveStatic() with a template processor is the simplest way of accomplishing this - one statement and a template processor. i.e.

 webServer.serveStatic("/", LittleFS, "/status/")
    .setTryGzipFirst(false)  // I think false would be a better default for this use case
    .setDefaultFile("index.html")
    .setTemplateProcessor(StatusProcessor);

It would be possible to add a serveStaticTemplate() to express this, but I think the implementation would still find its way into void AsyncStaticWebHandler::handleRequest()

The implementation could look something like this:

void AsyncStaticWebHandler::handleRequest(AsyncWebServerRequest *request) {
  // Get the filename from request->_tempObject and free it
  String filename((char *)request->_tempObject);
  free(request->_tempObject);
  request->_tempObject = NULL;

  if (request->_tempFile != true) {
    request->send(404);
    return;
  }

  if (_callback) {    // TemplateProcessor - assume each response is different
    AsyncWebServerResponse* response = new AsyncFileResponse(request->_tempFile, filename, emptyString, false, _callback);
    if (response) {
      request->send(response);
    } else {
#ifdef ESP32
      log_e("Failed to allocate");
#endif
      request->abort();
    }
    return;
  }


  time_t lw = request->_tempFile.getLastWrite();  // get last file mod time (if supported by FS)
  // set etag to lastmod timestamp if available, otherwise to size
  String etag;
  ... (more existing code)

[JosePineiro]

I've modified the library to fix the bug you reported.
I've created an example to test the new way of working:

1. If you use:

webServer.serveStatic("/", SPIFFS, "/status/", "no-cache")
.setTryGzipFirst(true) // Try to serve gzip files first. The gzip files will be STATIC and CACHED (do not use TemplateProcessor)
.setDefaultFile("index.html"); // Default file to serve

NOTE: TemplateProcessor IS NOT defined
In this case, all files will be cached with an ETAG. The ETAG will be obtained from the CRC of the gz trailer (if *.gz), the date (if the file system is dated), or the size.

2. If you use:

  webServer.serveStatic("/", SPIFFS, "/status/", "no-cache")
      .setTryGzipFirst(true)                  // Try to serve gzip files first. The gz files ara STATIC and CACHED (not use TemplateProcessor)
      .setDefaultFile("index.html")           // Default file to serve
      .setTemplateProcessor(StatusProcessor); // Use processor to inject dynamic data

NOTE: TemplateProcessor IS defined
In this case, pre-compressed files (e.g., image.jpg.gz) will be cached using the trailer's CRC. Uncompressed files will be considered dynamic and will not be cached.

Here's an example of how it would work. It's designed for lolin_s3_mini and PlatformIO.
There's a Python script that automatically GZ-compresses files that don't have templates when generating the filesystem.

I hope this is a good solution for you. It maintains compatibility with the current code and is much more robust when it comes to caching.

Download it from: https://github.com/JosePineiro/test_serveStatic