IdP Initiate SLO Validate SessionIndex #141
Conversation
Stores the SessionIndex in the cache, Validate SessionIndex on single logout.
|
@simo5 Can you take a look? |
| cache->key[0] = '\0'; | ||
| cache->access = apr_time_now(); | ||
| } | ||
| am_cache_mutex_unlock(r); |
There was a problem hiding this comment.
this is bad practice ... hiding a lock this deep (two nested) into a utility function is a sure way to get bugs later as it will be very difficult to remember and figure out.
As a general rule locking and unlocking should happen in the same calling function, so that it is very explicit what is the context of the locked code, and all error conditions can be properly checked.
Please rework this code to get the unlock out of this call and up a few levels.
There was a problem hiding this comment.
I agree. However there is a reason why I decided on this code.
The code before modification was to lock with am_get_request_session_nameid() and then unlock with am_delete_request_session().
I tried to make it consistent with the existing code.
I couldn't come up with a good fix, so I need someone else to make a better fix.
Internal Server Error when IdP-Initiated Single Logout(#140)
Stores the SessionIndex in the cache, Validate SessionIndex on single logout.