-
Notifications
You must be signed in to change notification settings - Fork 63
Fix ETag handling for dynamic template responses #271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
6665b47
a16b756
dcf0491
d1b0868
d41b258
bc2a00c
d7e91bc
44e9fde
22cc69d
aabdb77
72ab521
9e83287
04fd5e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -187,75 +187,107 @@ bool AsyncStaticWebHandler::_searchFile(AsyncWebServerRequest *request, const St | |
return found; | ||
} | ||
|
||
/** | ||
* @brief Handles an incoming HTTP request for a static file. | ||
* | ||
* This method processes a request for serving static files asynchronously. | ||
* It determines the correct ETag (entity tag) for caching, checks if the file | ||
* has been modified, and prepares the appropriate response (file response or 304 Not Modified). | ||
* | ||
* @param request Pointer to the incoming AsyncWebServerRequest object. | ||
*/ | ||
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; | ||
request->_tempObject = nullptr; | ||
|
||
if (request->_tempFile != true) { | ||
request->send(404); | ||
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; | ||
if (lw) { | ||
setLastModified(lw); | ||
#if defined(TARGET_RP2040) || defined(TARGET_RP2350) || defined(PICO_RP2040) || defined(PICO_RP2350) | ||
// time_t == long long int | ||
constexpr size_t len = 1 + 8 * sizeof(time_t); | ||
char buf[len]; | ||
char *ret = lltoa(lw ^ request->_tempFile.size(), buf, len, 10); | ||
etag = ret ? String(ret) : String(request->_tempFile.size()); | ||
#elif defined(LIBRETINY) | ||
long val = lw ^ request->_tempFile.size(); | ||
etag = String(val); | ||
#else | ||
etag = lw ^ request->_tempFile.size(); // etag combines file size and lastmod timestamp | ||
#endif | ||
// Get server ETag. If file is not GZ and we have a Template Processor, ETag=0 | ||
char etag[9]; | ||
const char *tempFileName = request->_tempFile.name(); | ||
const size_t lenFilename = strlen(tempFileName); | ||
|
||
if (lenFilename > T__GZ_LEN && memcmp(tempFileName + lenFilename - T__GZ_LEN, T__gz, T__GZ_LEN) == 0) { | ||
//File is a gz, get etag from CRC in trailer | ||
if (!AsyncWebServerRequest::_getEtag(request->_tempFile, etag)) { | ||
// File is corrupted or invalid | ||
log_e("File is corrupted or invalid: %s", tempFileName); | ||
request->send(404); | ||
return; | ||
} | ||
JosePineiro marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Reset file position to the beginning so the file can be served from the start. | ||
request->_tempFile.seek(0); | ||
} else if (_callback == nullptr) { | ||
// We don't have a Template processor | ||
uint32_t etagValue; | ||
time_t lastWrite = request->_tempFile.getLastWrite(); | ||
if (lastWrite > 0) { | ||
// Use timestamp-based ETag | ||
etagValue = static_cast<uint32_t>(lastWrite); | ||
} else { | ||
// No timestamp available, use filesize-based ETag | ||
size_t fileSize = request->_tempFile.size(); | ||
etagValue = static_cast<uint32_t>(fileSize); | ||
} | ||
snprintf(etag, sizeof(etag), "%08x", etagValue); | ||
} else { | ||
#if defined(TARGET_RP2040) || defined(TARGET_RP2350) || defined(PICO_RP2040) || defined(PICO_RP2350) || defined(LIBRETINY) | ||
etag = String(request->_tempFile.size()); | ||
#else | ||
etag = request->_tempFile.size(); | ||
#endif | ||
etag[0] = '\0'; | ||
} | ||
|
||
bool not_modified = false; | ||
AsyncWebServerResponse *response; | ||
|
||
// if-none-match has precedence over if-modified-since | ||
if (request->hasHeader(T_INM)) { | ||
not_modified = request->header(T_INM).equals(etag); | ||
} else if (_last_modified.length()) { | ||
not_modified = request->header(T_IMS).equals(_last_modified); | ||
} | ||
// Get raw header pointers to avoid creating temporary String objects | ||
const char *inm = request->header(T_INM).c_str(); // If-None-Match | ||
const char *ims = request->header(T_IMS).c_str(); // If-Modified-Since | ||
|
||
AsyncWebServerResponse *response; | ||
bool notModified = false; | ||
// 1. If the client sent If-None-Match and we have an ETag → compare | ||
if (*etag != '\0' && inm && *inm) { | ||
if (strcmp(inm, etag) == 0) { | ||
notModified = true; | ||
} | ||
} | ||
// 2. Otherwise, if there is no ETag and no Template processor but we have Last-Modified and Last-Modified matches | ||
else if (*etag == '\0' && _callback == nullptr && _last_modified.length() > 0 && ims && *ims && strcmp(ims, _last_modified.c_str()) == 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should respect There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please see bug #237. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The proposed code still fixes that bug. You have removed the call to |
||
log_d("_last_modified: %s", _last_modified.c_str()); | ||
log_d("ims: %s", ims); | ||
notModified = true; | ||
} | ||
|
||
if (not_modified) { | ||
if (notModified) { | ||
request->_tempFile.close(); | ||
response = new AsyncBasicResponse(304); // Not modified | ||
} else { | ||
response = new AsyncFileResponse(request->_tempFile, filename, emptyString, false, _callback); | ||
} | ||
|
||
if (!response) { | ||
if (!response) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a regression here -- this check must also applied to the |
||
#ifdef ESP32 | ||
log_e("Failed to allocate"); | ||
log_e("Failed to allocate"); | ||
#endif | ||
request->abort(); | ||
return; | ||
} | ||
|
||
response->addHeader(T_ETag, etag.c_str()); | ||
request->abort(); | ||
return; | ||
} | ||
|
||
if (_last_modified.length()) { | ||
response->addHeader(T_Last_Modified, _last_modified.c_str()); | ||
// Set ETag header | ||
if (*etag != '\0') { | ||
response->addHeader(T_ETag, etag, true); | ||
} | ||
// Set Last-Modified header | ||
if (_last_modified.length()) { | ||
response->addHeader(T_Last_Modified, _last_modified.c_str(), true); | ||
} | ||
} | ||
|
||
// Set cache control | ||
if (_cache_control.length()) { | ||
response->addHeader(T_Cache_Control, _cache_control.c_str()); | ||
response->addHeader(T_Cache_Control, _cache_control.c_str(), false); | ||
} else { | ||
response->addHeader(T_Cache_Control, T_no_cache, false); | ||
} | ||
|
||
request->send(response); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESP32 logging call must be #ifdef'd on ESP32.