-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
auracle fail with libcurl 8.7.1 #13209
Comments
CC @inglor |
CC @icing (for commit authorship) |
Browsing the auracle sources, I do not see how exactly the changed write handling can lead to a CURLE_WRITE_ERROR. The callback in auracle is quite simple and never returns an error. I think we need an auracle debug handler that emits more of curls messages in order to find out what is going wrong here. Any chances of giving that a try, @inglor? |
Oh,
The same with
|
I can reproduce what's likely the same issue with this command:
docker run --rm curlimages/curl -v --compressed 'https://aur.archlinux.org/rpc?v=5&type=info&arg'
which results in
curl: (23) Failed writing received data to disk/application
The key is the addition of --compressed which results in Brotli being
negotiated and used.
|
Could you test that theory by modifying the auracle source to comment out the |
Confirmed to work:
|
I've also confirmed that adding -H 'Accept-Encoding: zstd' or -H
'Accept-Encoding: gzip' alongside --compressed also fixes the problem.
The last commit to touch Brotli directly was
d7b6ce6 but that code doesn't show this
problem.
|
I was able to bisect the problem to this commit:
commit 463472a (HEAD, refs/bisect/bad)
Author: Stefan Eissing ***@***.***>
Date: Wed Feb 7 12:05:05 2024 +0100
lib: move client writer into own source
Refactoring of the client writer that passes the data to the
client/application's callback functions.
- split out into own source cw-out.[ch] from sendf.c
- move tempwrite and tempcount from data->state into the context of the
client writer
- redesign the 3 tempwrite dynbufs as a linked list of dynbufs. On
paused transfers, this allows to "record" interleaved HEADER/BODY
chunks to be "played back" in the same order on unpausing.
- keep the overall size limit of all buffered data to DYN_PAUSE_BUFFER.
On exceeding that, return CURLE_TOO_LARGE instead of
CURLE_OUT_OF_MEMORY as before.
- add method to be called when a transfer is DONE to allow writing of
any data still buffered
- when paused, record HEADER writes exactly as they come for later
playback. HEADERs are documented to be written one-by-one.
Closes #12898
|
I had bisected already, see initial post. But we came to the same conclusion. 👍 |
I missed that line completely! At least it's confirmed… |
Yes, definitely no wasted time. I had thought about doing another |
It's this Line 676 in c77bdf1
Wondering if this is correct... Is an ended stream a write error here? I guess no. |
Is this correct by any chance? diff --git a/lib/content_encoding.c b/lib/content_encoding.c
index c1abf24e8..03452fee0 100644
--- a/lib/content_encoding.c
+++ b/lib/content_encoding.c
@@ -673,7 +673,7 @@ static CURLcode brotli_do_write(struct Curl_easy *data,
return Curl_cwriter_write(data, writer->next, type, buf, nbytes);
if(!bp->br)
- return CURLE_WRITE_ERROR; /* Stream already ended. */
+ return CURLE_OK; /* Stream already ended. */
decomp = malloc(DSIZ);
if(!decomp) |
Am 28.03.2024 um 10:13 schrieb Christian Hesse ***@***.***>:
Is this correct by any chance?
diff --git a/lib/content_encoding.c b/lib/content_encoding.c
index c1abf24e8..03452fee0 100644
--- a/lib/content_encoding.c
+++ b/lib/content_encoding.c
@@ -673,7 +673,7 @@ static CURLcode brotli_do_write(struct Curl_easy *data,
return Curl_cwriter_write(data, writer->next, type, buf, nbytes);
if(!bp->br)
- return CURLE_WRITE_ERROR; /* Stream already ended. */
+ return CURLE_OK; /* Stream already ended. */
decomp = malloc(DSIZ);
if(!decomp)
Ah, you found it as well. There is a 0-length write at the end which the brotli encoding does not like. The fix I have in mind is slightly different. I'll have a PR soon.
… —
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were assigned.Message ID: ***@***.***>
|
- refs curl#13212 and curl#13209 - curl's transfer handling may write 0-length chunks at the end of the download with an EOS flag. (HTTP/2 does this commonly) - content encoders need to pass-through such a write and not count this as error in case they are finished decoding
Please see #13219 for a fix. |
I did this
I pushed
curl 8.7.1-1
to the Arch Linux testing repository earlier today. Turns out we have a regression with an AUR helper application auracle, that links againstlibcurl
. Sadly output is little to no useful only:Running
git bisect
I was pointed at 463472a being the first bad commit.Investigating the code I landed in src/aur/aur.cc line 468 where
msg
is assigned, andmsg->data.result
some lines below is23
, which isCURLE_WRITE_ERROR
. Withcurl 8.6.0
the result was0
orCURLE_OK
.From the commit message it is not obvious to me if the public api is supposed to change - I guess no. (Everything else should require a
soname
bump anyway.) So not sure what to blame here... Also possible that the api was misused and worked by chance only.I expected the following
The application should continue to work as expected, downloading its content with
libcurl
.curl/libcurl version
operating system
Arch Linux with testing repositories enabled
The text was updated successfully, but these errors were encountered: