Proposal to Optimize JWT Plugin by Replacing ngx.re.gmatch with string.sub in retrieve_tokens func

[dingjiayi]

Hello Kong Community,

I hope this message finds you well.

I have been working with the JWT plugin in the Kong repo and noticed that the current implementation uses ngx.re.gmatch(token_header, "\\s*[Bb]earer\\s+(.+)") to extract the token from the authorization header. While this approach is effective, I believe it can be optimized further.

In my local tests, I found that using the string.sub function instead of ngx.re.gmatch improves performance.

Here is my testing process to validate this optimization:

• I generated a large number of JWT tokens.

• I iterated over all the tokens and called the retrieve_tokens_from_authorization function to extract the token.

• I measured the time taken for step 2 using both ngx.re.gmatch and string.sub implementations of the retrieve_tokens_from_authorization function.

local ngx = ngx
local ngx_re_gmatch = ngx.re.gmatch
local ngx_now = ngx.now

local CHARSET = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
local CHARSET_LENGTH = #CHARSET
local function generate_random_string(length)
    local random_string = {}
    for i = 1, length do
        local random_index = math.random(1, CHARSET_LENGTH)
        random_string[i] = CHARSET:sub(random_index, random_index)
    end

    return table.concat(random_string)
end

local HEADER_LENGTH, PAYLOAD_LENGTH, SIGNATURE_LENGTH = 128, 1024, 360
local forged_tokens = {}
for i = 1, 200000 do
    table.insert(forged_tokens, string.format("Bearer %s.%s.%s", generate_random_string(HEADER_LENGTH), generate_random_string(PAYLOAD_LENGTH), generate_random_string(SIGNATURE_LENGTH)))
end

-- by ngx.re.gmatch
local function retrieve_tokens_from_authorization(token_header)
    local iterator, _ = ngx_re_gmatch(token_header, "\\s*[Bb]earer\\s+(.+)")
    local m, _ = iterator()
    if m and #m > 0 then
        return m[1]
    end
end

local string_sub = string.sub
local prefix_up, prefix_low = "Bearer ", "bearer "
local prefix_length = #prefix_up
-- by string.sub
local function retrieve_tokens_from_authorization(token_header)
    local prefix = string_sub(token_header, 1, prefix_length)
    if prefix ~= prefix_up and prefix ~= prefix_low then
        return
    end
    return string_sub(token_header, prefix_length + 1)
end

ngx.update_time()
local start_time = ngx_now()
local count = 0
for _, token in ipairs(forged_tokens) do
    local token_payload = retrieve_tokens_from_authorization(token)
    if not token_payload then
        print("Not retrieve token payloay from: ", token)
    end
    if #token_payload == (HEADER_LENGTH + PAYLOAD_LENGTH +SIGNATURE_LENGTH + 2) then
        count = count + 1
    end
end
ngx.update_time()
print("cost: ", ngx_now() - start_time)

The local test data is as follows:

1. The ngx.re.gmatch version took 1.2s
2. The string.sub version took 0.3s.

I open this discussion on whether this optimization can be considered for the JWT plugin.

[hanshuebner]

I open this discussion on whether this optimization can be considered for the JWT plugin.

It can be considered in general. It needs to be functionally equivalent to the current solution, however. Your proposal only works if there is one space between the "[Bb]earer" prefix and the token, where the original allows one or more whitespace characters.

You are invited to providing a pull request with your proposed changes. Please refer to our contributor guidelines to learn.

[dingjiayi]

It needs to be functionally equivalent to the current solution, however. Your proposal only works if there is one space between the "[Bb]earer" prefix and the token, where the original allows one or more whitespace characters.

Yes, my proposed solution only allows for one space between the '[Bb]earer' prefix and the b64token.
This is mainly based on the following reasons:

• According to the RFC6750 , only one space is allowed.

 b64token    = 1*( ALPHA / DIGIT / "-" / "." / "_" / "~" / "+" / "/" ) *"="
 credentials = "Bearer" 1*SP b64token

• From reviewing other repositories, they also assume that there is only one space between Bearer and the b64token.

I will provide a PR for this.

Thx.

[git-hulk]

@dingjiayi 1*SP means it required at least one SPACE instead of only one. For the detailed BNF, please refer to Augmented BNF section in RFC 1945.

[dingjiayi]

It needs to be functionally equivalent to the current solution, however. Your proposal only works if there is one space between the "[Bb]earer" prefix and the token, where the original allows one or more whitespace characters.

Yes, my proposed solution only allows for one space between the '[Bb]earer' prefix and the b64token. This is mainly based on the following reasons:

• According to the RFC6750 , only one space is allowed.

 b64token    = 1*( ALPHA / DIGIT / "-" / "." / "_" / "~" / "+" / "/" ) *"="
 credentials = "Bearer" 1*SP b64token

• From reviewing other repositories, they also assume that there is only one space between Bearer and the b64token.

I will provide a PR for this.

Thx.

@git-hulk Yes, from the RFC1945 *rules description, 1*SP indeed specifies that there must be at least one space.

The character "*" preceding an element indicates repetition.

In practice, it is almost universally assumed that there is only one space. Should we also consider implementing the plugin with the assumption of only one space?

Thank you.

[git-hulk]

To see if @hanshuebner has any comments on this. I think we should obey the RFC definition if it's reasonable.