You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2008/03/07 22:02:42 UTC
svn commit: r634821 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c
Author: covener
Date: Fri Mar 7 13:02:41 2008
New Revision: 634821
URL: http://svn.apache.org/viewvc?rev=634821&view=rev
Log:
* mod_ldap: Correctly return all requested attribute values
when some attributes have a null value.
PR: 44560
Submitted by: Anders Kaseorg <anders kaseorg.com>
Reviewed by: covener
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/ldap/util_ldap.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=634821&r1=634820&r2=634821&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Mar 7 13:02:41 2008
@@ -1,6 +1,10 @@
-*- coding: utf-8 -*-
Changes with Apache 2.3.0
[ When backported to 2.2.x, remove entry from this file ]
+
+ *) mod_ldap: Correctly return all requested attribute values
+ when some attributes have a null value.
+ PR 44560 [Anders Kaseorg <anders kaseorg.com>]
*) core: check symlink ownership if both FollowSymlinks and
SymlinksIfOwnerMatch are set [Nick Kew]
Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?rev=634821&r1=634820&r2=634821&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ldap/util_ldap.c (original)
+++ httpd/httpd/trunk/modules/ldap/util_ldap.c Fri Mar 7 13:02:41 2008
@@ -1462,13 +1462,10 @@
/* ...and entry is valid */
*binddn = apr_pstrdup(r->pool, search_nodep->dn);
if (attrs) {
- int i = 0, k = 0;
- while (attrs[k++]);
- *retvals = apr_pcalloc(r->pool, sizeof(char *) * k);
- while (search_nodep->vals[i]) {
- (*retvals)[i] = apr_pstrdup(r->pool,
- search_nodep->vals[i]);
- i++;
+ int i;
+ *retvals = apr_pcalloc(r->pool, sizeof(char *) * search_nodep->numvals);
+ for (i = 0; i < search_nodep->numvals; i++) {
+ (*retvals)[i] = apr_pstrdup(r->pool, search_nodep->vals[i]);
}
}
LDAP_CACHE_UNLOCK();
@@ -1712,13 +1709,10 @@
/* ...and entry is valid */
*binddn = apr_pstrdup(r->pool, search_nodep->dn);
if (attrs) {
- int i = 0, k = 0;
- while (attrs[k++]);
- *retvals = apr_pcalloc(r->pool, sizeof(char *) * k);
- while (search_nodep->vals[i]) {
- (*retvals)[i] = apr_pstrdup(r->pool,
- search_nodep->vals[i]);
- i++;
+ int i;
+ *retvals = apr_pcalloc(r->pool, sizeof(char *) * search_nodep->numvals);
+ for (i = 0; i < search_nodep->numvals; i++) {
+ (*retvals)[i] = apr_pstrdup(r->pool, search_nodep->vals[i]);
}
}
LDAP_CACHE_UNLOCK();
Re: svn commit: r634821 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c
Posted by Eric Covener <co...@gmail.com>.
On Tue, Mar 18, 2008 at 6:01 PM, Nick Kew <ni...@webthing.com> wrote:
> On Fri, 07 Mar 2008 21:02:42 -0000
> covener@apache.org wrote:
>
>
> > Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c
> > URL:
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?rev=634821&r1=634820&r2=634821&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/ldap/util_ldap.c (original) +++
> > httpd/httpd/trunk/modules/ldap/util_ldap.c Fri Mar 7 13:02:41 2008
> > @@ -1462,13 +1462,10 @@ /* ...and entry is valid */
> > *binddn = apr_pstrdup(r->pool, search_nodep->dn);
> > if (attrs) {
> > - int i = 0, k = 0;
> > - while (attrs[k++]);
> > - *retvals = apr_pcalloc(r->pool, sizeof(char *) *
> > k);
> > - while (search_nodep->vals[i]) {
> > - (*retvals)[i] = apr_pstrdup(r->pool,
> > -
> > search_nodep->vals[i]);
> > - i++;
> > + int i;
> > + *retvals = apr_pcalloc(r->pool, sizeof(char *) *
> > search_nodep->numvals);
> > + for (i = 0; i < search_nodep->numvals; i++) {
> > + (*retvals)[i] = apr_pstrdup(r->pool,
> > search_nodep->vals[i]); }
>
> Um, doesn't that need a not-null test to avoid a segfault?
> If the switch from while(...) to for(...) does anything,
> it must be 'cos there's a null in there, yesno?
Yes, vals[] can be swiss-cheese and the old loop would stop processing
early (attrs[], not vals[], is null terminated per the ldap API)
apr_pstrdup() accomodates for the null parameter down to .9.x
>
> Also, because I CBA to figure it out, I take it the
> search_nodep->vals[i] must live on the stack and/or get
> overwritten, so the copying is necessary?
The copying is necessary because these strings live in the shared
memory util_ldap/mod_ldap cache. The caller (authnz_ldap) doesn't
directly hold a cache lock, so there is no chance later on to copy
this info out safely.
--
Eric Covener
covener@gmail.com
Re: svn commit: r634821 - in /httpd/httpd/trunk: CHANGES
modules/ldap/util_ldap.c
Posted by Nick Kew <ni...@webthing.com>.
On Fri, 07 Mar 2008 21:02:42 -0000
covener@apache.org wrote:
> Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?rev=634821&r1=634820&r2=634821&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ldap/util_ldap.c (original) +++
> httpd/httpd/trunk/modules/ldap/util_ldap.c Fri Mar 7 13:02:41 2008
> @@ -1462,13 +1462,10 @@ /* ...and entry is valid */
> *binddn = apr_pstrdup(r->pool, search_nodep->dn);
> if (attrs) {
> - int i = 0, k = 0;
> - while (attrs[k++]);
> - *retvals = apr_pcalloc(r->pool, sizeof(char *) *
> k);
> - while (search_nodep->vals[i]) {
> - (*retvals)[i] = apr_pstrdup(r->pool,
> -
> search_nodep->vals[i]);
> - i++;
> + int i;
> + *retvals = apr_pcalloc(r->pool, sizeof(char *) *
> search_nodep->numvals);
> + for (i = 0; i < search_nodep->numvals; i++) {
> + (*retvals)[i] = apr_pstrdup(r->pool,
> search_nodep->vals[i]); }
Um, doesn't that need a not-null test to avoid a segfault?
If the switch from while(...) to for(...) does anything,
it must be 'cos there's a null in there, yesno?
Also, because I CBA to figure it out, I take it the
search_nodep->vals[i] must live on the stack and/or get
overwritten, so the copying is necessary?
--
Nick Kew
Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/