Hi, r1025 seems to break ldap search with global filters.
Reverted patch follows.
Index: trunk/roundcubemail/program/include/rcube_ldap.inc
--- trunk/roundcubemail/program/include/rcube_ldap.inc (revision 1025) +++ trunk/roundcubemail/program/include/rcube_ldap.inc (revision 787) @@ -1,3 +1,4 @@ <?php
/* +-----------------------------------------------------------------------+ @@ -288,5 +289,5 @@ // add general filter to query if (!empty($this->prop['filter']))
$filter = '(&('.$this->prop['filter'] .')' . $filter . ')';
$filter = '(&'.$this->prop['filter'] . $filter . ')';// set filter string and execute search
ciao, ema _______________________________________________ List info: http://lists.roundcube.net/dev/
On 19/02/2008 12:14 Emanuele Rocca wrote:
$filter = '(&('.$this->prop['filter'] .')' . $filter . ')';
$filter = '(&'.$this->prop['filter'] . $filter . ')';
I don't grok this patch, the property filter _should_ be in brackets - ldap AND syntax being (&(arg)(arg)).
$filter probably also needs to be in brackets.
Hello Colin,
- Colin Alston karnaugh@karnaugh.za.net, [2008-02-19 12:18 +0200]:
On 19/02/2008 12:14 Emanuele Rocca wrote:
$filter = '(&('.$this->prop['filter'] .')' . $filter . ')';
$filter = '(&'.$this->prop['filter'] . $filter . ')';I don't grok this patch, the property filter _should_ be in brackets -
ldap AND syntax being (&(arg)(arg)).
We have 'filter' => '(accountStatus=active)' in main.inc.php, which produces "bad search filters".
So, two options: reverting that patch or stating in main.inc.php that 'filter' should be without surrounding brackets; the current comment is: // &'d with search queries. ex: (status=act).
I'd be for the latter.
Thanks! ciao, ema _______________________________________________ List info: http://lists.roundcube.net/dev/
On Tue, Feb 19, 2008 at 12:33 PM, Emanuele Rocca ema@galliera.it wrote:
Hello Colin,
- Colin Alston karnaugh@karnaugh.za.net, [2008-02-19 12:18 +0200]:
On 19/02/2008 12:14 Emanuele Rocca wrote:
$filter = '(&('.$this->prop['filter'] .')' . $filter . ')';
$filter = '(&'.$this->prop['filter'] . $filter . ')';I don't grok this patch, the property filter _should_ be in brackets - ldap AND syntax being (&(arg)(arg)).
We have 'filter' => '(accountStatus=active)' in main.inc.php, which produces "bad search filters".
So, two options: reverting that patch or stating in main.inc.php that 'filter' should be without surrounding brackets; the current comment is: // &'d with search queries. ex: (status=act).
I'd be for the latter.
Hey guys,
so where do we stand on this? I'd be for "fixing" the comments. In general, I'd like to support the syntax most people are familiar wit and use. So I have no idea about general LDAP usage. But please let us know so we can fix that before we release the next version.
Thanks, Till _______________________________________________ List info: http://lists.roundcube.net/dev/
- till klimpong@gmail.com, [2008-02-22 12:38 +0100]:
On Tue, Feb 19, 2008 at 12:33 PM, Emanuele Rocca ema@galliera.it wrote:
So, two options: reverting that patch or stating in main.inc.php that 'filter' should be without surrounding brackets; the current comment is: // &'d with search queries. ex: (status=act).
I'd be for the latter.
so where do we stand on this? I'd be for "fixing" the comments. In general, I'd like to support the syntax most people are familiar wit and use. So I have no idea about general LDAP usage. But please let us know so we can fix that before we release the next version.
The current code in rcube_ldap.inc is perfectly OK, and I see that the comment has been fixed by thomasb in r1129.
Maybe you could also add an &'d example?
example: &(status=act)(visible=true)
That would clarify the proper usage even further, in my opinion.
Thanks guys. ciao, ema _______________________________________________ List info: http://lists.roundcube.net/dev/
On 19/02/2008 13:33 Emanuele Rocca wrote:
Hello Colin,
- Colin Alston karnaugh@karnaugh.za.net, [2008-02-19 12:18 +0200]:
On 19/02/2008 12:14 Emanuele Rocca wrote:
$filter = '(&('.$this->prop['filter'] .')' . $filter . ')';
$filter = '(&'.$this->prop['filter'] . $filter . ')';I don't grok this patch, the property filter _should_ be in brackets -
ldap AND syntax being (&(arg)(arg)).We have 'filter' => '(accountStatus=active)' in main.inc.php, which produces "bad search filters".
So, two options: reverting that patch or stating in main.inc.php that 'filter' should be without surrounding brackets; the current comment is: // &'d with search queries. ex: (status=act).
Sorry about the hit and run, this seems to have left Till and this issue in limbo.
It's a tough one, but now that I think about it, if people were to do their own & syntax then they might naturally use (&(prop1)(prop2)) which would produce an error case. If I look at Exim for example, it seems to be naturally assumed that any LDAP criteria will be enclosed with brackets.
Perhaps adding to this if there is no leading or trailing bracket (like if someone just added "accountStatus=active"), they should be added automatically. I don't have necessary the PHP fu to check that but I imagine failure matching /^(.*)$/ on $this->prop['filter'] did '('.$this->prop['filter'].')' The reason I feel both should be allowed is that the configuration option might be interpreted as a single attribute filter which is the case in many other tools in which case brackets are undesired - but that is also something which irritates me about those tools. _______________________________________________ List info: http://lists.roundcube.net/dev/
-
Colin Alston -
Emanuele Rocca -
till