Skip to content

Fix: ZoneSync delays in some installations caused problems #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 20, 2022

Conversation

route443
Copy link
Contributor

@route443 route443 commented Apr 14, 2022

  • Fix: ZoneSync delays in some installations caused problems

    Fix is slowing down the request processing in case the zone sync is not fast enough. This issue may occur in environments where NGINX cluster nodes can randomly process requests from user agents and there may be a situation where node "A" successfully received a token, and node "B" receives the next request in less than zone_sync_interval.
    Introduced zone_sync_leeway variable that specifies the maximum timeout for synchronizing ID tokens between cluster nodes. The request doesn't wait until it reaches the timeout, if the "session_jwt" variable "appeared" in key-value database before the timeout expires, the request will be released.

  • Fix: Double URL encoding in proxy upstream after internalRedirect

    In some cases the r.internalRedirect(uri) will double encode the URI. This problem may occur if OIDC module needs to update the set of tokens using the refresh token and redirect the user agent to the original request URI.

@route443 route443 requested a review from tippexs April 14, 2022 00:41
Copy link

@alebastr alebastr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but can I convince you to make a cleaner split between the two independent changes?
I.e. commit 1 - introduce retryOriginalRequest and start using it, commit 2 - add waitForSessionSync.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants