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

Add disable_h2 and matching h5vcc.settings #3979

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

jellefoks
Copy link
Member

This adds a command-line switch disable_h2 that disables the HTTP/2 protocol (spdy), and a matching h5vcc.settings parameter 'HTTP2' (with backing by PersistentSettings) for run-time disabling of HTTP/2 and spdy. Similar to 'QUIC' and 'HTTP3', the setting takes effect immediately for new connections only.

This also ensures that when the command-line parameter disable_quic or the new disable_h2 is used, that that can not be overuled later with 5vcc.settings or from the corresponding PersistentSetting.

b/205134049

This adds a command-line switch `disable_h2` that disables the HTTP/2
protocol (spdy), and a matching `h5vcc.settings` parameter 'HTTP2' (with
backing by `PersistentSettings`) for run-time disabling of HTTP/2 and
spdy. Similar to 'QUIC' and 'HTTP3', the setting takes effect
immediately for new connections only.

This also ensures that when the command-line parameter `disable_quic`
or the new `disable_h2` is used, that that can not be overuled later
with `5vcc.settings` or from the corresponding `PersistentSetting`.

b/205134049
@jellefoks jellefoks requested a review from a team as a code owner August 13, 2024 18:34
@jellefoks jellefoks enabled auto-merge (squash) August 13, 2024 19:52
Copy link

codecov bot commented Aug 13, 2024

Codecov Report

Attention: Patch coverage is 24.39024% with 31 lines in your changes missing coverage. Please review.

Project coverage is 57.67%. Comparing base (20d2e12) to head (0b88c8d).
Report is 2 commits behind head on main.

Files Patch % Lines
cobalt/network/url_request_context.cc 33.33% 12 Missing ⚠️
cobalt/h5vcc/h5vcc_settings.cc 8.33% 11 Missing ⚠️
cobalt/network/network_module.cc 27.27% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3979      +/-   ##
==========================================
- Coverage   57.74%   57.67%   -0.08%     
==========================================
  Files        1768     1746      -22     
  Lines       86446    86295     -151     
==========================================
- Hits        49918    49768     -150     
+ Misses      36528    36527       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@kaidokert kaidokert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - would be good to have second quick review as well

@jellefoks jellefoks merged commit 2ca4fc9 into youtube:main Aug 13, 2024
321 of 325 checks passed
@kaidokert kaidokert added cp-25.lts.1+ Cherry Pick to the 25.lts.1+ branch cp-24.lts.1+ Cherry Pick to the 24.lts.1+ branch labels Aug 13, 2024
application_settings_[kProtoHTTP2] = {};
}
} else {
if (next_protos_.back() == kProtoHTTP2) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it guaranteed that HTTP2 is at the end of supported protocol list? Even for without our addition?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's only added here and in the constructor.

cobalt-github-releaser-bot pushed a commit that referenced this pull request Aug 13, 2024
This adds a command-line switch `disable_h2` that disables the HTTP/2
protocol (spdy), and a matching `h5vcc.settings` parameter 'HTTP2' (with
backing by `PersistentSettings`) for run-time disabling of HTTP/2 and
spdy. Similar to 'QUIC' and 'HTTP3', the setting takes effect
immediately for new connections only.

This also ensures that when the command-line parameter `disable_quic` or
the new `disable_h2` is used, that that can not be overuled later with
`5vcc.settings` or from the corresponding `PersistentSetting`.

b/205134049

(cherry picked from commit 2ca4fc9)
cobalt-github-releaser-bot pushed a commit that referenced this pull request Aug 13, 2024
This adds a command-line switch `disable_h2` that disables the HTTP/2
protocol (spdy), and a matching `h5vcc.settings` parameter 'HTTP2' (with
backing by `PersistentSettings`) for run-time disabling of HTTP/2 and
spdy. Similar to 'QUIC' and 'HTTP3', the setting takes effect
immediately for new connections only.

This also ensures that when the command-line parameter `disable_quic` or
the new `disable_h2` is used, that that can not be overuled later with
`5vcc.settings` or from the corresponding `PersistentSetting`.

b/205134049

(cherry picked from commit 2ca4fc9)
jellefoks added a commit that referenced this pull request Aug 13, 2024
Refer to the original PR: #3979

This adds a command-line switch `disable_h2` that disables the HTTP/2
protocol (spdy), and a matching `h5vcc.settings` parameter 'HTTP2' (with
backing by `PersistentSettings`) for run-time disabling of HTTP/2 and
spdy. Similar to 'QUIC' and 'HTTP3', the setting takes effect
immediately for new connections only.

This also ensures that when the command-line parameter `disable_quic` or
the new `disable_h2` is used, that that can not be overuled later with
`5vcc.settings` or from the corresponding `PersistentSetting`.

b/205134049

Co-authored-by: Jelle Foks <[email protected]>
kaidokert pushed a commit that referenced this pull request Aug 13, 2024
Refer to the original PR: #3979

This adds a command-line switch `disable_h2` that disables the HTTP/2
protocol (spdy), and a matching `h5vcc.settings` parameter 'HTTP2' (with
backing by `PersistentSettings`) for run-time disabling of HTTP/2 and
spdy. Similar to 'QUIC' and 'HTTP3', the setting takes effect
immediately for new connections only.

This also ensures that when the command-line parameter `disable_quic` or
the new `disable_h2` is used, that that can not be overuled later with
`5vcc.settings` or from the corresponding `PersistentSetting`.

b/205134049

Co-authored-by: Jelle Foks <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cp-24.lts.1+ Cherry Pick to the 24.lts.1+ branch cp-25.lts.1+ Cherry Pick to the 25.lts.1+ branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants