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 field should be case insensitive #157

Open
KaiPetzke opened this issue Aug 11, 2020 · 5 comments
Open

headers field should be case insensitive #157

KaiPetzke opened this issue Aug 11, 2020 · 5 comments
Labels
good first issue great to start contributing

Comments

@KaiPetzke
Copy link

KaiPetzke commented Aug 11, 2020

Expected behaviour

The following code snippet:

    RestClient::Connection* conn = new RestClient::Connection("https://httpbin.org");
    RestClient::Response& r = conn->get("/get");
    for(auto& name : { "date", "Date" }) {
        try {
            std::string& value = r.headers.at(name);
            std::cout << name << ": \"" << value << "\"" << std::endl;
        } catch(std::out_of_range& oor) {
            std::cout << name << ": not found" << std::endl;
        }
    }

should yield a result similiar to the following:

date: "Tue, 11 Aug 2020 19:52:17 GMT"
Date: "Tue, 11 Aug 2020 19:52:17 GMT"

Actual behaviour

The result is as follows:

date: not found
Date: "Tue, 11 Aug 2020 19:52:17 GMT"

This is an error, as according to RFC 2616: "Field names are case-insensitive". And yes, unfortunately, there are servers, that change the case of their response header fields at will.

Environment and debugging details

This happens with any C compiler and libcurl

Suggested fix:

Use a custom compare function to make the std::map in RestClient::HeaderFields case-insensitive. I have attached a file with the suggested change. Yes, it uses std::tolower(), which unfortunately is locale dependant, for conversion to lowercase. So depending on language settings, tolower() might convert, say, Ç to ç or not. If that is considered an issue, an alternative would be to explicitely only map characters in the range 'A' to 'Z' to 'a' to 'z', and use this mapping function instead of std::tolower():

    unsigned char ascii_tolower(const unsigned char c) {
        return (c >= 'A' && c <= 'Z') ? ((c - 'A') + 'a') : c;
    }

restclient.diff.txt

@edwinpjacques
Copy link
Contributor

@KaiPetzke if you can please submit a fix for this it would be appreciated. It sounds like you already have it but are just unfamiliar with the contributing guidelines?

@edwinpjacques edwinpjacques added the good first issue great to start contributing label Oct 30, 2020
@KaiPetzke
Copy link
Author

Yeah, I didn't read the contribution guidelines. I am busy this week, will try to add a formal fix next week.

@edwinpjacques What do you suggest about my comments below the headline "suggested fix"? Do you prefer std::tolower() or the locale independant ascii_tolower()?

@edwinpjacques
Copy link
Contributor

std::tolower looks good. https://en.cppreference.com/w/cpp/string/byte/tolower

@KaiPetzke
Copy link
Author

I am still busy, will look at all these issues early next year.

@Mirza585
Copy link

Shall I fix the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue great to start contributing
Projects
None yet
Development

No branches or pull requests

3 participants