Skip to content

Conversation

@JosePineiro
Copy link

Fixes an regresive bug where Content-Type was not properly set when serving files, which specifically affected Firefox when using "view page source" functionality. The issue primarily manifested in Firefox's view-source feature, but the fix ensures consistent behavior across all browsers and use cases.

Changes:
Fixed missing contentType headers
Optimized string handling by replacing PSTR() with static constant

  • Optimized string handling by replacing snprintf_P with snprintf
  • Changed from dynamic buffer formatting to direct header addition using static template
  • Improved memory efficiency and code readability

Fixes an regresive bug where Content-Type was not properly set when serving files, which specifically affected Firefox when using "view page source" functionality.
The issue primarily manifested in Firefox's view-source feature, but the fix ensures consistent behavior across all browsers and use cases.

Changes:
Fixed missing contentType headers
Optimized string handling by replacing PSTR() with static constant
- Optimized string handling by replacing snprintf_P with snprintf
- Changed from dynamic buffer formatting to direct header addition using static template
- Improved memory efficiency and code readability
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Fixes a regression where the Content-Type header could be omitted when serving files (notably in Firefox’s view-source) and optimizes string handling by replacing PSTR calls with static constants.

  • Added T_inline and T_attachment literals and removed PSTR/snprintf_P usages
  • Inverted the contentType check to correctly set a default when none is provided
  • Updated fs.open calls to use explicit fs::FileOpenMode enum for clarity

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/literals.h Introduced T_inline and T_attachment constants for Content-Disposition headers
src/WebResponses.cpp Fixed empty contentType logic, swapped snprintf_P for snprintf and static templates
src/AsyncWebServerRequest.cpp Replaced mode string "r" with fs::FileOpenMode::read in fs.open
Comments suppressed due to low confidence (3)

src/WebResponses.cpp:737

  • There are no tests covering the case when contentType is empty. Consider adding a unit test to verify that the default Content-Type is set correctly.
  if (*contentType == '\0') {

src/WebResponses.cpp:748

  • [nitpick] The variable name buf is generic. A more descriptive name like dispositionHeaderBuf would improve readability.
    snprintf(buf, sizeof(buf), T_attachment, filename);

src/AsyncWebServerRequest.cpp:30

  • Using fs::FileOpenMode::read may affect compatibility with FS implementations expecting a mode string. Ensure all supported file systems accept this enum or update the documentation accordingly.
  File gzFile = fs.open(gzPath, fs::FileOpenMode::read);

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@me-no-dev me-no-dev merged commit 42d2aad into ESP32Async:main Jul 16, 2025
33 checks passed
@JosePineiro JosePineiro deleted the fix/send-content-type branch July 16, 2025 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants