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

Please revisit the comma separated header collapsing decision. #1120

Open
midnight-wonderer opened this issue Jan 22, 2020 · 5 comments
Open

Comments

@midnight-wonderer
Copy link

There was a decision to concatenate multiple header values using comma as a delimiter in the past. (#44)
And this is the real-world consequences of the decision.
sparklemotion/http-cookie#28

Could you somehow provide a way to access raw HTTP headers?

@iMacTia
Copy link
Member

iMacTia commented Feb 7, 2020

@midnight-wonderer apologies for the delay getting back to you on this.
I just don't want to take a rushed decision and I'm sure the core team did a lot of thinking back in the days when they agreed on keeping that behaviour.
So I'm carefully catching up to gather a full picture before taking a decision.

@iMacTia
Copy link
Member

iMacTia commented Feb 9, 2020

@midnight-wonderer OK so this should be the line code responsible for managing multiple headers values and concatenating them with a comma: https://github.com/lostisland/faraday/blob/master/lib/faraday/utils/headers.rb#L61

The ultimate objective here is to preserve backwards-compatibility, and I believe there might be a way to do just that: we could store a copy of the "raw" header value (as an array) into a separate key, allowing you to optionally access that using the alternative key.
For example, say your server returns multiple Set-Cookies, you will be able to retrieve that as a comma-separated list with Set-Cookie, and as an array as Set-Cookie-Raw (or something like that).

Would such a solution work for you?
I'd also like to add that one of the reasons this hasn't been raised so much in the past is that Faraday (and in general Ruby libraries) are supposed to be used server-side, where authentication through Cookies is considered a bad practice and API tokens should instead be preferred.

I'm not saying we shouldn't solve this anyway, but would you have a real-world example of application you're trying to write that doesn't work because of the decision that was taken years ago to use a comma-separated list? Your example of the cookies headers is not a really good one for the reason I stated above.

@midnight-wonderer
Copy link
Author

Well, for my particular use case, the additional key solution somehow works, but I prefer different interfaces for different use cases.

The old (current) behavior has its use case as a comma-separated field, and some people are relying on it. It would be better if I can access the value using a different method rather than adding more keys to the hash. This way, we would avoid the issue of collided keys also; where the server can send arbitrary non-standard headers, and they might have the same name as says, "Set-Cookie-Raw".

My actual use case is that I want to download a receipt file (in PDF) from a third-party website service providers and send it to my company. I have to download the file myself via browser every month, it became tedious, and I want to automate the process. I discovered that the authorization is simple, and I don't have to spin up a headless browser to download the PDF just cURL-liked libraries that support cookies would do, but the only cookie jar implementation in Ruby I could find is
https://github.com/sparklemotion/http-cookie
They weirdly implement overly complicated cookie parser, and I wondered why, so I dig further and find that some HTTP client implementation in Ruby concatenated header with the same name to a string. (e.g. Faraday)

The overly complicated method I mentioned also made a big assumption that cookies sent by servers are RFC-compliant. Rendered the library unusable when facing a non-standard cookie. (I found such cookie on Lynda.com, a LinkedIn company)

I already re-implemented the cookie jar library that addressed all my issues and polishing it right now.
I also reached out to Faraday to notify you about the issue, and here we are. I hope this explains the real-world situations.

@iMacTia
Copy link
Member

iMacTia commented Feb 10, 2020

@midnight-wonderer I see, I wasn't really sure about the connection between http-cookie and faraday but I assume you're simply using both in your project.
Not sure this will completely solve your issue (you might still need changes from the http-cookie team), but I'm happy to have this change implemented in Faraday.

I also agree with your comment, a separate ad-hoc method is definitely a better and more future-proof choice.
I can't tell when I'll have the time to pick this up, but if anyone wants to get their hands dirty in the meantime, I'll happily review a PR

@midnight-wonderer
Copy link
Author

The two linked because the decision of Faraday highly influences the decision of http-cookie; driven it to its current state.

I am abandoning http-cookie gem, the project is pretty much dead, no possibility of rescuing from me. I'm going with my own Cookiejar of Greed, a complete rewrite.

As for the changes in Faraday, I too, cannot take it right now. I guess we are stuck with another open issue for some time.

Thank you very much for taking your time reviewing this thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants