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

Disabling Muuntaja with Content-Type header is case-sensitive #99

Open
miikka opened this issue Apr 1, 2019 · 9 comments
Open

Disabling Muuntaja with Content-Type header is case-sensitive #99

miikka opened this issue Apr 1, 2019 · 9 comments

Comments

@miikka
Copy link
Contributor

miikka commented Apr 1, 2019

Muuntaja does not encode the response if a Content-Type header has been set. Header names are case-insensitive but Muuntaja's check is case-sensitive, so if you set the header content-type, it won't disable the encoding.

Considering that the Ring spec says that the request headers will be downcased, I'd expect downcased headers to work in the response maps as well.

@ikitommi
Copy link
Member

ikitommi commented Apr 1, 2019

I believe only request headers are downcased, response headers are sent as-is.

@miikka
Copy link
Contributor Author

miikka commented Apr 1, 2019

Yes, so there's no established convention on which case should be used by the application and the middlewares to avoid exactly this situation.

@ikitommi
Copy link
Member

ikitommi commented Apr 1, 2019

Hmm. The http spec says it's "Content-Type" and (all?) the clojure libs (ring, pedestal, yada, muuntaja) use this casing as well. I would say it's the convention to use.

https://github.com/ring-clojure/ring/blob/master/ring-core/src/ring/util/response.clj

I wonder if there any fast ways to do ignore-case lookup on pers.maps?

@miikka
Copy link
Contributor Author

miikka commented Apr 1, 2019

I don't know if there's a fast way. I suppose that ultimately this is a bug in the Ring spec, since it does not specify how to handle response headers in an interoperable way, but probably it's too late to change it now.

@ikitommi
Copy link
Member

ikitommi commented Apr 1, 2019

I recall James thinked aloud about Ring Spec2, with namespaces keys. So, maybe there is a chance to get something for this too? I would like to see Ring Requests and Responses as protocols myself....

@miikka
Copy link
Contributor Author

miikka commented Apr 2, 2019

I guess Ring's answer to this problem right now is that you should use find-header/get-header/update-header: https://github.com/ring-clojure/ring/blob/1d858fb13381e92f5ced4dedf42853fc062ace8c/ring-core/src/ring/util/response.clj#L186-L209

@chrisbetz
Copy link

Hi, working on that. Just to let you know. I'll prepare a PR as soon as I'm done. You might want to check out my fork in the meanwhile.

@cch1
Copy link

cch1 commented Mar 5, 2023

I'm using muuntaja with babashka/http-client and have run into this same problem with a server that returns "content-type" (lower-case).

(I'm considering hato, but am nervous about the lack of activity on that repo)

This is an awesome library and it's ability to work consistently for clients and servers, for requests and responses is refreshing. Thank you.

FWIW, I think the "safe" way to handle headers is something very much like the ring code referenced above. Another option would be to normalize the headers before processing using camel-snake-kebab.

@Ramblurr
Copy link

Ramblurr commented Nov 14, 2024

This issue came up in slack

  1. We initially thought it was a bug in metosin/ring-http-response for sending title cased headers. Headers should be lower-cased ring-http-response#35
  2. But then weavejester himself confirmed this was a bug in the spec and that actually Ring 1.x spec has nothing to say about the case of the response headers.

So as it stands, there is a footgun with muuntaja, you have to use lowercased response headers. Since Ring 1.x will not change on this matter and Ring 2.x isn't coming anttime soon (see last link above), maybe the solution is to:

  • Use find-header/get-header/update-header

OR

  • Document clearly in muuntaja (and reitit docs, because most folks will only read those) that your app should use lower cased response headers (or at least for content-type) ?

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

No branches or pull requests

5 participants