Conversation
apps/user_ldap/group_ldap.php
Outdated
| return $this->access->connection->getFromCache($cacheKey); | ||
| $inGroup = $this->access->connection->getFromCache($cacheKey); | ||
| if(!is_null($inGroup)) { | ||
| return $inGroup; |
There was a problem hiding this comment.
Do make static code scanners happy like Scrutinizer can you cast this to bool?
* getFromCache is wrapped in isCached * inbetween the two calls the cache entry hits it's TTL * getFromCache returns null * this fix only checkes if the returned value is null and return only non-null values
MorrisJobke
force-pushed
the
fix-ldap-cache-race-conditions
branch
from
5ebfdae to
62a5985
Compare
|
@LukasReschke I addressed your comments :) |
|
@LukasReschke @nickvergessen @blizzz Could we get this tested and merged? I really would like to have this in place. @davitol @owncloud/qa @owncloud/ldap We need some LDAP testing here. |
|
@karlitschek I would like to request a backport of this down to stable8.2 for this race condition. |
|
This fixes race conditions, that are really hard to reproduce. The testing here just involves a verification, that the LDAP connection still works. |
|
nice. please backport 👍 |
|
👍 WFM. Configuring a LDAP and a AD no problems were found |
|
I also got positive feedback. |
|
Integration tests still succeed 👍 |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
6 participants
return only non-null values
cc @blizzz @nickvergessen @LukasReschke