Later response headers override earlier response headers

[TimWolla]

If the server sends duplicate headers, e.g. set-cookie then the resulting headers table will only contain the value of the last time the header appeared.

The headers table either needs to contain nested tables or the values should be concatenated using a comma:

see RFC 7230#3.2.2

[anezirovic]

We'll probably go with folding, in order not to break code expecting headers in single level table. That could too potentially break some code checking the value of headers (and not checking for folded values)

[TimWolla]

Yes, I guess making it a multi level table would be too much of a BC break here.

[anezirovic]

This issue should be fixed now. We fold the headers when sending client requests, don't do any folding for responses.

[TimWolla]

I'm not sure I'm happy with the current implementation for responses. If there's a single occurrence of a response header I'll receive a string. If there's multiple occurences I'll receive a table. This requires me to check the return type. I feel like single headers should also be returned as a (single-item) table.

I would be fine with configuring the behavior by passing a special argument when using send, e.g.:

local response, err = http.head{
  url = "…",
  headers = headers,
  response_header_behavior = "fold" | "table" | "mixed",
}

[anezirovic]

I understand your pain, it's not fun to write the same code again and again.

Both, Request and Response objects now share the same format for .headers attribute (i.e for "multi headers" table value is also a table); when sending requests or responses these tables are handled automatically.

However, for header examination, it would be nice to have other options, so how about these two iterators, if it works for you, we'll add them as methods to request/response classes:

local hdrs = {
    blah = "first",
    doh = {"second", "third", "fifth"},
    wassup = "fourth",
    zz = {"sixth"}
}

--
-- HTTP headers iterator
--
-- Iterator returns key/value pairs for header names/values, making sure
-- that returned values are always of string type
-- (if necessary, it folds multiple headers with same name)
--
-- @param hdrs Headers table as received by http.get and friends
--
-- @return headers iterator suitable for usage in "for" loops
local function get_headers(hdrs)
    local function iter(tbl, k)
        local v
        k, v = next(tbl, k)

        if v ~= nil then
            if type(v) == "table" then
                return k, table.concat(v, ",")
            else
                return k, v
            end
        end
    end

    return iter, hdrs, nil
end

--
-- HTTP headers iterator
--
-- Returns key/value pairs for header names/values, for multiple headers
-- with same name it will return every name/value pair
-- (e.g. this is suitable to process responses with 'Set-Cookie' header)
--
-- @param hdrs Headers table as received by http.get and friends
-- @return headers iterator not suitable for usage in for loops
local function iget_headers(t)
    if t == nil then
        return function() end
    end

    local k
    local idx = 0
    local v_in

    return function ()
        local v
        if idx == 0 then
            k, v = next(t, k)
            if k == nil then return end
        else
            idx, v = next(v_in, idx)

            if idx == nil then
                idx = 0
                k, v = next(t, k)
            end
        end

        if k == nil then return end

        if type(v) == "table" then
            v_in = v
            idx = idx + 1
            return k, v[idx]
        else
            return k, v
        end
    end
end

print("---")
print("Folded headers:")

for k, v in get_headers(hdrs) do
    print(k, v)
end

print("---")
print("Unfolded headers:")

local next_header = iget_headers(hdrs)

while true do
    local hdr, hdr_val = next_header()
    if hdr == nil then break end
    print(hdr, hdr_val)
end

[TimWolla]

so how about these two iterators, if it works for you, we'll add them as methods to request/response classes:

get_headers is looking good for this feature request: https://github.com/TimWolla/haproxy-auth-request/pull/13. However I would also need a function where I can retrieve single string for a header name in O(1) time. I'm currently having this:

https://github.com/TimWolla/haproxy-auth-request/blob/f5e7b395aa86050316e947147f5d66b20dcd8b1f/auth-request.lua#L106

However since your latest patch I am not guaranteed that I get a string back if the backend sends two location response headers for whatever reason there might be. In that case the code would set a variable to a table which is not going to work.

The following functions should solve I use cases I believe.

• get_headers(hdrs): Returns an iterator for all headers, duplicate headers are folded.
• iget_headers(hdrs): Returns an iterator for all headers, duplicate headers are duplicated.
• get_header(hdrs, hdr): Returns a the value for a single header, duplicates are folded.
• iget_header(hdrs, hdr): Returns the last value for a single header.

For the location header I would then use txn:set_var("txn.auth_response_location", iget_header(hdrs, "location")). To generate variables for arbitrary response headers I would then use get_headers(hdrs) to receive folded headers.

[anezirovic]

Great, we're getting somewhere :-)

iget_header() is kinda ugly (I'm really missing generator functions && yield from Python here), I'll try to improve/simplify it.

As for other two functions, get_header(name, fold) method for Request/Response objects would be handy and mostly O(1). It would also lowercase the header name before searching (since internally, they are stored in lowercase)

Will get back with another iteration tomorrow morning.

[TimWolla]

I'm sure you'll figure out something good based off my requirements :smiley: Looking at the function examples above you are definitely more experienced in Lua than I am.

btw: Have you seen my email regarding the GitHub action for easy HAProxy + VTest installation: https://www.mail-archive.com/haproxy@formilux.org/msg37995.html? With all the added functionality automated tests are starting to become interesting.

[anezirovic]

I think the latest commit in the master 4ac4483 wraps this discussion nicely. I think helper functions will make our life easier.

Thanks for the vtest pointer, I'll definitely use that, but first things first, we should add some docs and examples.

[TimWolla]

The commit is looking good to me. Thanks.

I've updated the library within haproxy-auth-request, tests still pass: https://github.com/TimWolla/haproxy-auth-request/pull/23/files

[TimWolla]

And the iterator ist also working well: https://github.com/TimWolla/haproxy-auth-request/pull/24 (Edit: For 2.2+ at least. I'll have to investigate why my code fails on 2.1 and below).

No more complaints from my side. I guess this issue can be closed. Thanks.