lostisland / faraday

Simple, but flexible HTTP client library, with support for multiple backends.
https://lostisland.github.io/faraday
MIT License
5.71k stars 972 forks source link

Impossible to make request with same-name multi-value headers #1486

Closed DocX closed 1 year ago

DocX commented 1 year ago

Basic Info

Issue description

Setting multi-value headers in request method results in the headers to be send as single header with values joined by comma ,.

This makes it impossible to use some APIs that expects multiple values to be send as independent headers with the same name, example https://kubernetes.io/docs/reference/access-authn-authz/authentication/#user-impersonation

Steps to reproduce

Make request with Faraday:

require "faraday"
Faraday.get("http://localhost/", {}, { "Test" => ["foo", "bar"] })

Received request headers on server side:

GET / HTTP/1.1
Host: localhost
User-Agent: Faraday v2.7.4
Accept: */*
Accept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3
Test: foo, bar

Expected headers:

GET / HTTP/1.1
Host: localhost
User-Agent: Faraday v2.7.4
Accept: */*
Accept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3
Test: foo
Test: bar
iMacTia commented 1 year ago

I'm sorry @DocX, but this doesn't seem possible right now. Faraday does not prepare the actual HTTP request, adapters do. So although you can pass an array of values for a header, as you did, the underlying adapters don't support that, so we have to transform that into a comma-separated string.

I've tried passing the array as-is to Net::HTTP, and this is what I got:

uri = URI.parse('https://google.co.uk')
headers = { 'Test' => [1,2] }
Net::HTTP.get(uri, headers)
#=> /ruby/3.1.0/net/http/header.rb:21:in `block in initialize_http_header': undefined method `strip' for [1, 2]:Array (NoMethodError)
#
#        value = value.strip # raise error for invalid byte sequences

I also tried a more "sofisticated" adapter, which is also the one used in the offical Kubernetes Ruby client, but with no luck:

Excon.get('http://postman-echo.com/get', :headers => { 'Test' => [1,2] })
#=> {\"args\":{},\"headers\":{\"x-forwarded-proto\":\"http\",\"x-forwarded-port\":\"80\",\"host\":\"postman-echo.com\",\"x-amzn-trace-id\":\"Root=1-63ce6ce2-73beb1e66e5ed80c24c6b13d\",\"test\":\"1, 2\"},\"url\":\"http://postman-echo.com/get\"}

So there doesn't seem much we can do in Faraday unless Ruby clients support this first.

FWIW, I'd also like to point out that the Kubernetes API is actually not compliant with RFC 7230 - section 3.2.2:

A sender MUST NOT generate multiple header fields with the same field name in a message unless either the entire field value for that header field is defined as a comma-separated list [i.e., #(values)] or the header field is a well-known exception (as noted below).

Note: In practice, the "Set-Cookie" header field ([RFC6265]) often appears multiple times in a response message and does not use the list syntax, violating the above requirements on multiple header fields with the same name. Since it cannot be combined into a single field-value, recipients ought to handle "Set-Cookie" as a special case while processing header fields. (See Appendix A.2.3 of [Kri2001] for details.)

It seems what they ask is not technically allowed, as the only exception to the rule is the Set-Cookie header. This would make ruby clients even less inclined to support this.

I'll be closing this for now, at least until ruby clients better support this.

iMacTia commented 1 year ago

Actually the official Kubernetes client seems to be another one: https://github.com/kubernetes-client/ruby But this has not been updated in over 2 years and it stopped with K8S version 1.13

I had a quick look into the code, but no mention of the Impersonate-Group header 🤔

DocX commented 1 year ago

The golang kubernetes client supports multiple group headers for example:

https://github.com/kubernetes/client-go/blob/c6bd30b9ec5f668df191bc268c6f550c37726edb/transport/round_trippers.go#L251-L252

That said, given that the underlying adapaters do not support that, I agree with closing this here.

DocX commented 1 year ago

There is actually open issue in Kubernetes about this. Apparently comma has special semantics for Kubernetes, so just simply joining multiple values with comma is not possible.

ahmadgandar commented 1 year ago

Pada tanggal Sen, 23 Jan 2023 21.36, Lukáš Doležal @.***> menulis:

There is actually open issue https://github.com/pomerium/pomerium/pull/2051 in Kubernetes about this. Apparently comma has special semantics for Kubernetes, so just simply joining multiple values with comma is not possible.

— Reply to this email directly, view it on GitHub https://github.com/lostisland/faraday/issues/1486#issuecomment-1400449197, or unsubscribe https://github.com/notifications/unsubscribe-auth/A5EEM36I5PCANNGW5HGKTGLWT2JNHANCNFSM6AAAAAAUBY6TFY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

iMacTia commented 1 year ago

Thank you for sharing additional context and resources @DocX 🙏 I hope the K8S team will find a way to make this easier. It seems python clients have similar issues!