:allowed_clock_drift should be bidrectional#605
:allowed_clock_drift should be bidrectional#605pitbulk merged 7 commits intoSAML-Toolkits:masterfrom
Conversation
…efore" and X sec after "NotOnOrAfter"). Also improves readability of associated error messages.
|
Can you add some unittest to cover the cases?. It seems this change has not impacted the test suite and it should, so we may be missing some unitest here |
|
@pitbulk specs done, ready for review |
|
@johnnyshields the specs fail |
0f4a3ac to
3a04a03
Compare
7d0ee4e to
b79859f
Compare
|
@pitbulk this can be merged. the one failing run is not related to this PR |
|
@pitbulk let's merge? |
|
|
Yes, floats have built in inaccuracy (the uncertainty factor is "epsilon") which can cause <= to return false when it should be true. My code extends the clock drift factor by epsilon which errs on the side of being permissive when comparing exact times. |
| # @return [Float] | ||
| def allowed_clock_drift | ||
| return options[:allowed_clock_drift].to_f | ||
| options[:allowed_clock_drift].to_f.abs + Float::EPSILON |
There was a problem hiding this comment.
why the Float::EPSILON is required?
There was a problem hiding this comment.
Yes, floats have built in inaccuracy (the uncertainty factor is "epsilon") which can cause <= to return false when it should be true.
My code extends the clock drift factor by epsilon which errs on the side of being permissive when comparing exact times.
Fixes #599
:allowed_clock_drift should be bidrectional (allow X sec before "NotBefore" and X sec after "NotOnOrAfter"). Also improves readability of associated error messages.