ext-php-rs refactor #3
Conversation
Qard
force-pushed
the
ext-php-rs-refactor
branch
2 times, most recently
from
6b72a19 to
09f540c
Compare
crates/lang_handler/src/headers.rs
Outdated
| { | ||
| match self.0.get(key.as_ref()) { | ||
| Some(Header::Single(value)) => Some(value.clone()), | ||
| Some(Header::Multiple(values)) => values.last().cloned(), |
There was a problem hiding this comment.
not sure it is correct, I'm sure for some headers, for example Cache-control in response, they should be listed comma separated
There was a problem hiding this comment.
Yeah, at the moment this is leaving that up to the user. They can do get to get the last header, assuming only one matters. They can do get_all when they want an array of values, useful when needing to treat them as separate lines like with Cookies, and they can use get_line to get the headers joined into one comma-separated string, which is what the spec says is the only valid way to have multiple values in one string, but with the exception that any headers which already contain a comma (such as Cookies) may need special parsing and so are generally not recommended to use in that way as servers may not parse the header line correctly.
I opted for manual control here rather than encoding all the rules directly into the Headers type, which would be complicated and add maintenance burden to keep it updated, plus would not include any understanding of custom/non-standard headers. Just seemed like a better choice to leave it up to the user and suggest a default in the doc comments.
|
I left some comments, but maybe at this stage are not important, feel free to ignore them A-M-A-Z-I-N-G |
|
once CI is passing :) |
Qard
force-pushed
the
ext-php-rs-refactor
branch
3 times, most recently
from
cc058e5 to
7cad525
Compare
Qard
force-pushed
the
ext-php-rs-refactor
branch
11 times, most recently
from
641d0a4 to
e7353ea
Compare
Qard
force-pushed
the
ext-php-rs-refactor
branch
3 times, most recently
from
1bf1296 to
2adb1e4
Compare
Need to wire them up to something now...
3 participants
This is a refactor of #2 to instead use ext-php-rs as it is a lot easier to get it to build correctly and gives us more flexibility to support dynamic loaded extensions.