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.

Sign up for GitHub

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

Jump to bottom

curl errors after too many redirects #11871

Closed
Joshix-1 opened this issue Sep 16, 2023 · 5 comments
Closed

curl errors after too many redirects #11871

Joshix-1 opened this issue Sep 16, 2023 · 5 comments
Assignees
@bagder
Labels
HTTP regression

Comments

@Joshix-1
Copy link

Joshix-1 commented Sep 16, 2023 •
edited

I did this

the command

curl -sSfLD - --max-redirs -1 -o /dev/null https://httpbin.org/absolute-redirect/1050

outputs the following (shortened):

HTTP/2 302 
date: Sat, 16 Sep 2023 20:37:42 GMT
content-type: text/html; charset=utf-8
content-length: 289
location: http://httpbin.org/absolute-redirect/1049
server: gunicorn/19.9.0
access-control-allow-origin: *
access-control-allow-credentials: true

...

HTTP/1.1 302 FOUND
Date: Sat, 16 Sep 2023 20:39:03 GMT
Content-Type: text/html; charset=utf-8
Content-Length: 283
Connection: keep-alive
Server: gunicorn/19.9.0
Location: http://httpbin.org/absolute-redirect/2
Access-Control-Allow-Origin: *
Access-Control-Allow-Credentials: true

HTTP/1.1 302 FOUND
Date: Sat, 16 Sep 2023 20:39:03 GMT
Content-Type: text/html; charset=utf-8
Content-Length: 283
Connection: keep-alive
Server: gunicorn/19.9.0
Location: http://httpbin.org/absolute-redirect/1
Access-Control-Allow-Origin: *
curl: (56) Too large response headers: 307226 > 307200

On a personal project with bigger response headers it exited more quickly (after less redirects). So I'm guessing that the size counter doesn't reset after a redirect.

I expected the following

--max-redirs -1 should work correctly. It should follow pointless redirects forever.

curl/libcurl version

curl 8.3.0 (x86_64-pc-linux-gnu) libcurl/8.3.0 OpenSSL/3.1.2 zlib/1.3 brotli/1.0.9 zstd/1.5.5 libidn2/2.3.4 libpsl/0.21.2 (+libidn2/2.3.4) libssh2/1.11.0 nghttp2/1.56.0
Release-Date: 2023-09-13
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS brotli GSS-API HSTS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM NTLM_WB PSL SPNEGO SSL threadsafe TLS-SRP UnixSockets zstd

could not reproduce on 7.76.1

operating system

Arch Linux

uname -a

Linux box 6.1.52-1-lts #1 SMP PREEMPT_DYNAMIC Thu, 07 Sep 2023 05:17:41 +0000 x86_64 GNU/Linux
@bagder bagder added the HTTP label Sep 16, 2023
@bagder
Copy link
Member

bagder commented Sep 16, 2023

That's not a "crash", it returns an error.

@bagder bagder self-assigned this Sep 16, 2023
@Joshix-1 Joshix-1 changed the title curl crashes after too many redirects curl errors after too many redirects Sep 16, 2023
@bagder
Copy link
Member

bagder commented Sep 16, 2023

Regression from 3ee79c1

bagder added a commit that referenced this issue Sep 16, 2023
@bagder
http: use the per-request header counter to check for too large headers
90d7dfc 
Not the counter that accumulates all headers over all redirects.

Yes: this means that if you allow following unbounded redirects in
never-ending loops, curl can run out of memory.

Fixes #11871
Reported-by: Joshix-1 on github
@bagder bagder mentioned this issue Sep 16, 2023
Closed
bagder added a commit that referenced this issue Sep 17, 2023
@bagder
http: use the per-request header counter to check for too large headers
fadb2e0 
Not the counter that accumulates all headers over all redirects.

Do a second check for 20 times the limit for the accumulated size for
all headers.

Fixes #11871
Reported-by: Joshix-1 on github
Closes #11872
bagder added a commit that referenced this issue Sep 18, 2023
@bagder
http: use per-request counter to check too large headers
12756db 
Not the counter that accumulates all headers over all redirects.

Follow-up to 3ee79c1

Do a second check for 20 times the limit for the accumulated size for
all headers.

Fixes #11871
Reported-by: Joshix-1 on github
Closes #11872
@bagder
Copy link
Member

bagder commented Sep 18, 2023

It should follow pointless redirects forever.

It will not do that. curl keeps headers in memory for easy access after the transfer is done, which thus implicitly limits how many times it can loop when following redirects.

@Joshix-1
Copy link
Author

Joshix-1 commented Sep 18, 2023 •
edited

Why does it need all the headers?

If it needs them, then that's fine with me, I just think it would need an error message mentioning that. (or an option to not keep all the headers in memory, if that isn't too much work for such a pointless thing [following redirects forever])

@bagder
Copy link
Member

bagder commented Sep 18, 2023

such a pointless thing

Your pointless is someone else's best feature. And vice versa.

No, it cannot be asked to not store the headers.

@bagder bagder closed this as completed in 2cb0d34 Sep 18, 2023
ptitSeb pushed a commit to wasix-org/curl that referenced this issue Sep 25, 2023
@bagder @ptitSeb
http: use per-request counter to check too large headers
a1c6b7e 
Not the counter that accumulates all headers over all redirects.

Follow-up to 3ee79c1

Do a second check for 20 times the limit for the accumulated size for
all headers.

Fixes curl#11871
Reported-by: Joshix-1 on github
Closes curl#11872
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bagder bagder

Labels
HTTP regression
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

http: use the per-request header counter to check for too large headers curl/curl
2 participants
@bagder @Joshix-1