Content-Type header set to default when chunking static files

[iduquegit]

When serving small files (less than chunk size) the content-type header is passed to the browser. However, for large files it is set to default (text/html). I added the following line to make it work, but I'm not sure if it is the best way to do it.

esp_err_t PsychicResponse::sendChunk(uint8_t* chunk, size_t chunksize)
{
  /* Send the buffer contents as HTTP response chunk */
  ESP_LOGD(PH_TAG, "Sending chunk: %d", chunksize);
  httpd_resp_set_type(this->_request->request(), _contentType.c_str());   //<--- I added this line
  esp_err_t err = httpd_resp_send_chunk(request(), (char*)chunk, chunksize);
  if (err != ESP_OK) {
    ESP_LOGE(PH_TAG, "File sending failed (%s)", esp_err_to_name(err));

    /* Abort sending file */
    httpd_resp_sendstr_chunk(this->_request->request(), NULL);
  }

  return err;
}

Thank you, this is finally a good async server that does not hang the ESP32 :)

[mathieucarbou]

Thank you, this is finally a good async server that does not hang the ESP32 :)

What are the ones you tried ?

[iduquegit]

What are the ones you tried ?

I have a configuration web page that I have to serve over ethernet or WiFi and connecting to the ESP hotspot. I've tried different forks of ESPAsyncWebServer, using both AsyncTCP and AsyncTCPSock with very poor performance (I'm getting out of RAM and most of the times the browser didn't get the required files).

The second best option I was using is your fork mathieucarbou/ESPAsyncWebServer @ 3.6.0, serving manually the files and chunking the responses if the file was big enough (>20K). But even with this I'm getting out of RAM pretty quickly and it gets worse over the time (maybe some memory is not freed correctly in some cases).

I also tried to build a queue of requests and serve the files in a different task with no luck (how to do this properly, probably exceeds my knowledge).

For the moment PsychicHttp is the best in terms of RAM usage and I can see that is closing the connections when not needed.

Thank you for the hard work on both projects.

[mathieucarbou]

But even with this I'm getting out of RAM pretty quickly and it gets worse over the time (maybe some memory is not freed correctly in some cases).

Ok! Probably a usage issue in the app then. ESPAsyncWS is capable of serving huge files concurrently or big files from sdcard or flash with only using a minimal heap. There are examples of that in the project. In any case, if you want, you can open a ticket in the discussion tab to show your use case and what was not correctly working. It might be possible you were not using the right endpoint to do what you wanted. Some endpoints are doing string copy in a buffer because the data would become out of scope from caller side. Some endpoints are using references to avoid allocating.

and I can see that is closing the connections when not needed.

yeah, like both ;-)

About your request, you can look at how PsychicFileResponse::send() is implemented.

sendHeaders(); is called first, then a sendChunk() in a loop.

sendChunk() is just sending by batches the bytes to the browser. HTTP headers are sent first, so no need to reset them again they won't be sent.

[iduquegit]

It might be possible you were not using the right endpoint to do what you wanted.

That makes sense, I will take a closer look a that thanks for the clue :)

About your request, you can look at how PsychicFileResponse::send() is implemented.

I see how it's implemented, and all the headers are sent correctly, except for the Content-Type when the response is chunked. I'll try to explain it more clearly, as I believe there is some missing code. Take a look at the arrows in the code (<--------) for reference.

1. File not chunked

We send the file with PsychicResponse::send().

esp_err_t PsychicResponse::send()
{
  // esp-idf makes you set the whole status.
  sprintf(_status, "%u %s", _code, http_status_reason(_code));
  httpd_resp_set_status(_request->request(), _status);

  // set the content type
  httpd_resp_set_type(_request->request(), _contentType.c_str()); // <-------- Here Content Type is set before sending headers

  // our headers too
  this->sendHeaders();

  // now send it off
  esp_err_t err = httpd_resp_send(_request->request(), getContent(), getContentLength());

  // did something happen?
  if (err != ESP_OK)
    ESP_LOGE(PH_TAG, "Send response failed (%s)", esp_err_to_name(err));

  return err;
}

2. Chunked file

In this case we send the headers and then loop with sendChunk()

esp_err_t PsychicResponse::sendChunk(uint8_t* chunk, size_t chunksize)
{
  /* Send the buffer contents as HTTP response chunk */
  ESP_LOGD(PH_TAG, "Sending chunk: %d", chunksize);
  esp_err_t err = httpd_resp_send_chunk(request(), (char*)chunk, chunksize);
  if (err != ESP_OK) {
    ESP_LOGE(PH_TAG, "File sending failed (%s)", esp_err_to_name(err));

    /* Abort sending file */
    httpd_resp_sendstr_chunk(this->_request->request(), NULL);
  }

  return err;
}

If you look at httpd_resp_send_chunk it says:

If no status code and content-type were set, by default this will send 200 OK status code and content type as text/html. You may call the following functions before this API to configure the response headers httpd_resp_set_status() - for setting the HTTP status string, httpd_resp_set_type() - for setting the Content Type, httpd_resp_set_hdr() - for appending any additional field value entries in the response header

I believe we need to set the Content Type using httpd_resp_set_type() before sending the headers. I think I’ve found a better place to put this than what I suggested in my previous comment.

    /* Retrieve the pointer to scratch buffer for temporary storage */
    char* chunk = (char*)malloc(FILE_CHUNK_SIZE);
    if (chunk == NULL) {
      ESP_LOGE(PH_TAG, "Unable to allocate %" PRIu32 " bytes to send chunk", FILE_CHUNK_SIZE);
      httpd_resp_send_err(request(), HTTPD_500_INTERNAL_SERVER_ERROR, "Unable to allocate memory.");
      return ESP_FAIL;
    }
    
    // set the content type
    httpd_resp_set_type(request(), getContentType().c_str());       // <-------- Here we need to set the Content Type before sending headers

    sendHeaders();

    size_t chunksize;
    do {
      /* Read file in chunks into the scratch buffer */
      chunksize = _content.readBytes(chunk, FILE_CHUNK_SIZE);
      if (chunksize > 0) {
        err = sendChunk((uint8_t*)chunk, chunksize);
        if (err != ESP_OK)
          break;
      }

      /* Keep looping till the whole file is sent */
    } while (chunksize != 0);

[mathieucarbou]

yes indeed... would you mind sending a PR ? Your solution makes sense I think and is also as per the doc.

I will merge your PR after.

Note: you are using v2 right ?

[iduquegit]

Sorry, I've been busy these days. I just sent the PR setting the status too.

Note: you are using v2 right ?

yes

[hoeken]

Fixed in PR #207