-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
zig build: panic while downloading package #15590
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
Comments
Can you try it on https://github.com/hqnna/mocha? the codebase is more updated, though I had the same error. |
Thank you for the nice stack trace, fix coming along shortly. |
with --- a/lib/std/crypto/tls/Client.zig
+++ b/lib/std/crypto/tls/Client.zig
@@ -1059,7 +1059,7 @@ pub fn readvAdvanced(c: *Client, stream: anytype, iovecs: []const std.os.iovec)
if (frag1.len < second_len)
return finishRead2(c, first, frag1, vp.total);
- @memcpy(frag[0..in], first);
+ @memcpy(frag[0..first.len], first);
@memcpy(frag[first.len..][0..second_len], frag1[0..second_len]);
frag = frag[0..full_record_len];
frag1 = frag1[second_len..]; now I'm getting
Thanks for the report, I'll continue to work on this. |
Btw @truemedian, the TLS code can be more efficient if a larger output buffer is given to the read function: zig/lib/std/crypto/tls/Client.zig Lines 1171 to 1196 in d70853b
Currently the top branch is executing, but the happy path is the bottom one. |
Is there any wordaround for this issue? |
* When there is buffered cleartext, return it without calling the underlying read function. This prevents buffer overflow due to space used up by cleartext. * Avoid clearing the buffer when the buffered cleartext could not be completely given to the result read buffer, and there is some buffered ciphertext left. * Instead of rounding up the amount of bytes to ask for to the nearest TLS record size, round down, with a minimum of 1. This prevents the code path from being taken which requires extra memory copies. * Avoid calling `@memcpy` with overlapping arguments. closes #15590
* When there is buffered cleartext, return it without calling the underlying read function. This prevents buffer overflow due to space used up by cleartext. * Avoid clearing the buffer when the buffered cleartext could not be completely given to the result read buffer, and there is some buffered ciphertext left. * Instead of rounding up the amount of bytes to ask for to the nearest TLS record size, round down, with a minimum of 1. This prevents the code path from being taken which requires extra memory copies. * Avoid calling `@memcpy` with overlapping arguments. closes #15590
Unstable with TLS support failure. Ref: ziglang/zig#15590
I get exactly the same problem building a local sample app for the Zap library. The same app downloaded dependent packages and built correctly a few days ago, using an earlier zig version. |
Maybe it is worth to change the ReadError in Client.zig to also include the more fine grained TLS errors instead of generalizing them to TlsFailure? It would help to pinpoint a bit better what the cause is, and could be changed back again once we see that errors have been fixed. |
i'll de-dupe again :-) by closing the newly opened issues |
What would be more helpful here is the error return trace, which should include where the original error originated. |
Here is a trace from a debug build for BadReaderState.
And for TlsInitializationFailed
Also got this one:
TlsAlert
|
Looks like this is now fixed in latest master by: ab37ab3 With it package download works, If i revert it I get |
I cannot confirm that. The current downloadable zig master is My current output shows them all:
|
I managed to link the wrong commit... cc: @renerocksai |
Oh cool, thx for pointing that out. The currently downloadable version is 3 commits behind that one. So hopes are up again 😄! |
I built zig after this commit and cannot reproduce the problem anymore. @renerocksai: zap works as expected again, too. |
Errors still exist for me, unfortunately. |
unable to reproduce on my machine with commit ab37ab3 |
I still have sigsegv using 0.11.0-dev.3312+ab37ab33c |
Having a |
|
Thanks for clarification. I have to figure out how to enable TLS 1.3, in theory it's active but it seems like nginx doesn't like it right now |
Original bug was resolved. What remains is a duplicate of #14174. |
Zig Version
0.11.0-dev.2986+012f9a97e
Steps to Reproduce and Observed Behavior
Expected Behavior
hanna/mocha tests to be run
The text was updated successfully, but these errors were encountered: