Fix memleak and reset option

[jjsaunier]

Mainly backported from lwthiker/curl-impersonate#221

[jjsaunier]

There is no issue section, so I'm starting the conversation here. On this fork, with http2 only, the response is truncated with some server, E.g: https://www.yelp.com/search/snippet?find_desc=SEATOWN%20ELECTRIC&find_loc=MUKILTEO%2C%20WA

[perklet]

Does it work with the unpatched curl? Also enabled and created an issue for this.

[jjsaunier]

Does it work with the unpatched curl?

With the lwthiker version, yes it's working as expected, the response is not truncated. I did some testing, but so far I didn't get which changes affect http2 that way

[perklet]

It seems that removing the br encoding, i.e. Accept-Encoding: gzip, deflate, can fix this issue. There could be some bug introduced while handling brotli lib.

I guess the compressed content-length was used as the uncompressed content-length somehow.

BTW, which version were you using? The curl-8.1.1 based 0.6x branch or the 8.5.0 based 0.7x branch?

[jjsaunier]

Interesting, I'm using patches for curl 8.5.0 so 0.7x

[perklet]

Wait, it seems that curl itself does not handle this correctly. Can you reporduce this?

curl -H 'Accept-Encoding: gzip, deflate, br' \
  --compressed \
  'https://www.yelp.com/search/snippet?find_desc=SEATOWN%20ELECTRIC&find_loc=MUKILTEO%2C%20WA'

[jjsaunier]

I get the correct body and I tested on 2 version

curl 7.81.0 (x86_64-pc-linux-gnu) libcurl/7.81.0 OpenSSL/3.0.2 zlib/1.2.11 brotli/1.0.9 zstd/1.4.8 libidn2/2.3.2 libpsl/0.21.0 (+libidn2/2.3.2) libssh/0.9.6/openssl/zlib nghttp2/1.43.0 librtmp/2.3 OpenLDAP/2.5.17
Release-Date: 2022-01-05

curl 8.6.1-DEV (aarch64-apple-darwin23.2.0) libcurl/8.6.1-DEV (SecureTransport) OpenSSL/3.2.1 zlib/1.2.12 brotli/1.1.0 zstd/1.5.6 libidn2/2.3.7 libssh2/1.11.0 nghttp2/1.61.0 librtmp/2.3 OpenLDAP/2.6.7
Release-Date: [unreleased]

EDIT

Also tried against latest stable

curl 8.7.1 (aarch64-apple-darwin23.4.0) libcurl/8.7.1 (SecureTransport) OpenSSL/3.3.0 zlib/1.2.12 brotli/1.1.0 zstd/1.5.6 libidn2/2.3.7 libssh2/1.11.0 nghttp2/1.61.0 librtmp/2.3 OpenLDAP/2.6.7
Release-Date: 2024-03-27

[perklet]

I tried 8.5.0 and it did not work.

[jjsaunier]

I have tried with a docker version

docker run --rm curlimages/curl:8.5.0 -H 'Accept-Encoding: gzip, deflate, br' \
  --compressed \
  'https://www.yelp.com/search/snippet?find_desc=SEATOWN%20ELECTRIC&find_loc=MUKILTEO%2C%20WA'

on the 8.5, 8.6 I'm able to reproduce the issue, from 8.7.1 works

8.7.1

curl 8.7.1 (x86_64-pc-linux-musl) libcurl/8.7.1 OpenSSL/3.1.4 zlib/1.3.1 brotli/1.1.0 libidn2/2.3.4 libpsl/0.21.2 libssh2/1.11.0 nghttp2/1.58.0
Release-Date: 2024-03-27

8.5.0

curl 8.5.0 (x86_64-pc-linux-musl) libcurl/8.5.0 OpenSSL/3.1.4 zlib/1.3 brotli/1.1.0 libidn2/2.3.4 libpsl/0.21.2 (+libidn2/2.3.4) libssh2/1.11.0 nghttp2/1.58.0

[perklet]

Thanks for confirming this, I also tried 8.7.1 and it works as well.

