Issue with having multiple zones for a single key

[accountForIssues]

So, I'm trying to use this plugin to limit requests to a single user in the following way: 1 req/sec and 10 req/min.

rate_limit {
  zone header_limiting_min {
    key {header.authorization}
    events 10
    window 1m
   }

  zone header_limiting_sec {
  key {header.authorization}
  events 1
  window 1s
  }
}

The issue I'm having is that the plugin seems to be counting failed requests as well so I hit the rate-limit after 10 requests whether they were successful or not.

The other plugin works without this issue but it doesn't support the retry-after header.

Is this a bug or is there a better way of achieving what I want ?

[mholt]

What do you mean by "failed requests"? Can you be more specific?

[accountForIssues]

Sorry, I should have said rate-limited requests. e.g., with the above configuration, I test it by sending 10 req/sec.

Now, I get a success (200 OK) for 2 reqs. satisfying the 1 req/sec but I expect to see success for 10 requests which never happens. I assume this is because by that time more than 10 reqs have been hit.

The retry-after headers also say 59s after about 10-11 requests. Is it possible for it to keep a count of only 200 OK responses in the events ?

Caddy access log:

(python) script that sends the requests:

[mholt]

Well letsee, with the same key for those zones, both rate limits will apply to the same request. I'm not sure what you're trying to accomplish here exactly.

And I'm still unclear what you mean by "failed requests" -- can you please be specific? Do you mean any request that doesn't have a certain response code? By that time the request has already finished, so it can't be rate limited, right?

If you could break down exactly what you're trying to accomplish, that would be productive. I'll pull out a bug report template if that is helpful.

[accountForIssues]

I'm sorry I thought I explained it pretty well with examples. 200 is successful whereas 429 is not.

I want to restrict requests per user to 1 req/s and 10 req/min.

Caddyfile:

limit.{$MY_DOMAIN} {
    rate_limit {
         zone header_limiting_min {
             key {header.authorization}
             events 10
             window 1m
            }
         zone header_limiting_sec {
             key {header.authorization}
             events 1
             window 1s
            }
    	 }
         respond 200
}

I then test it by sending 10 requests / sec. It takes less than a minute to run so I expect to see 10 200 OK responses. I don't.

The plugin starts limiting right after ~1s even though the condition of 10 req/min was never satisfied.

(screenshots in my previous post)

With the other rate_limit plugin, the Caddyfile:

limit.{$MY_DOMAIN} {

    rate_limit {header.authorization} 1r/s
    rate_limit {header.authorization} 10r/m

    respond 200
    
}

Now, I see that it satisfies both conditions i.e., in total 10 requests get the 200 OK whereas all the others get the 429 response.

I can give you the whole log if you like but it's more of the same.

[mholt]

Ahhh, I see. The requests blocked by the more restrictive rate limiter also count towards the more permissive one.

So it sounds like you want the more permissive zone to yield to the more restrictive one, rather than enforcing both simultaneously.

I did some testing on my end.

What happens for you if you rename header_limiting_min to header_limiting_xyz?

[accountForIssues]

YES ! Okay, changing the name is a lot better ( can't believe I didn't think of that myself :) ) but it still doesn't fulfil the conditions.

I tested both plugins with 5 runs of 100 requests each with 0.01 s of sleep time in between each request. In between each run, I fully stopped, removed and recreated the docker containers.

Of the 100 requests,

this plugin always allows 6-7 requests on each run and rate-limits the rest.

the other plugin consistently allows 10 requests on each run and rate-limits the rest.

[mholt]

Okay, changing the name is a lot better ( can't believe I didn't think of that myself :) )

To be fair, this is unintuitive and likely a bug because we are, once again, relying on Go's ordering of object keys in JSON to ensure that the more restrictive zone comes first.

Can you show me a working config for the other rate limit plugin? I'll study and try to see what they do. But it sounds like we need to order the zones by ascending permissiveness. Maybe that is computed simply by doing events / window (i.e. more events == more permissive, thus put at end of list).

Does that sound about right?

this plugin always allows 6-7 requests on each run and rate-limits the rest.

Just to be sure, you're not doing any distributed RL? Distributed RL will be eventually consistent. But internal-only, hmm. I haven't experienced this behavior yet...

[accountForIssues]

For the other plugin, the Caddyfile is just this.

{
    order rate_limit before basicauth
}

limit.{$MY_DOMAIN} {

    rate_limit {header.authorization} 1r/s
    rate_limit {header.authorization} 10r/m

    respond 200
    
}

I don't think the order of the rules matter.

But it sounds like we need to order the zones by ascending permissiveness.

Yeah I would think so as well.

And no, I am not using any distributed limiting.

[mholt]

@accountForIssues So did you confirm, if you reorder those zones in your other-plugin config, does it still behave properly?

Anyway, I think I know how to fix the ordering issue. I'm still not sure why only 6-7 requests get allowed, though. (It works for me?) Let me know if you have any ideas on that one.

[mholt]

@accountForIssues Ok, I've pushed a fix for the sorting problem in 163fde7.

I haven't been able to reproduce the other problem you reported:

this plugin always allows 6-7 requests on each run and rate-limits the rest.

Can you try with the latest commit anyway and maybe provide some more details to reproduce the bug?

Also does it still happen with lower frequency / slower firing off requests? (i.e. maybe like 2 per second over a longer window?)

[accountForIssues]

So did you confirm, if you reorder those zones in your other-plugin config, does it still behave properly?

I stand corrected. The order there does matter. It just so happened that it did the expected behaviour so I didn't test it the other way.

Ok, I've pushed a fix for the sorting problem in 163fde7.

Thank you !

I built a new image for this update using the latest caddy builders and it seems to work exactly how I want. The issue with 6-7 requests is no more. It always allows 10 requests (or the more permissible limit).

I will be testing it more and will update if I encounter something unexpected.

[mholt]

Yay! Thanks for verifying. I will close but feel free to comment if needed!