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

Headers are lower-cased when sent and no option to disable this feature. #1492

Closed
mbilker opened this issue Apr 19, 2018 · 10 comments
Closed

Comments

@mbilker
Copy link
Contributor

mbilker commented Apr 19, 2018

I am trying to use Hyper with a stubborn web service that does not like requests with headers that do not have the first letter of each word capitalized (Content-Length for example). I tried out some other HTTP client libraries for Rust (like tk-http which hasn't been updated recently and mio_httpc which doesn't work all the time with said web service), but Hyper doesn't have the issues as with the other libraries except for the header name mangling.

After reading the RFC regarding header casing, I see that header names are supposed to be case-insensitive, so Hyper is not at fault here and from looking at HeaderMap I see that it relies on headers being lower-cased for performance.

If it is not possible to accommodate case-sensitive header names, then could it be possible to directly provide an array of headers to be used in an HTTP request?

@seanmonstar
Copy link
Member

I expected this problem to appear eventually. Clearly, the best thing to happen is for that server software to be fixed. But, I realized that there is likely other janky server software out there that also relies on title case, just rotting with no fix to ever come...

Possibly a solution could be for hyper to add an configuration option to force all header names to Title Case when writing to the socket. It could be some title_case(dst: &mut Vec<u8>, name: &str) {}, that intercepts and pushes upper-case bytes onto the destination...

@mbilker
Copy link
Contributor Author

mbilker commented Apr 21, 2018

That would perfectly fit my use case and not affect the functionality of the headers API for general use of the client library part of Hyper. Currently investigating how headers are written for HTTP/1.0 and HTTP/1.1 connections.

@mbilker
Copy link
Contributor Author

mbilker commented Apr 21, 2018

I found the write_headers function[1] and see that the client calls it when it is encoding the response. Is it more preferable to include a boolean parameter to tell write_headers to use title case or duplicate the write_headers function into one named, for example, write_headers_title_case and replace the extend call for the header name with one to title_case(dst, name)?

[1]

hyper/src/proto/h1/role.rs

Lines 638 to 645 in dfdca25

fn write_headers(headers: &HeaderMap, dst: &mut Vec<u8>) {
for (name, value) in headers {
extend(dst, name.as_str().as_bytes());
extend(dst, b": ");
extend(dst, value.as_bytes());
extend(dst, b"\r\n");
}
}

@seanmonstar
Copy link
Member

It's probably preferable to not have to check a boolean in a loop, so probably a second function.

@mbilker
Copy link
Contributor Author

mbilker commented Apr 21, 2018

Alright, that's what I did for my naive example code (that's still WIP and unpublished). Is it preferable to search for the hyphen and capitalize the next character? Or go character by character in a for loop keeping state in a boolean to capitalize the next character? Or using a while let Some loop so I could use chars.next() to avoid using a boolean to store state?

My naive example:

fn title_case(dst: &mut Vec<u8>, name: &str) {
    dst.reserve(name.len());

    let mut should_capitalize = true;

    for c in name.chars() {
        if should_capitalize {
            should_capitalize = false;

            for c in c.to_uppercase() {
                dst.push(c as u8);
            }
        } else {
            if c == '-' {
                should_capitalize = true;
            }

            dst.push(c as u8);
        }
    }
}

@mbilker
Copy link
Contributor Author

mbilker commented Apr 21, 2018

A while let Some version:

fn title_case(dst: &mut Vec<u8>, name: &str) {
    dst.reserve(name.len());

    let mut iter = name.chars();

    let push_uppercase = |dst: &mut Vec<u8>, iter: &mut ::std::str::Chars| {
        if let Some(c) = iter.next() {
            for c in c.to_uppercase() {
                dst.push(c as u8);
            }
        }
    };

    push_uppercase(dst, &mut iter);
    while let Some(c) = iter.next() {
        dst.push(c as u8);

        if c == '-' {
            push_uppercase(dst, &mut iter);
        }
    }
}

@seanmonstar
Copy link
Member

Since header names are required to be ASCII, you can just work with the bytes directly. That has better performance than char.

@mbilker
Copy link
Contributor Author

mbilker commented Apr 23, 2018

Revised this to use a byte slice (&[u8]) instead of a &str. I see that lowercase to uppercase conversion of ASCII characters can be done by XORing the lowercase byte with b' ' to get the uppercase character byte.

I probably should open up a PR for this now, but I am still trying to work out how to add a configuration option to toggle between title case and straight lowercase output.

@seanmonstar
Copy link
Member

Your use case is the client, right? So it could start there...

  1. Add some sort of http1_title_case_headers method to hyper::client::Builder.
  2. Pass the value of that when constructing the hyper::proto::h1::Conn.
  3. Pass that value to Http1Transaction::decode.

@mbilker
Copy link
Contributor Author

mbilker commented Apr 23, 2018

Made a PR. Thank you so very much for the guidance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants