bungle / lua-resty-session

Session library for OpenResty – flexible and secure
BSD 2-Clause "Simplified" License
320 stars 111 forks source link

Chunked cookies not cleared properly when session is modified and saved again #137

Closed alexdowad closed 2 years ago

alexdowad commented 2 years ago

If one sets session.data, calls session:save(), then modifies session.data and makes it smaller, then calls session:save() again, the second call to session:save() will modify the Set-Cookie header which was created by the first call. This is fine, but the problem is that if the first call set multiple cookie chunks, and the second call set a smaller number of cookie chunks, some spurious and unwanted chunks which were set by the first call will still remain in the Set-Cookie header and will bloat the response which is sent to the client.

I think something needs to be added here: https://github.com/bungle/lua-resty-session/blob/master/lib/resty/session.lua#L236

The added code might look something like:

if type(cookie_header) == "table" then
    local unwanted_chunk = cookie_parts + 1
    while true do
        local chunk_name = get_chunk_name(session.name, unwanted_chunk) -- Pull out helper function
        local found = false
        for cookie_index, cookie_content in ipairs(cookie_header) do
            if find(cookie_content, chunk_name, 1, true) == 1 then
                found = true
                table.remove(cookie_header, cookie_index)
                unwanted_chunk = unwanted_chunk + 1
                break
            end
        end
        if not found then break end
    end
end

If you agree that something like this should be done, I would be happy to work on a PR (including tests).

alexdowad commented 2 years ago

Sorry, looks like my analysis of this issue is incorrect. Closing this issue for now. Will re-open if necessary.

alexdowad commented 2 years ago

After further analysis, it seems that maybe there may actually be an issue here, but it's not what I thought it was. To avoid confusion caused by the incorrect report above, I will open a separate ticket...