Allow setting custom headers on client #47
Conversation
| self.buffer.extend_from_slice(b"\r\n"); | ||
| self.buffer.extend_from_slice(h.name.as_bytes()); | ||
| self.buffer.extend_from_slice(b": "); | ||
| self.buffer.extend_from_slice(h.value); |
There was a problem hiding this comment.
I'm just wondering here; should we do anything about custom header values possibly containing newlines and such, or perhaps add a comment somewhere to note that we shouldn't accept untrusted input or something?
There was a problem hiding this comment.
Ah, now I read it more, I can see that they probably are sanitzed via httparse!
There was a problem hiding this comment.
I'm fine with adding a note since it takes a slice of a type with public fields.
I.e. I had jsonrpsee with
pub fn custom_header(mut self, name: &'a str, value: &'a [u8]) -> Self {
self.headers.push(Header{name, value});
self
}
which essentially doesn't perform any checks. I'm a big fan of http's HeaderMap, both for performance and for its' clean API. Requires a bit more space for the construction of the map, however.
|
I had a think about this, and looked at https://datatracker.ietf.org/doc/html/rfc7230#section-3.2 and at how With that in mind, I think it's best to keep it simple here, and just stick a warning on If that was done, I'd be happy with this. I can see that it could be useful to return some custom details to a client, so the PR makes sense to me! |
| pub fn set_origin(&mut self, o: &'a str) -> &mut Self { | ||
| self.origin = Some(o); | ||
| /// Set connection headers to a slice. | ||
| pub fn set_headers(&mut self, h: &'a [Header]) -> &mut Self { |
There was a problem hiding this comment.
should we re-export httparse::Header as this is now exposed in the public API?
There was a problem hiding this comment.
+1 on re-exporting the Header from httparse. We could do some validation on headers in debug mode, but I'm also not sure it's all that necessary. We could provide an enum with all commonly used header names, but that still leaves the values to be open to potentially arbitrary sanitized values.
|
@maciejhirsz I think it's cleanest to leave that up to consuming implementations. We're capping the number of headers the server will take at 32 so the linear scan Line 62 in 4ec22f2 |
No description provided.