Multiple Set-Cookie headers in CPPRESTSDK

[JengdiB]

Hi,

I found that the implementation of web::http::http_header::add simply append the old header value with a new value using comma(',') as a separator.

    void add(const key_type& name, const _t1& value)
    {
        if (has(name))
        {
            m_headers[name] =  m_headers[name].append(_XPLATSTR(", ") + utility::conversions::print_string(value));
        }
        else
        {
            m_headers[name] = utility::conversions::print_string(value);
        }
    }

This make all "Set-Cookie" headers combined into one. It breaks the rfc6265 and it does not work for all browsers(i tested with Chrome, IE, and FireFox).

here is a snapshot for the RFC

Origin servers SHOULD NOT fold multiple Set-Cookie header fields into
   a single header field.  The usual mechanism for folding HTTP headers
   fields (i.e., as defined in [RFC2616]) might change the semantics of
   the Set-Cookie header field because the %x2C (",") character is used
   by Set-Cookie in a way that conflicts with such folding.

What I expect is that everytimes I call web::http::http_header::add I should get multiple 'Set-Header' in response.

So please advise if this should be fixed in the library.

[ras0219-msft]

This does look like a bug that should be fixed in the library.

Please feel free to send a PR.

[biskit1]

Hi there @ras0219-msft ,
Looks like the code referenced in this issue has been changed.
Should this issue be closed?

[jjmcwill]

No, it shouldn't be closed. While the code has changed, it still works effectively the same way.

     template<typename _t1>
     void add(const key_type& name, const _t1& value)
    {
         auto printedValue = utility::conversions::details::print_string(value);
         auto& mapVal = m_headers[name];
         if (mapVal.empty())
         {
             mapVal = std::move(printedValue);
         }
         else
         {
             mapVal.append(_XPLATSTR(", ")).append(std::move(printedValue));
         }
     }

http_headers still stores all headers as a map, which means single Key,Value pairs. The RFC clearly states that there can be multiple Set-Cookie headers, and the existing architecture isn't allowing it.

We just discovered this same problem, and it's a serious limitation for us.

[sinall]

The straightforward way is using a vector to hold values, and provide set/add methods, the set will replace values[0], and add will append to values vector. set method could be passed a vector, in that case, all values will be replaced.
The only thing I can see is that the existing calls to add needs to be changed to set, because once the value could hold more than 1 item, then add could mean either adding another one or adding another header.