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

Clarify if psuedo-headers are supported within HTTP header matching #740

Closed
hbagdi opened this issue Jul 29, 2021 · 6 comments
Closed

Clarify if psuedo-headers are supported within HTTP header matching #740

hbagdi opened this issue Jul 29, 2021 · 6 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/documentation Categorizes issue or PR as related to documentation.

Comments

@hbagdi
Copy link
Contributor

hbagdi commented Jul 29, 2021

What happened:

Pseudo-headers can be used to specify matches against non-header properties of HTTP requests: #733 (comment)

What you expected to happen:

The documentation of the API clarify if pseudo-headers are supported or not.

How to reproduce it (as minimally and precisely as possible):

N/A

@hbagdi hbagdi added kind/bug Categorizes issue or PR as related to a bug. kind/documentation Categorizes issue or PR as related to documentation. labels Jul 29, 2021
@hbagdi
Copy link
Contributor Author

hbagdi commented Jul 29, 2021

Another reason: Pseudo-headers are not very popular (at least I have not encountered them for matching purposes before) so mentioning them explicitly would be helpful to users and maintainers.

@tokers
Copy link
Contributor

tokers commented Sep 8, 2021

/assign

@jpeach
Copy link
Contributor

jpeach commented Sep 8, 2021

Validation to reject pseudo headers was added in #772

@youngnick
Copy link
Contributor

oof, that is a good point. That validation will also need to be fixed if we are going to allow pseudo-headers. In that PR, I was slightly in favor of not allowing them in favor of explicit config, but it's not a strong preference.

@hbagdi
Copy link
Contributor Author

hbagdi commented Sep 9, 2021

The guidance on this is now present:

// HTTPHeaderName is the name of an HTTP header.
//
// Valid values include:
//
// * "Authorization"
// * "Set-Cookie"
//
// Invalid values include:
//
// * ":method" - ":" is an invalid character. This means that pseudo headers are
// not currently supported by this type.
// * "/invalid" - "/" is an invalid character

/close

@k8s-ci-robot
Copy link
Contributor

@hbagdi: Closing this issue.

In response to this:

The guidance on this is now present:

// HTTPHeaderName is the name of an HTTP header.
//
// Valid values include:
//
// * "Authorization"
// * "Set-Cookie"
//
// Invalid values include:
//
// * ":method" - ":" is an invalid character. This means that pseudo headers are
// not currently supported by this type.
// * "/invalid" - "/" is an invalid character

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/documentation Categorizes issue or PR as related to documentation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants