clj-commons / aleph

Asynchronous streaming communication for Clojure - web server, web client, and raw TCP/UDP
http://aleph.io
MIT License
2.54k stars 241 forks source link

Add server option for user defined header key normalization function #584

Closed cnautze closed 2 years ago

cnautze commented 2 years ago

Description

Since some of our response header keys are case sensitive we would like to handle these differently than the vanilla capitalization currently used. The suggested change adds a server option :header-key-transform to pass in a function that is used for the non-standard header keys. This function expects a header key string and returns a normalized string. Looking forward to your feedback.

KingMob commented 2 years ago

Well, with the caveat that HTTP 1.1 headers are defined to be case-insensitive, I have no fundamental problem incorporating this functionality.

EDIT: Updated to be specifically about HTTP/1.1

KingMob commented 2 years ago

Hmm, upon further thought, I think I have a different solution.

First, this need is really niche. HTTP/1.1 is supposed to be case-insensitive, and most things support that. HTTP/2 requires lowercase headers, so it should be even less of a concern going forward.

Many of the intermediate Aleph fns have too many params already, but unfortunately, they're public, so we can't just collect them into maps without breakage, or I'd try that route to support you. (Though I am considering 2-arity versions that would do that at the cost of speed.)

Instead, I'd like to suggest you use the pipeline-transform parameter to insert a new channel handler in the Netty pipeline that alters headers after the main handler is done, but before anything gets written to the wire. It would look something like this:

(http/start-server
    your-main-handler
    {;; other parameters go here...
     :pipeline-transform
     (fn [^io.netty.channel.ChannelPipeline pipeline]
       ;; yes it's .addBefore, on the way out, Netty handlers go in reverse, sort of like interceptors
       (.addBefore pipeline
                   "request-handler"                        ; the Aleph name for the main handler
                   "server-header-transformer"              ; the name of your handler
                   (proxy [io.netty.channel.ChannelOutboundHandlerAdapter] []
                     (write [^io.netty.channel.ChannelHandlerContext ctx
                             ^Object msg
                             ^io.netty.channel.ChannelPromise p]
                       (when (instance? io.netty.handler.codec.http.HttpMessage msg)
                         (let [^io.netty.handler.codec.http.HttpMessage http-msg msg
                               headers (.headers http-msg)
                               server-val (.get headers "Server")]
                           (.set headers "SERVER" server-val))) ; make the "server" header uppercase
                       (.write ctx msg p)))))})

NB: if you're testing this with the Aleph client, you should know it also lower-cases headers it receives, via the NettyResponse and HeaderMap classes, so it may be hard to see the results if you're transforming another way. But I verified this technique works via Wireshark.

cnautze commented 2 years ago

Thank you for your feedback! I understand your objections. After an internal discussion we decided to go forward with our current solution (not the suggested changes) which is perhaps not the most beautiful but works for us.

KingMob commented 2 years ago

@cnautze I think the problem is, there's no good way to handle this, given the way Aleph currently works. Please don't hesitate to contribute again, though.