-
-
Notifications
You must be signed in to change notification settings - Fork 269
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
Deny requerst if :authority field is invalid only with CONNECT method #612
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Arthur LE MOIGNE <arthur.lemoigne@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes it possible to use h2 with k8s.
@seanmonstar do you see any issue with this change? |
Any update on this one? Just met the same problem when communicate with k8s via tonic :( |
Any update on this? Would be good to stop using a fork :) |
k8s 1.26 kubelet sets |
Even if recent version of k8s set
I do not really understand why maintainers do not want to review / merge / close this PR. I still hope this will be merged or discussed one day 😄. |
@arthurlm FWIW I agree with you. Just wanted to share what I know k8s kubelets can do today. |
Opcua crate patch update required because of [opcua#294](locka99/opcua#294) h2 patch needed because of the bad/missing Authority header, using upstream PR branch for this [h2#612](hyperium/h2#612) Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com>
Opcua crate patch update required because of [opcua#294](locka99/opcua#294) h2 patch needed because of the bad/missing Authority header, using upstream PR branch for this [h2#612](hyperium/h2#612) Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com>
Opcua crate patch update required because of [opcua#294](locka99/opcua#294) h2 patch needed because of the bad/missing Authority header, using upstream PR branch for this [h2#612](hyperium/h2#612) Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com>
Opcua crate patch update required because of [opcua#294](locka99/opcua#294) h2 patch needed because of the bad/missing Authority header, using upstream PR branch for this [h2#612](hyperium/h2#612) Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com>
Opcua crate patch update required because of [opcua#294](locka99/opcua#294) h2 patch needed because of the bad/missing Authority header, using upstream PR branch for this [h2#612](hyperium/h2#612) Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com>
* Use upstream version of h2 Go back to upstream h2 version as the go-grpc bug is long solved Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> * webhook: Upgrade actix Upgrade actix, actix-web and actix-rt to latest, This solves the following audit issues: - RUSTSEC-2020-0016 - RUSTSEC-2020-0056 - RUSTSEC-2021-0124 - RUSTSEC-2023-0034 Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> * Update all dependencies Opcua crate patch update required because of [opcua#294](locka99/opcua#294) h2 patch needed because of the bad/missing Authority header, using upstream PR branch for this [h2#612](hyperium/h2#612) Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> * Upgrade to 2021 edition This is needed to be able to upgrade prost dependency Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> * Update to rust 1.73.0 Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> * Fix clippy errors/warning with new rust version and edition Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> * Update tonic and prost Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> * Change uri used for patched h2 Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> * Update patch version Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Remove patch for opcua as upstream made release Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> * Also upgrade mockall Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> --------- Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Looking at RFCs:
So, from my understanding:
Following (1):
:authority
field is not mandatory in HTTP2.Following (2):
/
character is not expected for regular authority).:authority
. It should be treated as opaque string.Following (3):
:authority
field should be a valid host and port in the case ofCONNECT
method.NOTE 1: I have not read the whole RFCs but just read in details the mentioned sections. If anyone have better understanding of this RFCs, please feel free to comment / edit this PR 😉 !
NOTE 2: Please have a look at my first comment about k8s usage with
tonic
for more details.This change could fix a lot of already referenced issues / PRs using
h2
and gRPC /tonic
.It will also:
tonic
(which is not possible for now without forkingh2
)h2
to better fit to HTTP2 RFC specification 😁