Headers are corrupted on the client side

[hitecSmartHome]

I'm sending static content and settings some headers on it but it is corrupted on client side.

server->on("/index.html", HTTP_GET, [](PsychicRequest * request){
  if (request->hasHeader("If-None-Match") && request->header("If-None-Match") == String(etag2)) {
    PsychicResponse response304(request);
    response304.setCode(304);
    return response304.send();
  }
  PsychicResponse response(request);
  response.setContentType("text/html");
  response.addHeader("Content-Encoding", "gzip");
  response.addHeader("ETag", etag2);
  response.setContent(data2, sizeof(data2));
  return response.send();
});

I applied some debug logs to the lib but it looks OK.

void PsychicResponse::addHeader(const char *field, const char *value)
{
  //these get freed during send()
  HTTPHeader header;
  header.field = (char *)malloc(strlen(field)+1);
  header.value = (char *)malloc(strlen(value)+1);

  strlcpy(header.field, field, strlen(field)+1);
  strlcpy(header.value, value, strlen(value)+1);

  printf("Adding header: %s: %s\n", header.field, header.value);

  _headers.push_back(header);
}

Also in here

void PsychicResponse::sendHeaders()
{
  //get our global headers out of the way first
  for (HTTPHeader header : DefaultHeaders::Instance().getHeaders())
    httpd_resp_set_hdr(_request->request(), header.field, header.value);

  //now do our individual headers
  for (HTTPHeader header : _headers){
    printf("Sending header: %s: %s\n", header.field, header.value);
    esp_err_t resp = httpd_resp_set_hdr(this->_request->request(), header.field, header.value);

    if (resp != ESP_OK){
      printf("Failed to set header: %s: %s. Error: %s\n", header.field, header.value, esp_err_to_name(resp));
    }
  }

  // clean up our header variables after send
  for (HTTPHeader header : _headers)
  {
    free(header.field);
    free(header.value);
  }
  _headers.clear();
}

DEBUG LOGS

(9511) psychic: New client connected 51
(9513) httpd_uri: httpd_uri: URI '/' not found
Adding header: Content-Encoding: gzip
Adding header: ETag: cf77e49b1f78c9721d686cef353dfaf0
Sending header: Content-Encoding: gzip
Sending header: ETag: cf77e49b1f78c9721d686cef353dfaf0
(10015) httpd_uri: httpd_uri: URI '/favicon.ico' not found
Adding header: Content-Encoding: gzip
Adding header: ETag: cf77e49b1f78c9721d686cef353dfaf0
Sending header: Content-Encoding: gzip
Sending header: ETag: cf77e49b1f78c9721d686cef353dfaf0
(128908) httpd_uri: httpd_uri: URI '/' not found
Adding header: Content-Encoding: gzip
Adding header: ETag: cf77e49b1f78c9721d686cef353dfaf0
Sending header: Content-Encoding: gzip
Sending header: ETag: cf77e49b1f78c9721d686cef353dfaf0
(129416) httpd_uri: httpd_uri: URI '/favicon.ico' not found
Adding header: Content-Encoding: gzip
Adding header: ETag: cf77e49b1f78c9721d686cef353dfaf0
Sending header: Content-Encoding: gzip
Sending header: ETag: cf77e49b1f78c9721d686cef353dfaf0

[hitecSmartHome]

I don't understand how it is corrupted. Maybe some menuconfig option is not right.

[hitecSmartHome]

Maybe the max_resp_headers config for the server. Will test it

[mhaberler]

yeah looks like a knob in esp-idf

funny the chopping isnt reported

[hitecSmartHome]

It wasn't that. I set it to 100 and it still corrupted.

[hitecSmartHome]

[hitecSmartHome]

Browsing the menuconfig...

[hitecSmartHome]

@mhaberler Are you on arduino as a component of idf?

[hitecSmartHome]

Browsing the menuconfig...

Can't find anything...

[hitecSmartHome]

So there was a PR https://github.com/hoeken/PsychicHttp/pull/89 Which attempted to free the headers after sending. It then got reverted in this PR https://github.com/hoeken/PsychicHttp/pull/96 but did not merged.

I have removed these lines

for (HTTPHeader header : _headers)
  {
    free(header.field);
    free(header.value);
  }
  _headers.clear();

from the sendHeaders method and it started to work.

We probably have to free the headers after sent the whole response and not when the headers are sent.

[mhaberler]

great job! unfortunately underlines https://github.com/hoeken/PsychicHttp/issues/106

[hitecSmartHome]

Okay so if we are freeing the headers after the whole response is sent, it works like a charm

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);

    // 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));
    }

    // clean up our header variables after send
    for (HTTPHeader header : _headers) {
        free(header.field);
        free(header.value);
    }
    _headers.clear();

    return err;
}

[hitecSmartHome]

great job! unfortunately underlines #106

Yeah, @hoeken was so passionate about this porject. He was so active and developed this quickly. Sad that he also dissapeared.

[mhaberler]

let's see if the cleanup can be fitted into the PsychicResponse dtor, that'd be the better locus for cleanup

[hitecSmartHome]

Oh wait a minute. It is there in the dtor

PsychicResponse::~PsychicResponse() {
    // clean up our header variables.  we have to do this since httpd_resp_send doesn't store copies
    for (HTTPHeader header : _headers) {
        free(header.field);
        free(header.value);
    }
    _headers.clear();
}

wtf, there was duplicate code

[mhaberler]

yep, just noticed that too

[mhaberler]

so it can be taken out altogether

[mhaberler]

bingo, https + gzip works, congratulations!

[mhaberler]

looks as if part of https://github.com/hoeken/PsychicHttp/pull/89 was merged which introduced the duplicate header free

[mhaberler]

@hitecSmartHome I assume you are running your own fork

I'd propose to wait say a week and see if @hoeken comments

if not, it might make sense to collaborate on a fork

[mhaberler]

related: https://github.com/hoeken/PsychicHttp/pull/96

funnily https://github.com/hoeken/PsychicHttp/pull/96/commits/413125c98de78da38380fad563330a6fd4da8701 seems to NOT have caused a leak because it removed the dup free

[hitecSmartHome]

mhaberler

yes it would make sense and I'm in

[mhaberler]

I have the impression etags work now as well with https://github.com/BCsabaEngine/svelteesp32

re fork: I've penciled in Aug 2 to reconvene

[mhaberler]

@hitecSmartHome is this contact usable? https://www.hitecsmarthome.hu/en/products

drop me an email with coords at haberlerm at gmail

[proddy]

Yeah, @hoeken was so passionate about this porject. He was so active and developed this quickly. Sad that he also disappeared.

He's on the road a lot, as I recall, at sea sailing yachts from A to B. My guess is that he's in the middle of the Atlantic right now without internet.

[mhaberler]

hm, a fork can be undone by merging back

[mhaberler]

@proddy I see "and looking to collaborate on Secure HTTPS and MQTT servers for the ESP32 "in your profile - drop me an email with coords at haberlerm at gmail as well please

[hitecSmartHome]

Can we not use emails pls? Would be better with a gh discussion or a discord. It is more personal and real time. :D Also any attachment sending is significantly better.

[mhaberler]

should be taken care of by https://github.com/hoeken/PsychicHttp/commit/b17d8cc9a8d60f4d528293e6fe9d23352bbf3335 and works for me

@hitecSmartHome does this work for you as well?

[hitecSmartHome]

Will check

[hoeken]

Should work now, it's a reversion to the old code where the headers are freed in the destructor

On Wed, Aug 7, 2024, 11:02 HITEC @.***> wrote:

Will check

— Reply to this email directly, view it on GitHub https://github.com/hoeken/PsychicHttp/issues/108#issuecomment-2273813802, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABEHSXPE7JVHZUQWAWCXXTZQJAH7AVCNFSM6AAAAABLQDCB7SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZTHAYTGOBQGI . You are receiving this because you were mentioned.Message ID: @.***>

[hitecSmartHome]

Yep. It seems to work for me.

[mhaberler]

I guess you can close this issue now

[hitecSmartHome]

Oh sorry I tought it was closed.