Fix for issue #16 #17

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@mogsie

This fixes #16 but introduces a memory leak. The array of nonces is not just a nonce count, but an array of used nonces, which is pretty sequential. Some added logic to merge sequences would probably avoid the memory leak

@mogsie mogsie Fixes issue 18, but introduces a memory leak (the nonce counts). They…
… do expire after a while, but the overhead might be problematic if you have too many requests.
f2c1aa7
@mogsie

lol, I meant issue #16...

@gevorg
http-auth member

I think this is not the best way to proceed, because idea of nonce count is ignored simply. I will try to find better fix for issue #16. Thanks for your help.

Gevorg.

@gevorg gevorg closed this Apr 28, 2012
@mogsie

Yeah, this pull request was mostly a way of exposing the flaw, I guess.

Would it be better to e.g. merge consecutive nonce counters? Say you get a request with nc 1, 2, 3, 4, 6, 7 and 8. The array could contain tuples in some way indicating the ranges of "used nonce counts": 1..4, 6..8. When a request for nc 5 arrives (due to e.g. network latency) it's all good, and the ranges merge to 1..8.

  • The normal case of requests coming in order would only need a single range.
  • Out of order requests cause temporary holes to appear, but they are sealed whenever the late requests arrive.

Do clients always send sequential nonce counters? Is it required?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment