split header values on newlines - as created by Rack::Utils

[brushbox]

WIP: Seeking guidance.

I ran into a problem in our Rails app.

A recent change added an additional cookie into the mix. Up until now our specs have only ever set a single cookie.

Many specs set this additional cookie and were failing with an error header field value cannot include CR/LF.

Investigation showed that the Rails cookies middleware adds cookies using Rack::Utils.add_cookie_to_header which appends additional cookies to the header value with a newline between them.

This one-line change fixes that issue - it breaks no existing specs. But I don't think there are any specs that test this condition at all.

I'm more than happy to add a spec but I'm not sure where best to start or how to proceed on that front.

[bblimke]

@brushbox

The error "header field value cannot include CR/LF" comes from Net::HTTP. Do you know why there is an error when using WebMock and not when executing requests without WebMock?

Is this change not required in for other http clients?

[brushbox]

@bblimke That's a great question - I'm looking into it and will let you know what I find.

[brushbox]

@bblimke here's what I've found out so far:

In development we can run our Rails app as either rails server or foreman start these run things a little differently.

rails server: ends up running the app through rack-2.0.7/lib/rack/handler/webrick.rb where we see this:

 93:             elsif k.downcase == "set-cookie"
 94:               res.cookies.concat vs.split("\n")

foreman start: is running through unicorn and ends up at unicorn-5.4.1/lib/unicorn/http_response.rb where we see:

  # writes the rack_response to socket as an HTTP response
  def http_response_write(socket, status, headers, body,
                          req = Unicorn::HttpRequest.new)
    ...
    if headers
      ...
      headers.each do |key, value|
          if value =~ /\n/
            # avoiding blank, key-only cookies with /\n+/
            value.split(/\n+/).each { |v| buf << "#{key}: #{v}\r\n" }
          else
            buf << "#{key}: #{value}\r\n"
          end
        end
        ...
    end

so that explains why we don't have issues except in test.

As time allows I'll look into why/if we have different behaviour when WebMock is involved versus if we don't have WebMock involved in a test request.