joy-framework/http

Multi-value header support

Arteneko opened this issue ยท 8 comments

Sometimes, a response can have multiple headers sharing the same key (e.g. Set-Cookie).

The current implementation squashes everything through (apply table), thus discarding every header but the last.

I've been trying to work on a patch since I need multi-value header support, but not only that'll completely break the current format, the result I produced is... uncertain imho.

Would you mind giving some tips / opinions on this? Thanks

 (defn parse-headers [response]
   (let [str (get response :headers)]
     (->> (string/split "\r\n" str)
          (filter |(string/find ":" $))
-         (mapcat |(string/split ":" $ 0 2))
-         (map string/trim)
-         (apply table)
+         (map |(map string/trim (string/split ":" $ 0 2)))
+         (reduce |(let [key (first $1) value (last $1)]
+                    (if-let [entry (get $ key)]
+                      (set ($ key) [;entry value])
+                      (put $ key [value]))
+                    $) @{})
          (freeze))))
pepe commented

I think this approach cannot leed you to what you want. I would suggest looking at https://github.com/janet-lang/circlet/blob/ba672e7b38bd23b6e8bcde66a8faaf177a1dffbc/circlet_lib.janet#L38 for cookies parsing with peg (I authored, and @andrewchambers recommended on gitter) and start from that point. I admit that pegs are harder to grasp, but I would strongly recommend getting them if you like to stay with Janet, as they are a great tool.

Anyway, great work.

swlkr commented

Is this for incoming requests?

This might be a problem with joy-framework/halo maybe?

You should be able to send multiple Set-Cookie headers here https://github.com/joy-framework/joy/blob/master/src/joy/session.janet#L124-L125

There was also some preliminary support for this in halo here: joy-framework/halo@8315db9

Although, it might not work ๐Ÿ˜…

Oh no, it's not for a server, but a client. I'm trying to build a small thing that sends some HTTP requests to servers.

swlkr commented

Oh this http, jeez I was all mixed up with github notifications haha

swlkr commented

For me, I just fall back to imperative stuff, I'm not a functional purist haha

(defn merged-table [arr]
  (var output @{})

  (let [parts (partition 2 arr)]
    (each [k v] parts
      (if (get output k)
        (put output k (array/concat @[(get output k)] @[v]))
        (put output k v))))

  output)


(defn parse-headers [response]
  (let [str (get response :headers)]
    (->> (string/split "\r\n" str)
         (filter |(string/find ":" $))
         (mapcat |(string/split ":" $ 0 2))
         (map string/trim)
         (merged-table)
         (freeze))))
swlkr commented

I also clearly don't care about memory allocations too much ๐Ÿ˜‚

swlkr commented

Hopefully that commit fixes this

pepe commented

Here I want to apologise to @Arteneko for total misunderstanding and even repo mixup (I thought we are talking in server repo).