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.

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

ngtcp2: Support the latest update key callback type #4735

Merged
merged 1 commit into from Dec 20, 2019

Conversation

@jay
Copy link
Member

jay commented Dec 18, 2019

  • Remove our cb_update_key in favor of ngtcp2's new
    ngtcp2_crypto_update_key_cb which does the same thing.

Several days ago the ngtcp2_update_key callback function prototype was
changed in ngtcp2/ngtcp2@42ce09c. Though it would be possible to
fix up our cb_update_key for that change they also added
ngtcp2_crypto_update_key_cb which does the same thing so we'll use that
instead.

Ref: ngtcp2/ngtcp2@42ce09c

Closes #xxxx


Disclaimer: I'm not yet using HTTP/3. This is an attempt to fix this CI error:

  CC       vquic/libcurl_la-ngtcp2.lo
vquic/ngtcp2.c:559:3: error: initialization of ‘int (*)(ngtcp2_conn *, uint8_t *, uint8_t *, uint8_t *, uint8_t *, uint8_t *, uint8_t *, const uint8_t *, const uint8_t *, size_t,  void *)’ {aka ‘int (*)(struct ngtcp2_conn *, unsigned char *, unsigned char *, unsigned char *, unsigned char *, unsigned char *, unsigned char *, const unsigned char *, const unsigned char *, long unsigned int,  void *)’} from incompatible pointer type ‘int (*)(ngtcp2_conn *, uint8_t *, uint8_t *, uint8_t *, uint8_t *, void *)’ {aka ‘int (*)(struct ngtcp2_conn *, unsigned char *, unsigned char *, unsigned char *, unsigned char *, void *)’} [-Werror=incompatible-pointer-types]
   cb_update_key, /* update_key */
   ^~~~~~~~~~~~~
vquic/ngtcp2.c:559:3: note: (near initialization for ‘ng_callbacks.update_key’)
@jay jay added the HTTP/3 label Dec 18, 2019
@jay jay requested a review from tatsuhiro-t Dec 18, 2019
@bagder

This comment has been minimized.

Copy link
Member

bagder commented Dec 19, 2019

👍 - this fixes the build and I tried curl --http3 successfully against https://nghttp2.org:4433/ several times.

@bagder
bagder approved these changes Dec 19, 2019
@tatsuhiro-t

This comment has been minimized.

Copy link
Contributor

tatsuhiro-t commented Dec 20, 2019

Now that secrets are stored into ngtcp2_conn object, curl no longer needs to store them. qs->rx_secret and qs->tx_secret can be removed.

- Remove our cb_update_key in favor of ngtcp2's new
  ngtcp2_crypto_update_key_cb which does the same thing.

Several days ago the ngtcp2_update_key callback function prototype was
changed in ngtcp2/ngtcp2@42ce09c. Though it would be possible to
fix up our cb_update_key for that change they also added
ngtcp2_crypto_update_key_cb which does the same thing so we'll use that
instead.

Ref: ngtcp2/ngtcp2@42ce09c

Closes #4735
@jay jay force-pushed the jay:fix_ngtcp2_update_key branch from 1c909e5 to 10121a4 Dec 20, 2019
@jay

This comment has been minimized.

Copy link
Member Author

jay commented Dec 20, 2019

Now that secrets are stored into ngtcp2_conn object, curl no longer needs to store them. qs->rx_secret and qs->tx_secret can be removed.

Ok thanks, I have just amended the proposed changes to remove tx/rx secret from quicsocket entirely.

@jay jay closed this in 10121a4 Dec 20, 2019
@jay jay merged commit 10121a4 into curl:master Dec 20, 2019
23 checks passed
23 checks passed
LGTM analysis: Python No code changes detected
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
FreeBSD FreeBSD:freebsd-11-3-snap Task Summary
Details
FreeBSD FreeBSD:freebsd-12-1 Task Summary
Details
FreeBSD FreeBSD:freebsd-13-0-snap Task Summary
Details
LGTM analysis: C/C++ No new or fixed alerts
Details
buildbot/curl_winssl_cross_x64 Build done.
Details
buildbot/curl_winssl_cross_x64_dbg Build done.
Details
buildbot/curl_winssl_cross_x86 Build done.
Details
buildbot/curl_winssl_cross_x86_dbg Build done.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
curl.curl Build #20191220.1 succeeded
Details
curl.curl (macos cmake openssl) macos cmake openssl succeeded
Details
curl.curl (macos default) macos default succeeded
Details
curl.curl (macos libssh2) macos libssh2 succeeded
Details
curl.curl (macos torture) macos torture succeeded
Details
curl.curl (ubuntu HTTP only) ubuntu HTTP only succeeded
Details
curl.curl (ubuntu sync resolver) ubuntu sync resolver succeeded
Details
curl.curl (ubuntu torture tests) ubuntu torture tests succeeded
Details
curl.curl (ubuntu w/o HTTP/SMTP/IMAP) ubuntu w/o HTTP/SMTP/IMAP succeeded
Details
curl.curl (ubuntu w/o IPv6) ubuntu w/o IPv6 succeeded
Details
curl.curl (unbuntu default) unbuntu default succeeded
Details
@jay jay deleted the jay:fix_ngtcp2_update_key branch Dec 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.