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 [email protected]

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:]

@yxue
rename h2 to h2c
d6cb540 
Signed-off-by: crazyxy <[email protected]>
@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
View reviewed changes
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 •
edited
Loading

@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.

envoy/source/common/http/conn_manager_utility.cc

Line 25 in b2da45a

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?

@yxue
add comment
6bbddcf 
Signed-off-by: crazyxy <[email protected]>
@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
@yxue @danzh1989
http inspector: rename h2 to h2c (envoyproxy#8227)
36aca62 
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 <[email protected]>
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Oct 4, 2019
@yxue
http inspector: rename h2 to h2c (envoyproxy#8227)
443b945 
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 <[email protected]>
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Oct 4, 2019
@yxue
http inspector: rename h2 to h2c (envoyproxy#8227)
07c2b97 
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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@lizan lizan lizan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@lizan lizan

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@yxue @lizan