hyperium / h3

MIT License
575 stars 75 forks source link

Ensure authority and/or host fields are not empty #232

Closed Pi-Cla closed 1 month ago

Pi-Cla commented 3 months ago

We could have an authority or host that consists of an empty string

Pi-Cla commented 3 months ago

I am unsure how to correctly to create a HeaderField with an empty value for the tests. If that is even possible. If it isn't then maybe this PR is redundant and I should only update the compliance?

Ruben2424 commented 3 months ago

Hi, thanks for the PR! You are using the try_from method in the test to create the header. This method is also used wen receiving a request. try_from parses the header field to a typed header field from the http crate which allows no empty values.

I would leave the check and the test for the host field where it is. For the authority field, i would copy the compliance comment here and assert the result of try_from instead of into_request_parts

Pi-Cla commented 3 months ago

@Ruben2424 as it turns out, once I got the tests working I discovered that the initial code already errors out when either field is empty. The host field errors out when it is built into a Uri since the Uri Builder rejects empty Uris. At least we have more tests and documentation now