quiche: update QUICHE to 4f552f349b8df000af24bc6cfa0b78fdc2467fef by danzh2010 · Pull Request #19013 · envoyproxy/envoy

danzh2010 · 2021-11-16T03:33:23Z

Update QUICHE from 0b75841d5 to 4f552f349
https://github.com/google/quiche/compare/0b75841d5..4f552f349

$ git log 0b75841d5..4f552f349 --date=short --no-merges --format="%ad %al %s"

2021-11-15 wub Deprecate --gfe2_reloadable_flag_quic_bbr_start_new_aggregation_epoch_after_a_full_round
2021-11-15 wub Deprecate --gfe2_reloadable_flag_quic_bbr2_check_cwnd_limited_before_aggregation_epoch.
2021-11-15 fayang Make QUIC_BUG of "Trying to start blackhole detection without no bytes in flight" server side only.
2021-11-15 bnc Deprecate --gfe2_reloadable_flag_quic_reject_invalid_chars_in_field_value.
2021-11-15 quiche-dev Consolidates half_closed_local tracking. This is preparation for a refactoring of stream close events.
2021-11-12 quiche-dev Fixes stream unregistration in OgHttp2Session::CloseStream().
2021-11-12 quiche-dev Adds unit tests demonstrating bugs in existing oghttp2 handling of stream close events.
2021-11-12 wub Change QuicCryptoServerConfig::ParseSourceAddressToken from taking a 'SourceAddressTokens*' to a 'SourceAddressTokens&'.  The function dereferences this pointer without checking for nullptr, changing it to reference prevents caller from passing in a nullptr.
2021-11-12 wub Add a regression test for b/206077990.
2021-11-12 quiche-dev Factors out `MaybeSendBufferedData()`, and moves the functionality into `SendQueuedFrames()`.
2021-11-12 fayang Set chrome_value false for gfe2_reloadable_flag_quic_add_cached_network_parameters_to_address_token.
2021-11-12 quiche-dev Add missing nullptr check.
2021-11-11 wub Add retry_token, resumption_attempted and early_data_attempted to quic::ParsedClientHello.
2021-11-11 quiche-dev Another small fix from debugging: only mark a request stream ready to write if it includes a body.
2021-11-11 quiche-dev Fixes frame length calculations to use the actual serialized length.
2021-11-10 dschinazi Add support for draft-ietf-quic-version-negotiation-05
2021-11-10 vasilvv Make sure WebTransport over HTTP/3 is enabled on the client even when SETTINGS_ENABLE_CONNECT_PROTOCOL is not present.
2021-11-09 dschinazi Refactor QUIC version downgrade prevention, part 2
2021-11-09 dschinazi Refactor QUIC version downgrade prevention
2021-11-09 dschinazi Fix a QuicConnection connection close log
2021-11-08 danzh Internal change
2021-11-08 fayang Internal change
2021-11-08 fayang In QUIC, do not check amplification limit if there is pending timer credit. This would guarantee CRYPTO frame be retransmitted because of 1) PTO fires 2) bundled with outgoing ACKs.
2021-11-05 fayang Set gfe2_reloadable_flag_quic_verify_request_headers_2 chrome_value to true.
2021-11-05 wub Allow QuicToyClient to provide a client certificate to the server, if requested.
2021-11-04 quiche-dev Automated g4 rollback of changelist 386316152.
2021-11-04 danzh Validate QUIC request/response headers against invalid token and disallowed headers. Add empty string to disallow-list. Split --gfe2_reloadable_flag_quic_verify_request_headers into 2 flags: --gfe2_reloadable_flag_quic_verify_request_headers_2 to validate QUIC request/response headers against invalid request with ratio monitoring; mark H2 request with empty string header as invalid earlier in H2 stack. --gfe2_reloadable_flag_quic_act_upon_invalid_header return error response upon any invalid QUIC request header.
2021-11-04 quiche-dev Call visitor_.OnInvalidFrame() for oghttp2 header errors.
2021-11-04 wub Add CachedNetworkParameters to address token for IETF QUIC, and - min_rtt received from a validated token will be used to set the initial rtt, if connection option 'TRTT' is set. - Enable bandwidth resumption for IETF QUIC when connection options BWRE or BWMX exists.
2021-11-03 renjietang Add connection option to trigger path degrading on 1 PTO.
2021-11-03 quiche-dev Add default return statements to switch statements to appease GCC.
2021-11-03 quiche-dev Wrap OgHttp2Session callbacks with a latched_error_ check.
2021-11-03 quiche-dev Add mock methods to MockSpdyFramerVisitor.
2021-11-02 quiche-dev Change Http2VisitorInterface::OnInvalidFrame() to accept an InvalidFrameError enum.
2021-11-02 vasilvv Perform header-based draft version negotiation in WebTransport over HTTP/3.
2021-11-02 haoyuewang Use absl::optional<StatelessResetToken> in place of a separate boolean and token on QuicConnection::PathState.
2021-11-02 dschinazi Internal change
2021-11-02 wub Add mTLS support for IETF QUIC.
2021-11-02 wub Internal change
2021-11-02 vasilvv Internal change
2021-11-02 haoyuewang Update quic::IsAppleMobile to quic::IsAppleClient for better coverage of Apple related QUIC reverse path validation crash: 1) Added coverage for iOS Youtube (from cr/406134264) & iMM (from Sherlog) user agents. 2) Added coverage for Mac traffic.

Risk Level: low
Testing: existing H3 tests passed

https://github.com/google/quiche/compare/0b75841d5..4f552f349 $ git log 0b75841d5..4f552f349 --date=short --no-merges --format="%ad %al %s" 2021-11-15 wub Deprecate --gfe2_reloadable_flag_quic_bbr_start_new_aggregation_epoch_after_a_full_round 2021-11-15 wub Deprecate --gfe2_reloadable_flag_quic_bbr2_check_cwnd_limited_before_aggregation_epoch. 2021-11-15 fayang Make QUIC_BUG of "Trying to start blackhole detection without no bytes in flight" server side only. 2021-11-15 bnc Deprecate --gfe2_reloadable_flag_quic_reject_invalid_chars_in_field_value. 2021-11-15 quiche-dev Consolidates half_closed_local tracking. This is preparation for a refactoring of stream close events. 2021-11-12 quiche-dev Fixes stream unregistration in OgHttp2Session::CloseStream(). 2021-11-12 quiche-dev Adds unit tests demonstrating bugs in existing oghttp2 handling of stream close events. 2021-11-12 wub Change QuicCryptoServerConfig::ParseSourceAddressToken from taking a 'SourceAddressTokens*' to a 'SourceAddressTokens&'. The function dereferences this pointer without checking for nullptr, changing it to reference prevents caller from passing in a nullptr. 2021-11-12 wub Add a regression test for b/206077990. 2021-11-12 quiche-dev Factors out `MaybeSendBufferedData()`, and moves the functionality into `SendQueuedFrames()`. 2021-11-12 fayang Set chrome_value false for gfe2_reloadable_flag_quic_add_cached_network_parameters_to_address_token. 2021-11-12 quiche-dev Add missing nullptr check. 2021-11-11 wub Add retry_token, resumption_attempted and early_data_attempted to quic::ParsedClientHello. 2021-11-11 quiche-dev Another small fix from debugging: only mark a request stream ready to write if it includes a body. 2021-11-11 quiche-dev Fixes frame length calculations to use the actual serialized length. 2021-11-10 dschinazi Add support for draft-ietf-quic-version-negotiation-05 2021-11-10 vasilvv Make sure WebTransport over HTTP/3 is enabled on the client even when SETTINGS_ENABLE_CONNECT_PROTOCOL is not present. 2021-11-09 dschinazi Refactor QUIC version downgrade prevention, part 2 2021-11-09 dschinazi Refactor QUIC version downgrade prevention 2021-11-09 dschinazi Fix a QuicConnection connection close log 2021-11-08 danzh Internal change 2021-11-08 fayang Internal change 2021-11-08 fayang In QUIC, do not check amplification limit if there is pending timer credit. This would guarantee CRYPTO frame be retransmitted because of 1) PTO fires 2) bundled with outgoing ACKs. 2021-11-05 fayang Set gfe2_reloadable_flag_quic_verify_request_headers_2 chrome_value to true. 2021-11-05 wub Allow QuicToyClient to provide a client certificate to the server, if requested. 2021-11-04 quiche-dev Automated g4 rollback of changelist 386316152. 2021-11-04 danzh Validate QUIC request/response headers against invalid token and disallowed headers. Add empty string to disallow-list. Split --gfe2_reloadable_flag_quic_verify_request_headers into 2 flags: --gfe2_reloadable_flag_quic_verify_request_headers_2 to validate QUIC request/response headers against invalid request with ratio monitoring; mark H2 request with empty string header as invalid earlier in H2 stack. --gfe2_reloadable_flag_quic_act_upon_invalid_header return error response upon any invalid QUIC request header. 2021-11-04 quiche-dev Call visitor_.OnInvalidFrame() for oghttp2 header errors. 2021-11-04 wub Add CachedNetworkParameters to address token for IETF QUIC, and - min_rtt received from a validated token will be used to set the initial rtt, if connection option 'TRTT' is set. - Enable bandwidth resumption for IETF QUIC when connection options BWRE or BWMX exists. 2021-11-03 renjietang Add connection option to trigger path degrading on 1 PTO. 2021-11-03 quiche-dev Add default return statements to switch statements to appease GCC. 2021-11-03 quiche-dev Wrap OgHttp2Session callbacks with a latched_error_ check. 2021-11-03 quiche-dev Add mock methods to MockSpdyFramerVisitor. 2021-11-02 quiche-dev Change Http2VisitorInterface::OnInvalidFrame() to accept an InvalidFrameError enum. 2021-11-02 vasilvv Perform header-based draft version negotiation in WebTransport over HTTP/3. 2021-11-02 haoyuewang Use absl::optional<StatelessResetToken> in place of a separate boolean and token on QuicConnection::PathState. 2021-11-02 dschinazi Internal change 2021-11-02 wub Add mTLS support for IETF QUIC. 2021-11-02 wub Internal change 2021-11-02 vasilvv Internal change 2021-11-02 haoyuewang Update quic::IsAppleMobile to quic::IsAppleClient for better coverage of Apple related QUIC reverse path validation crash: 1) Added coverage for iOS Youtube (from cr/406134264) & iMM (from Sherlog) user agents. 2) Added coverage for Mac traffic. Signed-off-by: Dan Zhang <danzh@google.com>

Signed-off-by: Dan Zhang <danzh@google.com>

repokitteh-read-only · 2021-11-16T03:33:29Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @moderation

🐱

Caused by: #19013 was opened by danzh2010.

see: more, trace.

rojkov · 2021-11-16T07:51:05Z

/assign @RyanTheOptimist

RyanTheOptimist

Looks good modulo the windows compile failure:

bazel-out\x64_windows-opt\bin\external\envoy\bazel\foreign_cc\nghttp2\include\nghttp2/nghttp2.h(930): error C2065: 'nghttp2_data_source_read_callback': undeclared identifier
bazel-out\x64_windows-opt\bin\external\envoy\bazel\foreign_cc\nghttp2\include\nghttp2/nghttp2.h(930): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
bazel-out\x64_windows-opt\bin\external\envoy\bazel\foreign_cc\nghttp2\include\nghttp2/nghttp2.h(930): error C2513: 'int': no variable declared before '='
bazel-out\x64_windows-opt\bin\external\envoy\bazel\foreign_cc\nghttp2\include\nghttp2/nghttp2.h(930): fatal error C1903: unable to recover from previous error(s); stopping compilation

I think Biren looked into exactly this error recently, fwiw.

RyanTheOptimist · 2021-11-16T14:45:31Z

bazel/repositories.bzl

        name = "com_github_google_quiche",
        genrule_cmd_file = "@envoy//bazel/external:quiche.genrule_cmd",
        build_file = "@envoy//bazel/external:quiche.BUILD",
-        patches = ["@envoy//bazel/external:quiche.patch"],


I don't think this is going to work. Are there some patch hunks that are failing to apply?

Unfortunately, I have to bring it back.

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 · 2021-11-16T16:51:22Z

@DavidSchinazi

moderation · 2021-11-16T17:01:30Z

/lgtm deps

RyanTheOptimist · 2021-11-16T18:38:47Z

bazel/external/quiche.patch

 #include "absl/types/span.h"
 #include "http2/adapter/data_source.h"
 #include "http2/adapter/http2_protocol.h"
+ #include "http2/adapter/http2_visitor_interface.h"


We're planning to back-port these patches to QUICHE, right?

This was added in QUICHE so I needed to adjust the patch lines here.

nod but I mean all of the diffs in this file. For example, are we planning to back-port the ssize_t typdef, the NO_ERROR and the default returns?

+1, why not simply ban use of ssize_t in QUICHE via a presubmit?

IIRC, it's used by nghttp somewhere that oghttp has to overload (or some such).

Yeah, nghttp2 uses ssize_t. Envoy actually has a similar workaround in envoy/common/platform.h.

I'm working on back-porting the fixes in the patch file. Some of these patches will be unnecessary in the next QUICHE update.

Awesome, that's great to hear!

RyanTheOptimist · 2021-11-16T18:42:15Z

test/common/quic/envoy_quic_session_cache_test.cc

-  params->version = FakeVersionLabel;
-  params->supported_versions.push_back(FakeVersionLabel);
-  params->supported_versions.push_back(FakeVersionLabel2);
+  params->legacy_version_information = quic::TransportParameters::LegacyVersionInformation();


Oh, interesting. I'm slightly surprised that this is required because I thought it was optional. But I guess it's going to be optional eventually, just not yet?

It is optional indeed. But this test uses this field.

legacy_version_information is absl::optional but our code (currently) requires it to be present - we'll remove it at a later date. @danzh2010's change is the correct one here.

danzh2010 · 2021-11-16T19:19:53Z

/retest

repokitteh-read-only · 2021-11-16T19:19:56Z

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #19013 (comment) was created by @danzh2010.

see: more, trace.

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 · 2021-11-17T01:23:39Z

@envoyproxy/dependency-shepherds

danzh2010 · 2021-11-17T16:28:17Z

@moderation, need your another stamp on new commits.

danzh2010 · 2021-11-17T19:24:13Z

/assign @envoyproxy/dependency-shepherds

repokitteh-read-only · 2021-11-17T19:24:16Z

@envoyproxy/dependency-shepherds cannot be assigned to this issue.

🐱

Caused by: a #19013 (comment) was created by @danzh2010.

see: more, trace.

moderation · 2021-11-17T19:32:52Z

/lgtm deps

danzh2010 · 2021-11-17T19:47:22Z

@RyanTheOptimist, mind taking another look?

RyanTheOptimist

LGTM!

RyanTheOptimist · 2021-11-17T19:48:59Z

/assign @alyssawilk

RyanTheOptimist · 2021-11-17T19:49:12Z

@alyssawilk for Sr Maintainer review

danzh1989 added 2 commits November 15, 2021 17:51

fix build

9c91a4b

Signed-off-by: Dan Zhang <danzh@google.com>

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Nov 16, 2021

repokitteh-read-only bot assigned moderation Nov 16, 2021

repokitteh-read-only bot assigned RyanTheOptimist Nov 16, 2021

RyanTheOptimist previously approved these changes Nov 16, 2021

View reviewed changes

mattklein123 added the waiting label Nov 16, 2021

bring back patch

6926d61

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 dismissed RyanTheOptimist’s stale review via 6926d61 November 16, 2021 16:44

repokitteh-read-only bot removed the waiting label Nov 16, 2021

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Nov 16, 2021

RyanTheOptimist reviewed Nov 16, 2021

View reviewed changes

Merge branch 'master' into updatetar19

89678b5

Signed-off-by: Dan Zhang <danzh@google.com>

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Nov 16, 2021

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Nov 17, 2021

RyanTheOptimist approved these changes Nov 17, 2021

View reviewed changes

repokitteh-read-only bot assigned alyssawilk Nov 17, 2021

alyssawilk approved these changes Nov 17, 2021

View reviewed changes

alyssawilk merged commit 1eb1841 into envoyproxy:main Nov 17, 2021

Conversation

danzh2010 commented Nov 16, 2021

Uh oh!

repokitteh-read-only bot commented Nov 16, 2021

Uh oh!

rojkov commented Nov 16, 2021

Uh oh!

RyanTheOptimist left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danzh2010 commented Nov 16, 2021

Uh oh!

moderation commented Nov 16, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danzh2010 commented Nov 16, 2021

Uh oh!

repokitteh-read-only bot commented Nov 16, 2021

Uh oh!

danzh2010 commented Nov 17, 2021

Uh oh!

danzh2010 commented Nov 17, 2021

Uh oh!

danzh2010 commented Nov 17, 2021

Uh oh!

repokitteh-read-only bot commented Nov 17, 2021

Uh oh!

moderation commented Nov 17, 2021

Uh oh!

danzh2010 commented Nov 17, 2021

Uh oh!

RyanTheOptimist left a comment

Choose a reason for hiding this comment

Uh oh!

RyanTheOptimist commented Nov 17, 2021

Uh oh!

RyanTheOptimist commented Nov 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants