Skip to content
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

WebServer / StaticRequestHandler with mixed gz and non-gz contents #10824

Open
1 task done
tobozo opened this issue Jan 7, 2025 · 9 comments
Open
1 task done

WebServer / StaticRequestHandler with mixed gz and non-gz contents #10824

tobozo opened this issue Jan 7, 2025 · 9 comments
Labels
Type: Feature request Feature request for Arduino ESP32

Comments

@tobozo
Copy link
Contributor

tobozo commented Jan 7, 2025

Board

any

Device Description

any esp32 device

Hardware Configuration

any configuration using WebServer library

Version

3.1.0 (also on master)

IDE Name

Arduino IDE (any)

Operating System

Linux

Flash frequency

n/a

PSRAM enabled

yes

Upload speed

n/a

Description

Currentl StaticRequestHandler doesn't serve index.htm.gz file when index.htm exists.

&& !_fs.exists(path) prevents gz content to be served: when both compressed and uncompressed versions of same file are present, the uncompressed version is served.

if (!path.endsWith(FPSTR(mimeTable[gz].endsWith)) && !_fs.exists(path)) {

Moreover, StaticRequestHandler::handle() completely ignores the 'Accept-Encoding' header sent by the client, which should be checked for gzip and/or deflate before deciding if it is worth looking for a compressed version of the file.

Sketch

server.serveStatic("/", LittleFS, "/");

Debug Message

filesystem contents:

File: /index.htm     29878 bytes
File: /index.htm.gz  13698 bytes

current behaviour:

$ curl -I -H 'Accept-Encoding: gzip,deflate' http://192.168.1.13
HTTP/1.1 200 OK
Content-Type: text/html
Content-Length: 29878
Connection: close

expected behaviour:

$ curl -I -H 'Accept-Encoding: ' http://192.168.1.13
HTTP/1.1 200 OK
Content-Type: text/html
Content-Encoding: gzip
Content-Length: 13698
Connection: close

other expected behaviour (no accept-encoding headers):

$ curl -I http://192.168.1.13
HTTP/1.1 200 OK
Content-Type: text/html
Content-Length: 29878
Connection: close

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

  • I confirm I have checked existing issues, online documentation and Troubleshooting guide.
@tobozo tobozo added the Status: Awaiting triage Issue is waiting for triage label Jan 7, 2025
@me-no-dev
Copy link
Member

This is on purpose. The point is to be able to edit the HTML and when you are happy to turn it into gzip. Legacy from the FSBrowser. Firmware comes with some standard gzipped index and you can add your own uncompressed one through the editor/upload

@tobozo
Copy link
Contributor Author

tobozo commented Jan 7, 2025

This logic only makes sense with FSBrowser. and it breaks the principles of content negociation (should use the "best-suited" method in every situation).

@me-no-dev
Copy link
Member

I guess you are proposing to parse the Accept-Encoding header, as well as returning the GZIP by default (if present)?

@tobozo
Copy link
Contributor Author

tobozo commented Jan 7, 2025

yup, I'm not sure how to do that without affecting the FSBrowser editing workflow though

that's what I'm using in a custom handler, I also have to add Accept-Encoding via server.collectHeaders():

    bool hasGz = false;

    if( server.hasHeader("Accept-Encoding") ) {
      String acceptEncoding = server.header("Accept-Encoding");
      if( acceptEncoding.indexOf("gzip") > 0 || acceptEncoding.indexOf("deflate") > 0 ) {
        hasGz = _fs.exists(pathWithGz);
      }
    }

    if( hasGz ) {
      path = pathWithGz;
    }

@Jason2866 Jason2866 added Type: Feature request Feature request for Arduino ESP32 and removed Status: Awaiting triage Issue is waiting for triage labels Jan 8, 2025
@tobozo
Copy link
Contributor Author

tobozo commented Jan 10, 2025

Adding more feedback here as I found another anomaly in WebServer class:

WebServer::_prepareHeader() selects Chunked transfer encoding when the content length is unknown.

} else if (_contentLength == CONTENT_LENGTH_UNKNOWN && _currentVersion) { //HTTP/1.1 or above client
//let's do chunked
_chunked = true;
sendHeader(String(F("Accept-Ranges")), String(F("none")));
sendHeader(String(F("Transfer-Encoding")), String(F("chunked")));
}

While this is correct with HTTP 1.1, it isn't valid with HTTP 1.0.

With HTTP 1.0 the Content-Length header is optional: chunked transfer encoding is only available since HTTP 1.1.

@me-no-dev
Copy link
Member

we can not support requests with unknown lengths in HTTP1.0. Either send the length with 1.0 or use chunked with 1.1

@tobozo
Copy link
Contributor Author

tobozo commented Jan 10, 2025

Cloudflare does it.

This is implicit in both HTTP/1.0 and HTTP/1.1: a server response without content-length ends when the stream closes.

https://datatracker.ietf.org/doc/html/rfc1945#section-7.2.2

If a Content-Length header field is present, its value in bytes represents the length of the Entity-Body. Otherwise, the body length is determined by the closing the connection by the server.

Moreover, a HTTP/1.1 response without content-length is not necessarily chunked, it can be multipart/byteranges or support other (!=identity) transfer encodings.

https://datatracker.ietf.org/doc/html/rfc2616#section-4.4

2.If a Transfer-Encoding header field (section 14.41) is present and has any value other than "identity", then the transfer-length is defined by use of the "chunked" transfer-coding (section 3.6), unless the message is terminated by closing the connection.

@me-no-dev
Copy link
Member

Seems you have done quite a bit of research. Why not make a PR with the changes?

@tobozo
Copy link
Contributor Author

tobozo commented Jan 10, 2025

Why not make a PR with the changes?

Because I'm biased by my use case (to mimic apache's mod_gzip+mod_cache) and therefore at risk of bloating the library with maybe-unnecessary features.

In my use case, skipping the content-length header only makes sense when streaming content of unknown size generated on the fly (such as gzip/deflate), so I overloaded/inherited instead of modifying the WebServer library:

  • created a custom StaticRequestHandler with mod_gzip + mod_cache logic
  • added missing support for HTTP_HEAD to that custom handler
  • created a derivated class GzWebServer from WebServer
  • implemented GzWebserver::gzStream() to build+send its own HTTP response without content-length
  • added stream encoder callback to GzWebServer size_t(Stream *input, size_t len, Stream* output)
  • added filesystem encoder callback to GzWebServer size_t(fs::FS&fs, const String &inputFilename, bool use_cache)
  • upgraded to 3.1.1 and figured out a new MiddleWare class was introduced 🤦

I need study that new middleware class anyway, I haven't figured out yet if it should be involded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature request Feature request for Arduino ESP32
Projects
Development

No branches or pull requests

3 participants