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

HttpResponse的DumpHeaders方法序列化出的response header内的cookie字段重复 #597

Closed
caijillx opened this issue Jul 29, 2024 · 2 comments

Comments

@caijillx
Copy link
Contributor

我自己起了一个简单的http server用于测试libhv中httpclient
当我的服务器返回response header如下:

HTTP/1.1 200 OK
Content-Type:text/html; charset=UTF-8
Server:net
Connection:close
Date:Mon, 29 Jul 2024 02:33:01 GMT
Cache-Control:private
Expires:Mon, 29 Jul 2024 02:33:01 GMT
Content-Length: 9
Set-Cookie: session_id=abc123; Expires=Mon, 29 Jul 2024 03:03:01 GMT; Path=/; Secure; HttpOnly

当我在回掉函数里使用HttpResponsePtr->DumpHeaders方法时,其得到的结果是,

Cache-Control: private
Connection: close
Content-Length: 9
Content-Type: text/html; charset=UTF-8
Date: Mon, 29 Jul 2024 02:33:01 GMT
Expires: Mon, 29 Jul 2024 02:33:01 GMT
Server: net
Set-Cookie: session_id=abc123; Expires=Mon, 29 Jul 2024 03:03:01 GMT; Path=/; Secure; HttpOnly
Set-Cookie: session_id=abc123; Path=/; Expires=Mon, 29 Jul 2024 03:03:01 GMT; Secure; HttpOnly

在我看了DumpHeaders过程发现该过程会对headers和cookies进行解析

void HttpMessage::DumpHeaders(std::string& str) {
    FillContentType();
    FillContentLength();
    // headers
    for (auto& header: headers) {
        // http2 :method :path :scheme :authority :status
        if (*str.c_str() != ':') {
            // %s: %s\r\n
            str += header.first;
            str += ": ";
            // fix CVE-2023-26148
            // if the value has \r\n, translate to \\r\\n
            if (header.second.find("\r") != std::string::npos ||
                header.second.find("\n") != std::string::npos) {
                std::string newStr = "";
                for (size_t i = 0; i < header.second.size(); ++i) {
                    if (header.second[i] == '\r') {
                        newStr += "\\r";
                    } else if (header.second[i] == '\n') {
                        newStr += "\\n";
                    } else {
                        newStr += header.second[i];
                    }
                }
                str += newStr;
            } else {
                str += header.second;
            }
            str += "\r\n";
        }
    }

    // cookies
    const char* cookie_field = "Cookie";
    if (type == HTTP_RESPONSE) {
        cookie_field = "Set-Cookie";
    }
    for (auto& cookie : cookies) {
        str += cookie_field;
        str += ": ";
        str += cookie.dump();
        str += "\r\n";
    }
}

但是headers内部本身就包含了cookie信息,随后再对cookie序列化,导致最终得到cookie重复。

@caijillx
Copy link
Contributor Author

caijillx commented Aug 1, 2024

作者你好,我进一步看了解析部分的源码,发现问题出现在Http1Parse.h里的handle_header方法

    void handle_header() {
        if (header_field.size() != 0) {
            if (stricmp(header_field.c_str(), "Set-CooKie") == 0 ||
                stricmp(header_field.c_str(), "Cookie") == 0) {
                HttpCookie cookie;
                if (cookie.parse(header_value)) {
                    parsed->cookies.emplace_back(cookie);
                }
            }
            parsed->headers[header_field] = header_value;
            header_field.clear();
            header_value.clear();
        }
    }

考虑到response header里经常会出现多个Set-Cookie字段,将该字段放入headers这个map数据结构会导致重复覆盖。建议直接解析到cookie就返回。headers内不存cookie

@caijillx
Copy link
Contributor Author

caijillx commented Aug 1, 2024

#599 可以考虑一下这个修复

@ithewei ithewei closed this as completed Aug 1, 2024
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