Skip to content

http inspector: rename h2 to h2c#8227

Merged
lizan merged 2 commits intoenvoyproxy:masterfrom
yxue:master
Sep 17, 2019
Merged

http inspector: rename h2 to h2c#8227
lizan merged 2 commits intoenvoyproxy:masterfrom
yxue:master

Conversation

@yxue
Copy link
Copy Markdown
Member

@yxue yxue commented Sep 12, 2019

Signed-off-by: crazyxy yxyan@google.com

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Description: Rename h2 to h2c to respect the standard
Risk Level: Low
Testing: Unit test
Docs Changes: N/A
Release Notes: N/A
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: crazyxy <yxyan@google.com>
@yxue yxue requested a review from lizan as a code owner September 12, 2019 21:20
lizan
lizan previously approved these changes Sep 12, 2019
Copy link
Copy Markdown
Member

@lizan lizan left a comment

Choose a reason for hiding this comment

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

Nice catch.

@lizan
Copy link
Copy Markdown
Member

lizan commented Sep 13, 2019

@yxue actually hold on this, can you double check this work with HCM when codec is auto? I guess this line will create http1 codec if ALPN is set to "h2c":
https://github.com/envoyproxy/envoy/blob/master/source/common/http/conn_manager_utility.cc#L41

@yxue
Copy link
Copy Markdown
Member Author

yxue commented Sep 13, 2019

@lizan it doesn't affect the selection of the codec no matter whichever I set here. It's seems to be a bug. When selecting the codec, it searches the connection's protocol first and for raw buffer transport socket, the protocol is empty. The codec selection will fallback to searching the preface inside the request body.

std::string ConnectionManagerUtility::determineNextProtocol(Network::Connection& connection,

@lizan
Copy link
Copy Markdown
Member

lizan commented Sep 16, 2019

Can you add a comment/TODO in HCM or HTTP inspector, to use detected protocol from http inspector and support h2c token?

Signed-off-by: crazyxy <yxyan@google.com>
@lizan lizan merged commit 8c28a4f into envoyproxy:master Sep 17, 2019
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Sep 24, 2019
Description: Rename h2 to h2c to respect the standard
Risk Level: Low
Testing: Unit test
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: crazyxy <yxyan@google.com>
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Oct 4, 2019
Description: Rename h2 to h2c to respect the standard
Risk Level: Low
Testing: Unit test
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: crazyxy <yxyan@google.com>
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Oct 4, 2019
Description: Rename h2 to h2c to respect the standard
Risk Level: Low
Testing: Unit test
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: crazyxy <yxyan@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants