search

quictls / openssl

TLS/SSL and crypto library with QUIC APIs
https://quictls.github.io/openssl
Apache License 2.0
366 stars 50 forks source link

Add quic patches to 3.1.0 #111

Closed wbl closed 1 year ago

wbl commented 1 year ago

Pretty sure I got all the version numbers and such in the rebase, but entirely possible I missed a few.

nibanks commented 1 year ago

From my testing so far, perf looks good. I just kicked off our full MsQuic CI automation here to fully test this.

tmshort commented 1 year ago

I did do the:

diff <(git show -U0 $commit1) <(git show -U0 $commit2)

command for each pair of commits on 3.0.8+quic and this PR (I scripted it, don't worry! 😄), and everything generally looks good. Other than the two things noted above, all I saw were context and whitespace differences.

tmshort commented 1 year ago

This is the fuzzer error:

clang-15: error: unsupported option '--with-fuzzer-lib=/usr/lib/libFuzzingEngine'

Looks to be an environment error; I wonder if it's because we don't have an up-to-date master branch or something?

wbl commented 1 year ago

I'll go fix the README issue and adjust the other commit Monday and push an updated one.

wbl commented 1 year ago

I've added in the README commits and further bumped the version in the README. I'll pass on messing with the EBDIC stuff since Todd doesn't think there is a need and I'm not quite sure I understand it.

pheiduck commented 1 year ago

I wonder if it's because we don't have an up-to-date master branch or something?

@tmshort Yep, This branch uses clang-15 and master clang-12 where the error does not occur.

nibanks commented 1 year ago

@wbl any ETA on this PR? Thanks!

wbl commented 1 year ago

I was hoping @tmshort would be able to look again to confirm I did the README right, or @kaduk be able to take a look. If you think its good to merge despite that, go ahead: I'm not quite sure what our norms are.

nibanks commented 1 year ago

I was hoping @tmshort would be able to look again to confirm I did the README right, or @kaduk be able to take a look. If you think its good to merge despite that, go ahead: I'm not quite sure what our norms are.

I would like one of them to take a look as well if possible.

nibanks commented 1 year ago

@tmshort or @kaduk would you be able to take a quick look? Thanks!

tmshort commented 1 year ago

Sorry; I'm not getting notifications (or they are being getting lost) for this repo.

tmshort commented 1 year ago

Still not worried about Fuzz. I will push the latest OpenSSL master branch, even though this repo won't use it. We will likely have to keep that one up to date as well.