CURL and finagle failing to parse :set-cookies with '\n'

[kroncatti]

CURL and finagle failing to parse :set-cookies with '\n'

[kroncatti]

Thank you very much for the constructive feedback and for the note concerning it being a breaking change. Gonna close the issue.

[ptaoussanis]

@kroncatti Hi Kaleb, about to head to bed but will try take a look at this properly tomorrow. Apologies for the trouble!

[kroncatti]

Hey @ptaoussanis, don't apologize at all! thank you for your fast answer!

[ptaoussanis]

@kroncatti Hi Kaleb,

Just want to make sure that I understand this issue clearly. How exactly are finagle and curl involved here?

1. You have an http-kit server with a route that sets >1 cookie
2. You're using finagle and curl to make a request against that http-kit server route, and those requests are failing with the errors you mentioned above?

Is that right?

[ptaoussanis]

Looking into this a bit closer:

(do
  (require '[org.httpkit.server :as hks])
  (require '[org.httpkit.client :as hkc])
  (require '[ring.adapter.jetty :as jetty])
  (require '[clj-http.client    :as clj-http]))

(def resp
  {:status 200
   :body "body"
   :headers
   {"x-foo"      ["bar1" "bar2"]
    "set-cookie" ["cookie1=val1" "cookie2=val2"]}})

(def test-handler (fn [_ring-req] resp))

(do
  (def server (hks/run-server  test-handler {:port 63331}))
  (def jetty  (jetty/run-jetty test-handler {:port 63332 :join? false})))

(comment
  (server)
  (.stop jetty))

;; http-kit client -> http-kit server
(get-in @(hkc/get "http://localhost:63331") [:headers :set-cookie])
;; => "cookie1=val1\ncookie2=val2"

;; clj-http -> http-kit server
(get-in (clj-http/get "http://localhost:63331" {:decode-cookies false}) [:headers "set-cookie"])
;; => ["cookie1=val1" "cookie2=val2"]

;; http-kit client -> Jetty
(get-in @(hkc/get "http://localhost:63332") [:headers :set-cookie])
;; => "cookie1=val1\ncookie2=val2"

;; clj-http -> Jetty
(get-in (clj-http/get "http://localhost:63332" {:decode-cookies false}) [:headers "set-cookie"])
;; => ["cookie1=val1" "cookie2=val2"]

and trying curl:

curl --verbose http://localhost:63331
*   Trying 127.0.0.1:63331...
* Connected to localhost (127.0.0.1) port 63331 (#0)
> GET / HTTP/1.1
> Host: localhost:63331
> User-Agent: curl/7.79.1
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Set-Cookie: cookie1=val1; Path=/http;
< Set-Cookie: cookie2=val2; Path=/http;
< content-length: 5
< Server: http-kit
< Date: Thu, 12 Oct 2023 07:39:14 GMT
< 
* Connection #0 to host localhost left intact
body


curl --version
curl 7.79.1 (x86_64-apple-darwin21.0) libcurl/7.79.1 (SecureTransport) LibreSSL/3.3.6 zlib/1.2.11 nghttp2/1.45.1
Release-Date: 2021-09-22
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS GSS-API HSTS HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz MultiSSL NTLM NTLM_WB SPNEGO SSL UnixSockets

So at least from a quick check on my side:

• http-kit server's output seems okay, and consistent with Jetty
• curl 7.79.1 against http-kit server seems okay

It'd be helpful if you could clarify with a precise example of exactly:

• What code you're calling
• What output you expect
• What output you're instead seeing

Otherwise it's quite possible I'm misunderstanding what exactly you're trying to do.

Thanks!

[kroncatti]

Hey @ptaoussanis, thank you very much for your detailed answer.

Indeed, all the examples above are running and I am not being able to reproduce it locally (both with curl or finagle) because creating a finagle client is a little bit tricky (my knowledge in Scala is close to zero lol) and we have a bunch of abstractions over it in the company that I just cannot expose here, I'll keep trying to make it simpler from my side so I can paste some actual code here to give you a little bit more visibility on the issue.

For now, what I can say is the following:

• This is not a bug per see. The issue here is that, we have RFC-7230 and finagle is trying to enforce it in the headers, and that is why having this separator is not recommended, because some clients are going to refuse it as finagle did.

FYI, RFC-9110 made 7230 obsolete, but still, it says:

Field values containing CR, LF, or NUL characters are invalid and dangerous, due to the varying ways that implementations might parse and interpret those characters; a recipient of CR, LF, or NUL within a field value MUST either reject the message or replace each of those characters with SP before further processing or forwarding of that message. Field values containing other CTL characters are also invalid; however, recipients MAY retain such characters for the sake of robustness when they appear within a safe context (e.g., an application-specific quoted string that will not be processed by any downstream HTTP parser).

What are your thoughts about it ?

[kroncatti]

About my idea of rolling back the previous PR: I am not really sure if this is the best solution either because, as the person who commited said, we may have , by a part of the header itself, and the parse may become tricky. We could discuss other options if you are open to it.

[ptaoussanis]

@kroncatti Hi Kaleb,

and that is why having this separator is not recommended, because some clients are going to refuse it as finagle did.

It'd be helpful to understand how exactly finagle and curl are involved in what you're trying to do. How are either of them encountering a newline?

Are you using http-kit client, and then somehow passing http-kit client's response headers through to finagle or curl?

We could discuss other options if you are open to it.

Before we discuss possible solutions, I think it'd be important to be clear on what precisely the problem is.

[kroncatti]

Sorry for jumping directly to conclusions, you are completely right about understanding better the situation here as we may impact a lot of users if anything is changed.

Well, basically:

1. We have a pedestal server/service that, for one of its routes, it is using http-kit to perform HTTP requests with embedded TLS certificates to an external provider. This server is working fine, even with the NEWLINE character, we are having issues when calling it from other places with finagle clients.
2. From another pedestal server/service, we are trying to use our standard HTTP client that uses abstractions over finagle to perform requests on the server described above.

From what we've observed, using 2.52.0 as we were using before, everything goes through fine. But with 2.70.0, the following exception started to pop on step 2. (I just pasted a portion of the stacktrace here):

{"cause":"Header 'set-cookie': only ' ' and '\\t' are allowed after '\\n' in value","trace":"[com.twitter.finagle.http.headers.Rfc7230HeaderValidation$ validateValue \"Rfc7230HeaderValidation.scala\" 160]\n[com.twitter.finagle.netty4.http.handler.HeaderValidatorHandler$ validateHeaders \"HeaderValidatorHandler.scala\" 79` 

We are sure that the "issue" is with the NEWLINE character for us in the header because doing a replace on it for the :set-cookie header in the first pedestal server I described before returning it solved while using 2.7.0.

I'll keep trying to reproduce it.

[ptaoussanis]

is using http-kit to perform HTTP requests with embedded TLS certificates to an external provider.

So you mean you're using http-kit client to make requests? (I'm trying to make a clear distinction here between http-kit client (makes HTTP requests) and http-kit server (which responds to HTTP requests).

This server is working fine, even with the NEWLINE character, we are having issues when calling it from other places with finagle clients.

I'm not really clear on what this means, sorry. What do you mean "the server is working fine even with the NEWLINE character"? How is the server seeing a newline character?

And what do you mean "having issues calling it from other places"?

But with 2.70.0, the following exception started to pop on step 2. (I just pasted a portion of the stacktrace here):

The difficulty with the output you're sharing here is that you're not really contextualizing it.

For someone else (me in this case) to understand what's going on, it's helpful to understand:

• What code is being called
• What output that code is generating
• In what way is that output wrong/unexpected

For example I'm still not quite clear on whether finagle or curl are complaining about bad input (i.e. request headers are bad), or output (i.e. response headers are bad).

My current best guess is the following:

1. Your system is using http-kit client to make a request to some server
2. The http-kit client displays response headers with a newline as a result of #502
3. You're somehow passing the unparsed headers from 2 to finagle and/or curl, and they are complaining about the malformed request (input) headers.

If that's the case, then the issue is that you'll need to do some parsing of the headers in step 2 - they're not safe for just relaying as is. (This'd also be true for clj-http, which interprets multiple headers as a vector).

But if that's not what's going on, and you're instead suggesting that http-kit server is somehow responding with a newline in a header somewhere - then that's a different story, and sounds like a bug.

But first step in proceeding here would be a clear description of the flow of what's going on, otherwise I need to do a lot of guessing.

[kroncatti]

So you mean you're using http-kit client to make requests? (I'm trying to make a clear distinction here between http-kit client (makes HTTP requests) and http-kit server (which responds to HTTP requests).

Yes. We are using an http-kit client to perform requests. We don't hold an http-kit server.

I'm not really clear on what this means, sorry. What do you mean "the server is working fine even with the NEWLINE character"? How is the server seeing a newline character? And what do you mean "having issues calling it from other places"?

Sorry for not being able to disclose the code. I meant that, the pedestal server that is using an http-kit client to perform requests is working fine. Meaning that, the request goes through and it gets back normally. It is seeing the newline character as it is when http-kit client is used to call the external provider.

If that's the case, then the issue is that you'll need to do some parsing of the headers in step 2 - they're not safe for just relaying as is. (This'd also be true for clj-http, which interprets multiple headers as a vector).

Your best guess is correct. So, probably parsing on our side will be the best solution then from what you explained.

Thanks!

[ptaoussanis]

Sorry for not being able to disclose the code.

To be clear- while specific code is ideal when possible, it's not strictly necessary. Just having an idea of how pieces fit together and how the component in question is being used can be helpful to maintainers in understanding what might be going on and therefore where to look.

(Mentioning this as constructive feedback for future issues).

Your best guess is correct. So, probably parsing on our side will be the best solution then from what you explained.

Okay, great - that makes sense. In that case I'd definitely suggest that you need a step to transform the http-kit client response headers to something that can be safely passed on to curl or finagle, etc. What we're talking about here isn't covered by the HTTP spec - it's about the interface between how http-kit client returns response headers, and how curl and co. want their headers fed in as input.

Actually, you should be able to take advantage of #502 here since the newline should let you more easily split multi-valued headers.

I will note: #502 should have been accompanied by a breaking notice since folks (like you) may have been depending on the previous behaviour. Sorry about that!

Thanks!

You're welcome, best of luck!

[kroncatti]

Just to provide some "lessons learned" here that may help a bunch of other folks. In short:

Issue

• We have a pedestal production web-server.
• We are using http-kit to perform outbound requests from the server.
• When receiving multiple Set-Cookie headers back, http-kit would use the same key to store those multiple headers as they have the same name and we cannot have two keys in a map with the same name, but would separate them by using the /n character.
• Pedestal was not being able to externalize the headers directly like this.

Solution

• Instead of externalize it as:

 {:headers {"Set-Cookie" "my-cookie=cookie1 \n my-other-cookie=cookie2"}}

that was causing failures, we just simply parsed with a split as follows:

 {:headers {"Set-Cookie" ["my-cookie=cookie1" "my-other-cookie=cookie2"]}}

and then Pedestal understood it properly.