refactor: optimize AsyncFileResponse handling #240
Conversation
This refactor improves AsyncFileResponse::AsyncFileResponse(File content, const String &path, const char *contentType, bool download, AwsTemplateProcessor callback) The updated implementation: - Replaces the use of String::endsWith() with a more efficient memcmp() check for gzip file suffix. - Fix the download of precompresed archives. The old behavior was wrong. - Simplifies content-disposition header generation using consistent logic for both inline and download cases. Overall, the refactor enhances performance slightly and ensures more consistent behavior when serving compressed downloadable files.
|
Async FileResponse(File content, const String &path, const char *content Type, bool download, AwsTemplateProcessor callback) In this case, a compressed file is downloaded with the name of an uncompressed file and without setting ContentEncoding = gzip. This PR adds ContentEncoding = gzip when necessary. |
|
This PR needs considerable testing before it get's approved. Maybe if it was split into string manipulations and gzip changes, it would have been easier to deal with. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Bug: In this case, a compressed file is downloaded with the name of an uncompressed file and without setting ContentEncoding = gzip. I've simplified the PR. Only one line has been changed, which fixes this bug. |
That's the corner use case I was talking about in a previous PR comment I suppose ?. I agree with it, though it changes a little the behavior ? Maybe in the case of a file manager where image.png sits next to image.png.gz and user wants to download image.png, not image.png.gz. In this case, image.png.gz will be downloaded in place of image.png ? I would vote in accepting this behavior change since most of the usage in a MCU is to put a gz fiel to be served compressed. |
There was a problem hiding this comment.
Pull Request Overview
This refactor optimizes the AsyncFileResponse constructor by improving gzip file handling and content-disposition header generation. The changes focus on fixing the logic for serving precompressed archives and enhancing performance through more efficient string comparisons.
- Removes the
!downloadcondition from gzip file detection logic to fix handling of precompressed downloadable files - Replaces inefficient
String::endsWith()calls withmemcmp()for better performance (based on description) - Simplifies content-disposition header generation for consistent inline/download behavior
|
@me-no-dev : can you please have a look again ? PR looks good to me. Thanks! |
This refactor improves AsyncFileResponse::AsyncFileResponse(File content, const String &path, const char *contentType, bool download, AwsTemplateProcessor callback) The updated implementation:
Overall, the refactor enhances performance slightly and ensures more consistent behavior when serving compressed downloadable files.