I searched the curl issue section, but didn't find the particular issue or fix. Maybe we should simply skip 8.5.0 and use 8.7.1. Actually I have started working on it in the merge-8.7.1 branch yesterday.

[jjsaunier]

Do you have any blockers or spot something that can be painful to update? if I can help let me know

[perklet]

AFAIK, it's a regular update from 8.5.0 to 8.7.1. However, I will be on vacation for a week celebrating labor day from tomorrow, if you are in a hurry, feel free to continue work on it.

I have pushed my latest changes, it can compile at least, but there are still some problems I want to solve:

1. The major change is within tool_getparam.c, it's almost a rewrite, and still not working, complaining about curl: option --tls-permute-extensions: is unknown, which should be fixed.
2. The Makefile.dist is removed. For windows, we need to use cmake to generate the targets in win/build.sh of curl-impersonate, see my notes in export.sh. Also related to Unknown CMake command openssl_check_symbol_exists for USE_ECH option curl-impersonate#65.
3. Add an option called CURLOPT_HTTP2_STREAM_EXCLUSIVE to resovle Add options to change h2 priority weight and exclusive value  curl-impersonate#60. Note the stream weight can be adjusted with existing option CURLOPT_HTTP2_STREAM_WEIGHT.

I have invited you as a collaborator of this fork, so you can directly work on it.

[jjsaunier]

Ok I will take a look on 1 & 3 thank you

[jjsaunier]

Is this dockerfile correct for build https://github.com/yifeikong/curl-impersonate/blob/main/docker/debian.dockerfile ?

Because I'm getting errors

33.85 vtls/openssl.c: In function 'ossl_connect_step1':
33.85 vtls/openssl.c:4101:5: error: implicit declaration of function 'SSL_CTX_set_extension_order' [-Werror=implicit-function-declaration]
33.85  4101 |     SSL_CTX_set_extension_order(backend->ctx, data->set.str[STRING_TLS_EXTENSION_ORDER]);
33.85       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
34.27 cc1: some warnings being treated as errors
34.27 make[2]: *** [Makefile:3373: vtls/libcurl_impersonate_chrome_la-openssl.lo] Error 1

While boringssl is correctly patched and configure for the build

[perklet]

The dockerfile never worked since forked from upstream, you are welcome to fix it :)

I mainly used macOS as my development platform.

[jjsaunier]

The compilation looks correct on docker (I mean, it run as it should), and the boringssl patch is also the latest which include SSL_CTX_set_extension_order. Are you muting those warning/errors at compile time?

can you show the command you run to build curl, the equivalent of https://github.com/yifeikong/curl-impersonate/blob/main/docker/debian.dockerfile#L89

The only fix I have implemented to fix the docker build at the moment, it's adding the zstd-dev package

[perklet]

For macOS, it's basically the INSTALL.md, and export the zstd paths, which was added recently.

export LDFLAGS="-L$(brew --prefix zstd)/lib" CPPFLAGS="-I$(brew --prefix zstd)/include"
cd build
../configure
make clean
make chrome-build
sudo gmake chrome-install

For Linux, you can take the github workflow as a reference. However, you do not need to build and link libz/zstd etc from source probably.

can you show the command you run to build curl, the equivalent of https://github.com/yifeikong/curl-impersonate/blob/main/docker/debian.dockerfile#L89

The most relevant part is:

https://github.com/yifeikong/curl-impersonate/blob/ace896b8a0ec4cf7d266209ed74ded30f55a7169/Makefile.in#L241-L280

When running locally

https://github.com/yifeikong/curl-impersonate/blob/ace896b8a0ec4cf7d266209ed74ded30f55a7169/.github/workflows/build-and-test-make.yml#L105-L110

When running in GitHub actions.

the boringssl patch is also the latest

You need to make sure libcurl links to the patched boringssl.

[perklet]

The ./configure script is nested. First, you configure the curl-impersonate project, to generate a Makefile from Makefile.in. Then you call make chrome-build, which will call the configure/cmake/make of each components. Options like --with-zstd were passed from the outer configure to inner one. It's quite confusing at the first glance.