[PATCH] suppress SUEXEC_BIN printfs when suexec disabled

[David Jones]

mod_info and apachectl -V both output SUEXEC info, even if suexec has been
explicitly turned off with --disable-suexec.
example of current apachectl -V after running with ./configure
--disable-suexec:

>bin/apachectl -V
...
Server compiled with
 -D APACHE_MPM_DIR="server/mpm/worker"
 -D APR_HAS_SENDFILE
 -D APR_HAS_MMAP
 -D APR_HAVE_IPV6 (IPv4-mapped addresses enabled)
 -D APR_USE_SYSVSEM_SERIALIZE
 -D APR_USE_PTHREAD_SERIALIZE
 -D SINGLE_LISTEN_UNSERIALIZED_ACCEPT
 -D APR_HAS_OTHER_CHILD
 -D AP_HAVE_RELIABLE_PIPED_LOGS
 -D DYNAMIC_MODULE_LIMIT=128
 -D APR_CHARSET_EBCDIC
 -D HTTPD_ROOT="/apache"
 *-D SUEXEC_BIN="/apache/bin/suexec"


*Note: some existing Apache and third party code assumes that SUEXEC_BIN is
defined, so patch uses a new separate define (AP_HAS_SUEXEC)

Question: Not sure if the expansion in ./configure sets enable_suexec=no for
windows or other platforms as needed, or does there
need to be a #if define(WIN32) || define(BEOS) || define(NETWARE) ...   =>
#define AP_HAS_SUEXEC 0 someplace?


Index: server/main.c
===
--- server/main.c   (revision 60)
+++ server/main.c   (working copy)
@@ -226,7 +226,7 @@
 printf(" -D HTTPD_ROOT=\"" HTTPD_ROOT "\"\n");
 #endif

-#ifdef SUEXEC_BIN
+#if AP_HAS_SUEXEC
 printf(" -D SUEXEC_BIN=\"" SUEXEC_BIN "\"\n");
 #endif

Index: modules/generators/mod_info.c
===
--- modules/generators/mod_info.c   (revision 60)
+++ modules/generators/mod_info.c   (working copy)
@@ -498,7 +498,7 @@
 ap_rputs(" -D HTTPD_ROOT=\"" HTTPD_ROOT "\"\n", r);
 #endif

-#ifdef SUEXEC_BIN
+#if AP_HAS_SUEXEC
 ap_rputs(" -D SUEXEC_BIN=\"" SUEXEC_BIN "\"\n", r);
 #endif

Index: configure.in
===
--- configure.in(revision 60)
+++ configure.in(working copy)
@@ -561,6 +561,13 @@
   progname="httpd"] )

 # SuExec parameters
+if test "$enable_suexec" = "no"; then
+AC_DEFINE(AP_HAS_SUEXEC, 0,
+  [suexec is disabled])
+else
+AC_DEFINE(AP_HAS_SUEXEC, 1,
+  [suexec is enabled])
+fi
 AC_ARG_WITH(suexec-bin,
 APACHE_HELP_STRING(--with-suexec-bin,Path to suexec binary),[
   AC_DEFINE_UNQUOTED(SUEXEC_BIN, "$withval", [Path to suexec binary] )


suexec.patch
Description: Binary data

[PATCH] Response to TRACE garbled from EBCDIC platform

[David Jones]

The response to TRACE when "TraceEnable Off" is not used on an EBCDIC
platform is partially in ASCII and partially in EBCDIC (part readable, part
garbage).

routine ap_send_http_trace in modules/http_filters.c recreates the request
and echoes it back when TRACE is requested.
form_header_field (from apr_table_do) has the necessary EBCDIC conditional
code to translate those fields to ASCII.
But, the request header and ending CRLF are created outside of that call,
and need to translated as well.

Index: httpd.h
===
--- httpd.h (revision 579232)
+++ httpd.h (working copy)
@@ -649,6 +649,8 @@
 #define LF '\n'
 #define CRLF "\r\n"
 #endif /* APR_CHARSET_EBCDIC */
+/* Useful for common code with either platform charset. */
+#define CRLF_ASCII "\015\012"

 /**
  * @defgroup values_request_rec_body Possible values for
request_rec.read_body

Index: http_filters.c
===
--- http_filters.c  (revision 656736)
+++ http_filters.c  (working copy)
@@ -1058,12 +1058,23 @@
 /* Now we recreate the request, and echo it back */

 bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+#if APR_CHARSET_EBCDIC
+{
+char *tmp;
+apr_size_t len;
+len = strlen(r->the_request);
+tmp = apr_pmemdup(r->pool, r->the_request, len);
+ap_xlate_proto_to_ascii(tmp, len);
+apr_brigade_putstrs(bb, NULL, NULL, tmp, CRLF_ASCII, NULL);
+}
+#else
 apr_brigade_putstrs(bb, NULL, NULL, r->the_request, CRLF, NULL);
+#endif
 h.pool = r->pool;
 h.bb = bb;
 apr_table_do((int (*) (void *, const char *, const char *))
  form_header_field, (void *) &h, r->headers_in, NULL);
-apr_brigade_puts(bb, NULL, NULL, CRLF);
+apr_brigade_puts(bb, NULL, NULL, CRLF_ASCII);

 /* If configured to accept a body, echo the body */
 if (bodylen) {


httpd.h.patch
Description: Binary data


http_filters.c.patch
Description: Binary data

Re: PATCH: build failure from ./Makefile due to no ability to add CFLAGS to buildmark compile

[David Jones]

Yes,
updated our build and can now compile and link fine,

Thanks,
David

On 8/2/07, Joe Orton <[EMAIL PROTECTED]> wrote:
>
> On Fri, Jul 27, 2007 at 01:18:08PM -0400, David Jones wrote:
> > zOS needs to compile with extra CFLAGS in order to link correctly.
> > After revisions 153273/153266 to ./Makefile.in there is no ability to
> add
> > any flags as buildmark.c is made without them  directly using compile,
> i.e:
> >
> > PROGRAM_PRELINK = $(COMPILE) $(top_srcdir)/server/buildmark.c
>
> COMPILE does already include ALL_CFLAGS etc, though.  Is this really
> just a failure to link the non-libtool-built object?
>
> joe
>

PATCH: build failure from ./Makefile due to no ability to add CFLAGS to buildmark compile

[David Jones]

zOS needs to compile with extra CFLAGS in order to link correctly.
After revisions 153273/153266 to ./Makefile.in there is no ability to add
any flags as buildmark.c is made without them  directly using compile, i.e:

PROGRAM_PRELINK = $(COMPILE) $(top_srcdir)/server/buildmark.c

This patch will allow any system that needs extra flags on compile to work,
while not affecting gcc compiles. Tested on several flavors of linux, aix,
zOS

>svn diff Makefile.in 
Index: Makefile.in 
===
--- Makefile.in  (revision 494665)
+++ Makefile.in  (working copy)
@@ -4,8 +4,8 @@

 PROGRAM_NAME = $(progname)
 PROGRAM_SOURCES  = modules.c
-PROGRAM_LDADD= buildmark.o $(HTTPD_LDFLAGS) $(PROGRAM_DEPENDENCIES)
$(EXTRA_LIBS) $(AP_LIBS) $(LIBS)
-PROGRAM_PRELINK  = $(COMPILE) -c $(top_srcdir)/server/buildmark .c
+PROGRAM_LDADD= buildmark.lo $(HTTPD_LDFLAGS)
$(PROGRAM_DEPENDENCIES) $(EXTRA_LIBS) $(AP_LIBS) $(LIBS)
+PROGRAM_PRELINK  = $(LIBTOOL) --mode=compile $(COMPILE)  -c
$(top_srcdir)/server/buildmark.c
 PROGRAM_DEPENDENCIES = \
   server/libmain.la \
   $(BUILTIN_LIBS) \

status of incorrect use of an enum in util_ldap.h struct PATCH?

[David Jones]

I put 2 posts out awhile ago and haven't seen anything on them, can someone
commit/comment on them, or should i bugzilla?

post on Feb 23: PATCH: incorrect use of an enum in util_ldap.h struct

http://mail-archives.apache.org/mod_mbox/httpd-dev/200702.mbox/[EMAIL PROTECTED]

post on Feb 19: Remove dead defines in util_ldap.c

http://mail-archives.apache.org/mod_mbox/httpd-dev/200702.mbox/[EMAIL PROTECTED]

Re: PATCH: support utilities should enable crypt() , current htdbm checks broken

[David Jones]

patch for trunk:



Index: htdbm.c
===
--- htdbm.c(revision 526861)
+++ htdbm.c(working copy)
@@ -69,7 +69,7 @@
#define ALG_APMD5 1
#define ALG_APSHA 2

-#if APR_HAVE_CRYPT_H
+#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
#define ALG_CRYPT 3
#endif

@@ -311,12 +311,12 @@
case ALG_PLAIN:
/* XXX this len limitation is not in sync with any HTTPd len.
*/
apr_cpystrn(cpw,htdbm->userpass,sizeof(cpw));
-#if APR_HAVE_CRYPT_H
+#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
fprintf(stderr, "Warning: Plain text passwords aren't supported
by the "
"server on this platform!\n");
#endif
break;
-#if APR_HAVE_CRYPT_H
+#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
case ALG_CRYPT:
(void) srand((int) time((time_t *) NULL));
to64(&salt[0], rand(), 8);
@@ -347,7 +347,7 @@
static void htdbm_usage(void)
{

-#if APR_HAVE_CRYPT_H
+#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
#define CRYPT_OPTION "d"
#else
#define CRYPT_OPTION ""
@@ -367,7 +367,7 @@
fprintf(stderr, "   -c   Create a new database.\n");
fprintf(stderr, "   -n   Don't update database; display results on
stdout.\n");
fprintf(stderr, "   -m   Force MD5 encryption of the password
(default).\n");
-#if APR_HAVE_CRYPT_H
+#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
fprintf(stderr, "   -d   Force CRYPT encryption of the password (now
deprecated).\n");
#endif
fprintf(stderr, "   -p   Do not encrypt the password (plaintext).\n");
@@ -474,7 +474,7 @@
case 's':
h->alg = ALG_APSHA;
break;
-#if APR_HAVE_CRYPT_H
+#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
case 'd':
h->alg = ALG_CRYPT;
    break;


On 4/4/07, Jeff Trawick <[EMAIL PROTECTED]> wrote:


On 4/4/07, Jeff Trawick <[EMAIL PROTECTED]> wrote:
> On 3/23/07, David Jones <[EMAIL PROTECTED]> wrote:
> > ok here's the simple patch at the 2.0.x level that just checks
platforms for
> > htdbm.c

> Can you post a post to htdbm.c at trunk?

whoops, make that "Can you post a PATCH..."



htdbm.trunk.patch
Description: Binary data

Re: PATCH: support utilities should enable crypt() , current htdbm checks broken

[David Jones]

ok here's the simple patch at the 2.0.x level that just checks platforms for
htdbm.c

Also appended is the semi-related patch for htpasswd.c that adds TPF to the
platforms checked in 2 cases where its missed, which seems like an
oversight.

===
--- htdbm.c(revision 521875)
+++ htdbm.c(working copy)
@@ -66,7 +66,7 @@
#define ALG_APMD5 1
#define ALG_APSHA 2

-#if APR_HAVE_CRYPT_H
+#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
#define ALG_CRYPT 3
#endif

@@ -309,7 +309,7 @@
/* XXX this len limitation is not in sync with any HTTPd len.
*/
apr_cpystrn(cpw,htdbm->userpass,sizeof(cpw));
break;
-#if APR_HAVE_CRYPT_H
+#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
case ALG_CRYPT:
(void) srand((int) time((time_t *) NULL));
to64(&salt[0], rand(), 8);
@@ -340,7 +340,7 @@
static void htdbm_usage(void)
{

-#if APR_HAVE_CRYPT_H
+#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
#define CRYPT_OPTION "d"
#else
#define CRYPT_OPTION ""
@@ -360,7 +360,7 @@
fprintf(stderr, "   -c   Create a new database.\n");
fprintf(stderr, "   -n   Don't update database; display results on
stdout.\n");
fprintf(stderr, "   -m   Force MD5 encryption of the password
(default).\n");
-#if APR_HAVE_CRYPT_H
+#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
fprintf(stderr, "   -d   Force CRYPT encryption of the password (now
deprecated).\n");
#endif
fprintf(stderr, "   -p   Do not encrypt the password (plaintext).\n");
@@ -467,7 +467,7 @@
case 's':
h->alg = ALG_APSHA;
break;
-#if APR_HAVE_CRYPT_H
+#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
case 'd':
h->alg = ALG_CRYPT;
break;




On 3/23/07, Jeff Trawick <[EMAIL PROTECTED]> wrote:


On 3/20/07, William A. Rowe, Jr. <[EMAIL PROTECTED]> wrote:
> Jeff Trawick wrote:
> > On 3/20/07, William A. Rowe, Jr. <[EMAIL PROTECTED]> wrote:
> >> httpd does not ;-)
> >
> > httpd the project (vs. apr, apr-util), not httpd the program (vs.
> > htdbm, htpasswd)
> >
> > as in "In httpd, we don't call crypt(), we call APR..."
>
> So... what I suggest is;
>
> 1. use the same test from htpasswd to determine if crypt is used for
>htdbm from 2.0 - 2.2-branch.  E.g. which platforms?
>
> 2. use the APR_HAVE_CRYPT_H just to decide to include crypt.h, and the
>UNISTD test for unistd.h.
>
> 3. for trunk (2.4) forwards, add a new macro to APR trunk (1.3.x) that
>would 'reveal' if apr_password_* API's include crypt() support.
>
> Does that sound sane?

sounds sane to me...

looking for crypt() (issue #1) is cuter, but as it isn't the perfect
solution anyway (issue #4) then don't bother with something that has a
remote potential of hiding crypt() from somebody who has it today



htdbm.checkplatforms.patch
Description: Binary data


htpasswd.missingTPF.patch
Description: Binary data

Re: PATCH: support utilities should enable crypt() , current htdbm checks broken

[David Jones]

Re: Jeff's last note, yes httpd does call crypt() directly,

Any other questions that need discussion / resolution?

David Jones
--
[EMAIL PROTECTED]


On 3/16/07, Jeff Trawick <[EMAIL PROTECTED]> wrote:


On 3/16/07, William A. Rowe, Jr. <[EMAIL PROTECTED]> wrote:
> Jeff Trawick wrote:
> >
> > APR doesn't pretend to figure out for APR apps exactly what the system
> > provides, though there is currently a spotty set of APR_HAS_foo.
> >
> > Meanwhile, httpd goes and searches on its own for things APR doesn't
> > tell anyone about.  I'm curious about other opinions on whether or not
> > it is APR's job to tell what is available.
>
> In httpd, we don't call crypt(), we call APR

maybe this is the point of confusion...

httpd does call crypt()

$ grep crypt support/*c
...
support/htdbm.c:apr_cpystrn(cpw, crypt(htdbm->userpass,
salt), sizeof(cpw) - 1);
...
support/htpasswd.c:apr_cpystrn(cpw, crypt(pw, salt), sizeof(cpw) -
1);
...

PATCH: support utilities should enable crypt() , current htdbm checks broken

[David Jones]

Support utilities should enable crypt() iff it is available.
Using the presence of  does not reliably determine if crypt() is
available.
Specifically z/OS supports crypt, but does not have , so it is
broken when checking APR_HAVE_CRYPT_H.

Added crypt to AC_CHECK_FUNCS in httpd's configure.in, this creates a
HAVE_CRYPT define. Then changed the checks in htpasswd.c and htdbm.c to
check HAVE_CRYPT.

This will let htdbm.c determine crypt() support accurately (it currently
checks APR_HAVE_CRYPT_H) and htpasswd.c use a more concise and consistent
check (it currently checks if OS = WIN32 || TPF || NETWARE)


Note: This also fixes a TPF bug as they need to switch from crypt to MD5
like the other systems who don't have crypt:
  Currently the check to automatically switch from using crypt to md5 is:
#if !(defined(WIN32) || defined(NETWARE))
  All other checks for not supporting crypt in htdbm.c are:
#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
  From the man page for htpasswd:
  -d Use crypt() encryption for passwords. The default on  all
plat-
 forms but Windows, Netware and TPF. Though possibly
supported by
 htpasswd on all platforms, it is  not  supported  by  the
httpd
 server on Windows, Netware and TPF



Index: configure.in
===
--- configure.in(revision 518254)
+++ configure.in(working copy)
@@ -389,6 +389,7 @@

dnl ## Check for library functions
AC_SEARCH_LIBS(sqrt, m)
+AC_SEARCH_LIBS(crypt, crypt ufc)

dnl See Comment #Spoon

@@ -399,6 +400,7 @@
bindprocessor \
prctl \
timegm \
+crypt
)

dnl confirm that a void pointer is large enough to store a long integer

Index: support/htdbm.c
===
--- support/htdbm.c(revision 494665)
+++ support/htdbm.c(working copy)
@@ -29,6 +29,7 @@
#include "apr_file_info.h"
#include "apr_pools.h"
#include "apr_signal.h"
+#include "ap_config.h"
#include "apr_md5.h"
#include "apr_sha1.h"
#include "apr_dbm.h"
@@ -69,7 +70,7 @@
#define ALG_APMD5 1
#define ALG_APSHA 2

-#if APR_HAVE_CRYPT_H
+#ifdef HAVE_CRYPT
#define ALG_CRYPT 3
#endif

@@ -311,12 +312,12 @@
case ALG_PLAIN:
/* XXX this len limitation is not in sync with any HTTPd len.
*/
apr_cpystrn(cpw,htdbm->userpass,sizeof(cpw));
-#if APR_HAVE_CRYPT_H
+#ifdef HAVE_CRYPT
fprintf(stderr, "Warning: Plain text passwords aren't supported
by the "
"server on this platform!\n");
#endif
break;
-#if APR_HAVE_CRYPT_H
+#ifdef HAVE_CRYPT
case ALG_CRYPT:
(void) srand((int) time((time_t *) NULL));
to64(&salt[0], rand(), 8);
@@ -347,7 +348,7 @@
static void htdbm_usage(void)
{

-#if APR_HAVE_CRYPT_H
+#ifdef HAVE_CRYPT
#define CRYPT_OPTION "d"
#else
#define CRYPT_OPTION ""
@@ -367,7 +368,7 @@
fprintf(stderr, "   -c   Create a new database.\n");
fprintf(stderr, "   -n   Don't update database; display results on
stdout.\n");
fprintf(stderr, "   -m   Force MD5 encryption of the password
(default).\n");
-#if APR_HAVE_CRYPT_H
+#ifdef HAVE_CRYPT
fprintf(stderr, "   -d   Force CRYPT encryption of the password (now
deprecated).\n");
#endif
fprintf(stderr, "   -p   Do not encrypt the password (plaintext).\n");
@@ -474,7 +475,7 @@
case 's':
h->alg = ALG_APSHA;
break;
-#if APR_HAVE_CRYPT_H
+#ifdef HAVE_CRYPT
case 'd':
h->alg = ALG_CRYPT;
break;

Index: support/htpasswd.c
===
--- support/htpasswd.c(revision 494665)
+++ support/htpasswd.c(working copy)
@@ -45,6 +45,7 @@
#include "apr_file_io.h"
#include "apr_general.h"
#include "apr_signal.h"
+#include "ap_config.h"

#if APR_HAVE_STDIO_H
#include 
@@ -175,7 +176,7 @@
apr_cpystrn(cpw,pw,sizeof(cpw));
break;

-#if !(defined(WIN32) || defined(NETWARE))
+#ifdef HAVE_CRYPT
case ALG_CRYPT:
default:
(void) srand((int) time((time_t *) NULL));
@@ -215,12 +216,12 @@
apr_file_printf(errfile, " -n  Don't update file; display results on "
"stdout." NL);
apr_file_printf(errfile, " -m  Force MD5 encryption of the password"
-#if defined(WIN32) || defined(TPF) || defined(NETWARE)
+#ifndef HAVE_CRYPT
" (default)"
#endif
"." NL);
apr_file_printf(errfile, " -d  Force CRYPT encryption of the password"
-#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE)))
+#ifdef HAVE_CRYPT
" (default)"
#endif
"." NL);
@@ -435,14 +436,14 @@
check_args(pool, argc, argv, &alg, &mask, &user, &pwfilename,
&password);


-#if defined(WIN32) || defined(NETWARE)
+#ifndef HAVE_CRYPT
if (alg == ALG_CRYPT) {
alg = ALG_APMD5;
apr_file_printf(errfile, "Automatically using MD5 format." NL);
}
#endif

-

Re: util_ldap.c use of hardcoded sizelimit on ldap_search_ext_s causing error

[David Jones]

 NULL, NULL, NULL, APR_LDAP_SIZELIMIT,
&res))
== LDAP_SERVER_DOWN)
{
ldc->reason = "ldap_search_ext_s() for user failed with server
down";



On 3/8/07, Brad Nicholes <[EMAIL PROTECTED]> wrote:


Please submit a complete patch against trunk for the apr-util code that
includes the ZOS define.  This should include the makefile magic that
defines APR_HAS_ZOS_LDAPSDK as well.  Also include a patch for util_ldap.c
that will define APR_LDAP_SIZELIMIT if the version of apr-util does not
include the #define.

Brad

>>> On Wed, Mar 7, 2007 at  8:36 AM, in message
<[EMAIL PROTECTED]>, "David
Jones"
<[EMAIL PROTECTED]> wrote:
> Patch to commit if no further comments.
> Note that it does not have the ZOS define yet, and does not synch apr-
util
> with httpd.
>   to avoid synch problems i could add to util_ldap:
> #ifndef APR_LDAP_SIZELIMIT
> #define APR_LDAP_SIZELIMIT - 1
> #endif
>
>
>
> Index: modules/ldap/util_ldap.c
> ==
>>
>> =
>> ---  modules/ldap/util_ldap.c(revision 510991)
>> +++ modules/ldap/util_ldap.c(working copy)
>> @@ - 52,9 +52,6 @@
>>  #define LDAP_CA_TYPE_BASE64 2
>>  #define LDAP_CA_TYPE_CERT7_DB   3
>>
>> - #ifndef LDAP_NO_LIMIT
>> - #define LDAP_NO_LIMIT - 1
>> - #endif
>>
>>  module AP_MODULE_DECLARE_DATA ldap_module;
>>
>> @@ - 660,7 +657,7 @@
>>  /* search for reqdn */
>>  if ((result = ldap_search_ext_s(ldc- >ldap, (char *)reqdn,
>> LDAP_SCOPE_BASE,
>>  "(objectclass=*)", NULL, 1,
>> - NULL, NULL, NULL, LDAP_NO_LIMIT,
>> &res))
>> +NULL, NULL, NULL,
APR_LDAP_SIZELIMIT,
>> &res))
>>  == LDAP_SERVER_DOWN)
>>  {
>>  ldc- >reason = "DN Comparison ldap_search_ext_s() "
>> @@ - 938,7 +935,7 @@
>>  if ((result = ldap_search_ext_s(ldc- >ldap,
>>  (char *)basedn, scope,
>>  (char *)filter, attrs, 0,
>> - NULL, NULL, NULL, LDAP_NO_LIMIT,
>> &res))
>> +NULL, NULL, NULL,
APR_LDAP_SIZELIMIT,
>> &res))
>>  == LDAP_SERVER_DOWN)
>>  {
>>  ldc- >reason = "ldap_search_ext_s() for user failed with
server
>> down";
>> @@ - 1178,7 +1175,7 @@
>>  if ((result = ldap_search_ext_s(ldc- >ldap,
>>  (char *)basedn, scope,
>>  (char *)filter, attrs, 0,
>> - NULL, NULL, NULL, LDAP_NO_LIMIT,
>> &res))
>> +NULL, NULL, NULL,
APR_LDAP_SIZELIMIT,
>> &res))
>>  == LDAP_SERVER_DOWN)
>>  {
>>  ldc- >reason = "ldap_search_ext_s() for user failed with
server
>> down";
>> Index: apr- util/include/apr_ldap.h.in
>> ===
>> ---  apr- util/include/apr_ldap.h.in(revision 515593)
>> +++ apr- util/include/apr_ldap.h.in(working copy)
>> @@ - 93,6 +93,15 @@
>>  #define LDAPS_PORT 636  /* ldaps:/// default LDAP over TLS port */
>>  #endif
>>
>> +/*
>> + * For ldap function calls that input a size limit on the number of
>> returned entries.
>> + * Some SDKs do not have the define for LDAP_DEFAULT_LIMIT (- 1) or
>> LDAP_NO_LIMIT (0)
>> + */
>> +#ifdef LDAP_DEFAULT_LIMIT
>> +#define APR_LDAP_SIZELIMIT LDAP_DEFAULT_LIMIT
>> +#else
>> +#define APR_LDAP_SIZELIMIT - 1 /* equivalent to LDAP_DEFAULT_LIMIT */
>> +#endif
>>
>>  /* Note: Macros defining const casting has been removed in APR v1.0,
>>   * pending real support for LDAP v2.0 toolkits.
>
>
>
> On 3/2/07, Brad Nicholes <[EMAIL PROTECTED]> wrote:
>>
>> Looks good, I think I like your first suggestion better, putting the
>> #ifdef in apr_ldap.h.in.  This seems a little more straight forward
rather
>> than hiding the value in configure.
>>
>> Brad
>>
>> >>> On 3/1/2007 at 7:07 PM, in message
>> <[EMAIL PROTECTED]>, "David
>> Jones"
>> <[EMAIL PROTECTED]> wrote:
>> > How about:
>> > changes to apr_ldap.h.in:
>> > #define APR_HAS_ZOS_LDAPSDK   @apu_has_ldap_zos@
>> &g

Re: util_ldap.c use of hardcoded sizelimit on ldap_search_ext_s causing error

[David Jones]

Patch to commit if no further comments.
Note that it does not have the ZOS define yet, and does not synch apr-util
with httpd.
 to avoid synch problems i could add to util_ldap:
#ifndef APR_LDAP_SIZELIMIT
#define APR_LDAP_SIZELIMIT -1
#endif



Index: modules/ldap/util_ldap.c
==


=
--- modules/ldap/util_ldap.c(revision 510991)
+++ modules/ldap/util_ldap.c(working copy)
@@ -52,9 +52,6 @@
 #define LDAP_CA_TYPE_BASE64 2
 #define LDAP_CA_TYPE_CERT7_DB   3

-#ifndef LDAP_NO_LIMIT
-#define LDAP_NO_LIMIT -1
-#endif

 module AP_MODULE_DECLARE_DATA ldap_module;

@@ -660,7 +657,7 @@
 /* search for reqdn */
 if ((result = ldap_search_ext_s(ldc->ldap, (char *)reqdn,
LDAP_SCOPE_BASE,
 "(objectclass=*)", NULL, 1,
-NULL, NULL, NULL, LDAP_NO_LIMIT,
&res))
+NULL, NULL, NULL, APR_LDAP_SIZELIMIT,
&res))
 == LDAP_SERVER_DOWN)
 {
 ldc->reason = "DN Comparison ldap_search_ext_s() "
@@ -938,7 +935,7 @@
 if ((result = ldap_search_ext_s(ldc->ldap,
 (char *)basedn, scope,
 (char *)filter, attrs, 0,
-NULL, NULL, NULL, LDAP_NO_LIMIT,
&res))
+NULL, NULL, NULL, APR_LDAP_SIZELIMIT,
&res))
 == LDAP_SERVER_DOWN)
 {
 ldc->reason = "ldap_search_ext_s() for user failed with server
down";
@@ -1178,7 +1175,7 @@
 if ((result = ldap_search_ext_s(ldc->ldap,
 (char *)basedn, scope,
 (char *)filter, attrs, 0,
-NULL, NULL, NULL, LDAP_NO_LIMIT,
&res))
+NULL, NULL, NULL, APR_LDAP_SIZELIMIT,
&res))
 == LDAP_SERVER_DOWN)
 {
 ldc->reason = "ldap_search_ext_s() for user failed with server
down";
Index: apr-util/include/apr_ldap.h.in
===
--- apr-util/include/apr_ldap.h.in(revision 515593)
+++ apr-util/include/apr_ldap.h.in(working copy)
@@ -93,6 +93,15 @@
 #define LDAPS_PORT 636  /* ldaps:/// default LDAP over TLS port */
 #endif

+/*
+ * For ldap function calls that input a size limit on the number of
returned entries.
+ * Some SDKs do not have the define for LDAP_DEFAULT_LIMIT (-1) or
LDAP_NO_LIMIT (0)
+ */
+#ifdef LDAP_DEFAULT_LIMIT
+#define APR_LDAP_SIZELIMIT LDAP_DEFAULT_LIMIT
+#else
+#define APR_LDAP_SIZELIMIT -1 /* equivalent to LDAP_DEFAULT_LIMIT */
+#endif

 /* Note: Macros defining const casting has been removed in APR v1.0,
  * pending real support for LDAP v2.0 toolkits.




On 3/2/07, Brad Nicholes <[EMAIL PROTECTED]> wrote:


Looks good, I think I like your first suggestion better, putting the
#ifdef in apr_ldap.h.in.  This seems a little more straight forward rather
than hiding the value in configure.

Brad

>>> On 3/1/2007 at 7:07 PM, in message
<[EMAIL PROTECTED]>, "David
Jones"
<[EMAIL PROTECTED]> wrote:
> How about:
> changes to apr_ldap.h.in:
> #define APR_HAS_ZOS_LDAPSDK   @apu_has_ldap_zos@
>
> #if APR_LDAP_HAS_ZOS_LDAPSDK
> #define APR_LDAP_SIZELIMIT  LDAP_NO_LIMIT
> #else
> #ifdef LDAP_DEFAULT_LIMIT
> #define APR_LDAP_SIZELIMIT LDAP_DEFAULT_LIMIT
> #else
> #define APR_LDAP_SIZELIMIT -1 /* equivalent to LDAP_DEFAULT_LIMIT */
> #endif
> #endif
>
>
> This part of  the util_ldap.c patch at the bottom could allow
util_ldap.c to
> compile regardless of apr-util level, but would not typically commit it?
> +#ifndef APR_LDAP_SIZELIMIT
> +#define APR_LDAP_SIZELIMIT -1
>  #endif
>
>
>
> Or could add info to apu-conf.m4 for each SDK, eliminating the need for
the
> ZOS specific #if (would just need #define APR_LDAP_SIZELIMIT
> @apu_ldap_sizelimit)
> (If get any input from other SDKs then could replace its  -1 with
> LDAP_DEFAULT_LIMIT or LDAP_NO_LIMIT as i did for z/OS)
>
> Index: apu-conf.m4
> ===
> RCS file: /m0xa/cvs/phoenix/2.2.4/srclib/apr-util/build/apu-conf.m4,v
> retrieving revision 1.2
> diff -u -d -b -r1.2 apu-conf.m4
> --- apu-conf.m4 12 Feb 2007 18:19:20 -  1.2
> +++ apu-conf.m4 1 Mar 2007 20:07:26 -
>
> @@ -267,10 +273,13 @@
>  apu_has_ldap_sslinit="0"
>  apu_has_ldapssl_install_routines="0"
>  apu_has_ldap_openldap="0"
>  +apu_has_ldap_sizelimit="0"
> @@ -354,42 +363,57 @@
>AC_EGREP_CPP([OpenLDAP], [$lber_h
> $ldap_h
> LDAP_VENDOR_NAME

Re: util_ldap.c use of hardcoded sizelimit on ldap_search_ext_s causing error

[David Jones]

How about:
changes to apr_ldap.h.in:
#define APR_HAS_ZOS_LDAPSDK   @apu_has_ldap_zos@

#if APR_LDAP_HAS_ZOS_LDAPSDK
#define APR_LDAP_SIZELIMIT  LDAP_NO_LIMIT
#else
#ifdef LDAP_DEFAULT_LIMIT
#define APR_LDAP_SIZELIMIT LDAP_DEFAULT_LIMIT
#else
#define APR_LDAP_SIZELIMIT -1 /* equivalent to LDAP_DEFAULT_LIMIT */
#endif
#endif


This part of  the util_ldap.c patch at the bottom could allow util_ldap.c to
compile regardless of apr-util level, but would not typically commit it?
+#ifndef APR_LDAP_SIZELIMIT
+#define APR_LDAP_SIZELIMIT -1
#endif



Or could add info to apu-conf.m4 for each SDK, eliminating the need for the
ZOS specific #if (would just need #define APR_LDAP_SIZELIMIT
@apu_ldap_sizelimit)
(If get any input from other SDKs then could replace its  -1 with
LDAP_DEFAULT_LIMIT or LDAP_NO_LIMIT as i did for z/OS)

Index: apu-conf.m4
===
RCS file: /m0xa/cvs/phoenix/2.2.4/srclib/apr-util/build/apu-conf.m4,v
retrieving revision 1.2
diff -u -d -b -r1.2 apu-conf.m4
--- apu-conf.m4 12 Feb 2007 18:19:20 -  1.2
+++ apu-conf.m4 1 Mar 2007 20:07:26 -

@@ -267,10 +273,13 @@
apu_has_ldap_sslinit="0"
apu_has_ldapssl_install_routines="0"
apu_has_ldap_openldap="0"
+apu_has_ldap_sizelimit="0"
@@ -354,42 +363,57 @@
  AC_EGREP_CPP([OpenLDAP], [$lber_h
   $ldap_h
   LDAP_VENDOR_NAME], [apu_has_ldap_openldap="1"
+   apu_ldap_sizelimit="-1"
   apr_cv_ldap_toolkit="OpenLDAP"])
fi
if test "x$apr_cv_ldap_toolkit" = "x"; then
  AC_EGREP_CPP([Sun Microsystems Inc.], [$lber_h
   $ldap_h
   LDAP_VENDOR_NAME], [apu_has_ldap_solaris="1"
+   apu_ldap_sizelimit="-1"
   apr_cv_ldap_toolkit="Solaris"])
fi
if test "x$apr_cv_ldap_toolkit" = "x"; then
  AC_EGREP_CPP([Novell], [$lber_h
   $ldap_h
   LDAP_VENDOR_NAME], [apu_has_ldap_novell="1"
+   apu_ldap_sizelimit="-1"
   apr_cv_ldap_toolkit="Novell"])
fi
if test "x$apr_cv_ldap_toolkit" = "x"; then
  AC_EGREP_CPP([Microsoft Corporation.], [$lber_h
   $ldap_h
   LDAP_VENDOR_NAME], [apu_has_ldap_microsoft="1"
+   apu_ldap_sizelimit="-1"

apr_cv_ldap_toolkit="Microsoft"])
fi
if test "x$apr_cv_ldap_toolkit" = "x"; then
  AC_EGREP_CPP([Netscape Communications Corp.], [$lber_h
   $ldap_h
   LDAP_VENDOR_NAME], [apu_has_ldap_netscape="1"
+   apu_ldap_sizelimit="-1"
   apr_cv_ldap_toolkit="Netscape"])
fi
if test "x$apr_cv_ldap_toolkit" = "x"; then
  AC_EGREP_CPP([mozilla.org], [$lber_h
   $ldap_h
   LDAP_VENDOR_NAME], [apu_has_ldap_mozilla="1"
+   apu_ldap_sizelimit="-1"
   apr_cv_ldap_toolkit="Mozilla"])
fi
if test "x$apr_cv_ldap_toolkit" = "x"; then
+  AC_EGREP_CPP([IBM], [$lber_h
+   $ldap_h
+   LDAP_VENDOR_NAME], [apu_has_ldap_zos="1"
+
apu_ldap_sizelimit="LDAP_NO_LIMIT"
+   apr_cv_ldap_toolkit="ZOS"])
+fi
+if test "x$apr_cv_ldap_toolkit" = "x"; then
  apu_has_ldap_other="1"
+  apu_ldap_sizelimit="-1"
  apr_cv_ldap_toolkit="unknown"
fi
+
  ])
fi

@@ -398,15 +422,20 @@
LIBS=$save_libs
  ])

+AC_SUBST(apu_ldap_sizelimit)
AC_SUBST(ldap_h)
AC_SUBST(lber_h)
AC_SUBST(ldap_ssl_h)

@@ -415,6 +444,7 @@
AC_SUBST(apu_has_ldap_microsoft)
AC_SUBST(apu_has_ldap_netscape)
AC_SUBST(apu_has_ldap_mozilla)
+AC_SUBST(apu_has_ldap_zos)
AC_SUBST(apu_has_ldap_other)

])




And finally this same either way except for the question on #ifndef
APR_LDAP_SIZELIMIT
Index: util_ldap.c
===
RCS file: /m0xa/cvs/phoenix/2.2.4/modules/ldap/util_ldap.c,v
retrieving revision 1.3
diff -u -d -b -r1.3 util_ldap.c
--- util_ldap.c 15 Feb 2007 18:55:41 -  1.3
+++ util_ldap.c 1 Mar 2007 20:19:39 -
@@ -45,15 +45,8 @@
#include "unixd.h"
#endif

-#ifndef LDAP_NO_LIMIT
-#define LDAP_NO_LIMIT -1
+#ifndef APR_LDAP_SIZELIMIT
+#define APR_LDAP_SIZELIMIT -1
#endif

module AP_MODULE_DECLARE_DATA ldap_module;
@@ -681,7 +681,7 @@
/* search for reqdn */
if ((result = ldap_search_ext_s(ldc->ldap, (char *)reqdn,
LDAP_SCOPE_BASE,
"(objectclass=*)", NULL, 1,
-NULL, NULL, 

Re: util_ldap.c use of hardcoded sizelimit on ldap_search_ext_s causing error

[David Jones]

Sorry for the delay.
We use our own z/OS specific SDK. There is also a Tivoli SDK , [see Eric
Covener's appends and
http://issues.apache.org/bugzilla/attachment.cgi?id=19394  waiting for
input], which shares some commonality with z/OS  (Tivoli can accept the -1
without a problem, but it acts like 0).

Thoughts are:


1) LDAP_HAS_ZOS_LDAPSDK isn't an apache define yet. (The Tivoli append adds
a LDAP_HAS_TIVOLI_LDAPSDK to apu-conf.m4, and we would do similar). So if it
shouldn't be put in svn yet skip the top 3 lines and what we're left with
isn't much different than the original hardcoded -1, but at least it puts
some doc in the code about whats going on.

#ifdef LDAP_HAS_ZOS_LDAPSDK
#define LDAP_LIMIT_VALUE LDAP_NO_LIMIT
#else
#ifdef LDAP_DEFAULT_LIMIT
#define LDAP_LIMIT_VALUE LDAP_DEFAULT_LIMIT
#else
#define LDAP_LIMIT_VALUE -1 /* equivalent to LDAP_DEFAULT_LIMIT */
#endif
#endif

2)Or the flipside, assuming everyone else who defines 0 and not -1 wants to
use 0:

#ifdef LDAP_HAS_NOVELL_LDAPSDK
#define LDAP_LIMIT_VALUE -1
#else
#ifdef LDAP_DEFAULT_LIMIT
#define LDAP_LIMIT_VALUE LDAP_DEFAULT_TIME
#else
#ifdef LDAP_NO_LIMIT
#define LDAP_LIMIT_VALUE LDAP_NO_LIMIT
#else
#define LDAP_LIMIT_VALUE -1
#endif
#endif
#endif

3) Or maybe moving it and define a APR_LDAP_DEFAULT_SIZELIMIT instead of
keeping it in util_ldap.c

4) Or some complicated(?) conf magic that would involve getting a handle and
then calling ldap_set_option(ldap, LDAP_OPT_SIZELIMIT, -1);  and setting
APR_LDAP_DEFAULT_SIZELIMIT to -1 or 0 accordingly.


On 2/23/07, Brad Nicholes <[EMAIL PROTECTED]> wrote:


What LDAP client SDK does z/OS use? (Novell, OpenLDAP, Netscape, Other???)

Brad

>>> On 2/22/2007 at 12:52 PM, in message
<[EMAIL PROTECTED]>, "David
Jones"
<[EMAIL PROTECTED]> wrote:
> Its the z/OS, has LDAP_NO_SIZELIMIT defined. Does not have nor support
> LDAP_DEFAULT_SIZELIMIT
>
> On 2/22/07, Brad Nicholes <[EMAIL PROTECTED]> wrote:
>>
>> >>> On 2/22/2007 at 7:12 AM, in message
>> <[EMAIL PROTECTED]>, "David
>> Jones"
>> <[EMAIL PROTECTED]> wrote:
>> > How about something alone these lines? It assumes there is nobody
with
>> > LDAP_DEFAULT_LIMIT undefined AND LDAP_NO_LIMIT defined, but still
>> supports
>> > and wishes to use the -1 value.
>> >
>> > --- util_ldap.c.defaultlimitWed Feb 21 16:08:51 2007
>> > +++ util_ldap.c.nolimit Thu Feb 15 12:50:09 2007
>> > @@ -52,15 +52,9 @@
>> >  #define LDAP_CA_TYPE_BASE64 2
>> >  #define LDAP_CA_TYPE_CERT7_DB   3
>> >
>> > -#ifdef LDAP_DEFAULT_LIMIT
>> > -#define LDAP_LIMIT_VALUE LDAP_DEFAULT_LIMIT
>> > -#else
>> > -#ifndef LDAP_NO_LIMIT  /* Have neither LDAP_DEFAULT_LIMIT or
>> LDAP_NO_LIMIT
>> > */
>> > -#define LDAP_LIMIT_VALUE  -1
>> > -#else  /* Have LDAP_NO_LIMIT, but not
>> LDAP_DEFAULT_LIMIT */
>> > -#define LDAP_LIMIT_VALUE LDAP_NO_LIMIT
>> > -#endif /* !LDAP_NO_LIMIT */
>> > -#endif /* LDAP_DEFAULT_LIMIT */
>> > +#ifndef LDAP_NO_LIMIT
>> > +#define LDAP_NO_LIMIT -1
>> > +#endif
>> >
>> >  module AP_MODULE_DECLARE_DATA ldap_module;
>> >
>> > @@ -680,7 +674,7 @@
>> >  /* search for reqdn */
>> >  if ((result = ldap_search_ext_s(ldc->ldap, (char *)reqdn,
>> > LDAP_SCOPE_BASE,
>> >  "(objectclass=*)", NULL, 1,
>> > -NULL, NULL, NULL,
LDAP_LIMIT_VALUE,
>> > &res))
>> > +NULL, NULL, NULL, LDAP_NO_LIMIT,
>> &res))
>> >  == LDAP_SERVER_DOWN)
>> >  {
>> >  ldc->reason = "DN Comparison ldap_search_ext_s() "
>> > @@ -958,7 +952,7 @@
>> >  if ((result = ldap_search_ext_s(ldc->ldap,
>> >  (char *)basedn, scope,
>> >  (char *)filter, attrs, 0,
>> > -NULL, NULL, NULL,
LDAP_LIMIT_VALUE,
>> > &res))
>> > +NULL, NULL, NULL, LDAP_NO_LIMIT,
>> &res))
>> >  == LDAP_SERVER_DOWN)
>> >  {
>> >  ldc->reason = "ldap_search_ext_s() for user failed with
server
>> > down";
>> > @@ -1198,7 +1192,7 @@
>> >  if ((result = ldap_search_ext_s(ldc->ldap,
>> >  (char *)basedn, scope,
>> >

PATCH: incorrect use of an enum in util_ldap.h struct

[David Jones]

In util_ldap.h there is a use of an enum in util_ldap.h (struct
util_ldap_connection_t) with the assumption that it is of length int.
Similar enum in aaa/mod_authnz_ldap.c.
As the size of an enum is indeterminate  this call in util_ldap.c may/may
not get a valid value based on how the compiler handles it:
  ldap_set_option(ldc->ldap, LDAP_OPT_DEREF, &(ldc->deref));

By changing the stuct to use an int type for deref instead of deref_options
we guarantee valid behavior.


Index: httpd-trunk/include/util_ldap.h
===
--- httpd-trunk/include/util_ldap.h(revision 494665)
+++ httpd-trunk/include/util_ldap.h(working copy)
@@ -92,7 +92,7 @@

const char *host;   /* Name of the LDAP server (or
space separated list) */
int port;   /* Port of the LDAP server */
-deref_options deref;/* how to handle alias dereferening
*/
+int deref;  /* how to handle alias dereferening
*/

const char *binddn; /* DN to bind to server (can be
NULL) */
const char *bindpw; /* Password to bind to server (can
be NULL) */
@@ -202,11 +202,11 @@
 *  use this connection while it is busy. Once you are finished with a
connection,
 *  apr_ldap_connection_close() must be called to release this
connection.
 * @deffunc util_ldap_connection_t *util_ldap_connection_find(request_rec
*r, const char *host, int port,
- *   const char
*binddn, const char *bindpw, deref_options deref,
+ *   const char
*binddn, const char *bindpw, int deref,
 *   int
netscapessl, int starttls)
 */
APR_DECLARE_OPTIONAL_FN(util_ldap_connection_t
*,uldap_connection_find,(request_rec *r, const char *host, int port,
-  const char *binddn, const
char *bindpw, deref_options deref,
+  const char *binddn, const
char *bindpw, int deref,
  int secure));

/**
Index: httpd-trunk/modules/ldap/util_ldap.c
===
--- httpd-trunk/modules/ldap/util_ldap.c(revision 510991)
+++ httpd-trunk/modules/ldap/util_ldap.c(working copy)
@@ -443,7 +443,7 @@
uldap_connection_find(request_rec *r,
  const char *host, int port,
  const char *binddn, const char *bindpw,
-  deref_options deref, int secure)
+  int deref, int secure)
{
struct util_ldap_connection_t *l, *p; /* To traverse the linked list */
int secureflag = secure;
Index: httpd-trunk/modules/aaa/mod_authnz_ldap.c
===
--- httpd-trunk/modules/aaa/mod_authnz_ldap.c(revision 494665)
+++ httpd-trunk/modules/aaa/mod_authnz_ldap.c(working copy)
@@ -57,7 +57,7 @@
char **attributes;  /* Array of all the attributes to
return */
int scope;  /* Scope of the search */
char *filter;   /* Filter to further limit the search
*/
-deref_options deref;/* how to handle alias dereferening */
+int deref;  /* how to handle alias dereferening */
char *binddn;   /* DN to bind to server (can be NULL)
*/
char *bindpw;   /* Password to bind to server (can be
NULL) */


deref01.patch
Description: Binary data

Re: util_ldap.c use of hardcoded sizelimit on ldap_search_ext_s causing error

[David Jones]

Its the z/OS, has LDAP_NO_SIZELIMIT defined. Does not have nor support
LDAP_DEFAULT_SIZELIMIT

On 2/22/07, Brad Nicholes <[EMAIL PROTECTED]> wrote:


>>> On 2/22/2007 at 7:12 AM, in message
<[EMAIL PROTECTED]>, "David
Jones"
<[EMAIL PROTECTED]> wrote:
> How about something alone these lines? It assumes there is nobody with
> LDAP_DEFAULT_LIMIT undefined AND LDAP_NO_LIMIT defined, but still
supports
> and wishes to use the -1 value.
>
> --- util_ldap.c.defaultlimitWed Feb 21 16:08:51 2007
> +++ util_ldap.c.nolimit Thu Feb 15 12:50:09 2007
> @@ -52,15 +52,9 @@
>  #define LDAP_CA_TYPE_BASE64 2
>  #define LDAP_CA_TYPE_CERT7_DB   3
>
> -#ifdef LDAP_DEFAULT_LIMIT
> -#define LDAP_LIMIT_VALUE LDAP_DEFAULT_LIMIT
> -#else
> -#ifndef LDAP_NO_LIMIT  /* Have neither LDAP_DEFAULT_LIMIT or
LDAP_NO_LIMIT
> */
> -#define LDAP_LIMIT_VALUE  -1
> -#else  /* Have LDAP_NO_LIMIT, but not
LDAP_DEFAULT_LIMIT */
> -#define LDAP_LIMIT_VALUE LDAP_NO_LIMIT
> -#endif /* !LDAP_NO_LIMIT */
> -#endif /* LDAP_DEFAULT_LIMIT */
> +#ifndef LDAP_NO_LIMIT
> +#define LDAP_NO_LIMIT -1
> +#endif
>
>  module AP_MODULE_DECLARE_DATA ldap_module;
>
> @@ -680,7 +674,7 @@
>  /* search for reqdn */
>  if ((result = ldap_search_ext_s(ldc->ldap, (char *)reqdn,
> LDAP_SCOPE_BASE,
>  "(objectclass=*)", NULL, 1,
> -NULL, NULL, NULL, LDAP_LIMIT_VALUE,
> &res))
> +NULL, NULL, NULL, LDAP_NO_LIMIT,
&res))
>  == LDAP_SERVER_DOWN)
>  {
>  ldc->reason = "DN Comparison ldap_search_ext_s() "
> @@ -958,7 +952,7 @@
>  if ((result = ldap_search_ext_s(ldc->ldap,
>  (char *)basedn, scope,
>  (char *)filter, attrs, 0,
> -NULL, NULL, NULL, LDAP_LIMIT_VALUE,
> &res))
> +NULL, NULL, NULL, LDAP_NO_LIMIT,
&res))
>  == LDAP_SERVER_DOWN)
>  {
>  ldc->reason = "ldap_search_ext_s() for user failed with server
> down";
> @@ -1198,7 +1192,7 @@
>  if ((result = ldap_search_ext_s(ldc->ldap,
>  (char *)basedn, scope,
>  (char *)filter, attrs, 0,
> -NULL, NULL, NULL, LDAP_LIMIT_VALUE,
> &res))
> +NULL, NULL, NULL, LDAP_NO_LIMIT,
&res))
>  == LDAP_SERVER_DOWN)
>  {
>  ldc->reason = "ldap_search_ext_s() for user failed with server
> down";
>

Maybe I missed this before, but what platform or LDAP SDK does this fail
on?  The Novell LDAP SDK obviously supports LDAP_DEFAULT_SIZELIMIT (-1) and
according to the OpenLDAP source code, it also supports the same
functionality if the value of sizelimit is -1 even though it does not
specifically define LDAP_DEFAULT_SIZELIMIT.  I don't know what the Netscape
or Microsoft SDKs support other than the fact that we have been passing
those SDKs the same -1 value without a problem.  I believe that the only
reason why we see the hardcoded -1 rather than a #define is simply because
not all of the SDKs provide a #define yet they all seems to support the
functionality.  We just need to validate that theory.

Brad

Re: util_ldap.c use of hardcoded sizelimit on ldap_search_ext_s causing error

[David Jones]

How about something alone these lines? It assumes there is nobody with
LDAP_DEFAULT_LIMIT undefined AND LDAP_NO_LIMIT defined, but still supports
and wishes to use the -1 value.

--- util_ldap.c.defaultlimitWed Feb 21 16:08:51 2007
+++ util_ldap.c.nolimit Thu Feb 15 12:50:09 2007
@@ -52,15 +52,9 @@
#define LDAP_CA_TYPE_BASE64 2
#define LDAP_CA_TYPE_CERT7_DB   3

-#ifdef LDAP_DEFAULT_LIMIT
-#define LDAP_LIMIT_VALUE LDAP_DEFAULT_LIMIT
-#else
-#ifndef LDAP_NO_LIMIT  /* Have neither LDAP_DEFAULT_LIMIT or LDAP_NO_LIMIT
*/
-#define LDAP_LIMIT_VALUE  -1
-#else  /* Have LDAP_NO_LIMIT, but not LDAP_DEFAULT_LIMIT */
-#define LDAP_LIMIT_VALUE LDAP_NO_LIMIT
-#endif /* !LDAP_NO_LIMIT */
-#endif /* LDAP_DEFAULT_LIMIT */
+#ifndef LDAP_NO_LIMIT
+#define LDAP_NO_LIMIT -1
+#endif

module AP_MODULE_DECLARE_DATA ldap_module;

@@ -680,7 +674,7 @@
/* search for reqdn */
if ((result = ldap_search_ext_s(ldc->ldap, (char *)reqdn,
LDAP_SCOPE_BASE,
"(objectclass=*)", NULL, 1,
-NULL, NULL, NULL, LDAP_LIMIT_VALUE,
&res))
+NULL, NULL, NULL, LDAP_NO_LIMIT, &res))
== LDAP_SERVER_DOWN)
{
ldc->reason = "DN Comparison ldap_search_ext_s() "
@@ -958,7 +952,7 @@
if ((result = ldap_search_ext_s(ldc->ldap,
(char *)basedn, scope,
(char *)filter, attrs, 0,
-NULL, NULL, NULL, LDAP_LIMIT_VALUE,
&res))
+NULL, NULL, NULL, LDAP_NO_LIMIT, &res))
== LDAP_SERVER_DOWN)
{
ldc->reason = "ldap_search_ext_s() for user failed with server
down";
@@ -1198,7 +1192,7 @@
if ((result = ldap_search_ext_s(ldc->ldap,
(char *)basedn, scope,
(char *)filter, attrs, 0,
-NULL, NULL, NULL, LDAP_LIMIT_VALUE,
&res))
+NULL, NULL, NULL, LDAP_NO_LIMIT, &res))
== LDAP_SERVER_DOWN)
{
ldc->reason = "ldap_search_ext_s() for user failed with server
down";

On 2/20/07, Brad Nicholes <[EMAIL PROTECTED]> wrote:


>>> On 2/19/2007 at 9:29 AM, in message
<[EMAIL PROTECTED]>, "Jeff
Trawick"
<[EMAIL PROTECTED]> wrote:
> On 2/15/07, David Jones <[EMAIL PROTECTED]> wrote:
>> Currently util_ldap.c has a hard coded -1 as the search limit value
(meaning
>> infinite/no limit) on ldap_search_ext_s() calls.  Some platforms cannot
>> handle the -1, but need a 0.  Linux, zoS (and others) have a
LDAP_NO_LIMIT
>> value in ldap.h.
>>  Below is a patch, allows those who have LDAP_NO_LIMIT value to take
>> advantage of it, and others to continue using a -1 value.
>
> patch committed to trunk and proposed for backport 2.2.x
> my guess is that -1 is rarely/never the proper value, but that isn't
> so easy to confirm; hopefully the symbol is always available in modern
> SDK level

The values of 0 and -1 have a different meaning at least in the Novell
LDAP SDK.  A value of 0 or LDAP_NO_LIMIT specifies that the search truely
has no limit to the number of entries that will be returned.  A value of -1
or LDAP_DEFAULT_SIZELIMIT specifies that the search should default to the
session value or the value that was set in the session by
LDAP_OPT_SIZELIMIT.  Changing the sizelimit parameter from -1 to
LDAP_NO_LIMIT in the calls to ldap_search_ext_s() removes the ability to
control the size limit through the session options.  In fact the patch that
was submitted will cause the ldap_search_ext_s() function to act differently
depending on whether the LDAP SDK has defined LDAP_NO_LIMIT or not.

I can't confirm this because I haven't been able to find it documented for
all SDKs but I would assume that the initial reason for specifying -1 rather
than LDAP_NO_LIMIT or LDAP_DEFAULT_SIZELIMIT is because the intention was to
make the call to ldap_search_ext_s() defer to the size limit specified in
the session.  But not all SDKs define LDAP_DEFAULT_SIZELIMIT, therefore -1
was hardcoded.  Can those that know the OpenLDAP or Microsoft LDAP SDKs
confirm that those SDKs support a -1 or LDAP_DEFAULT_SIZELIMIT?

In the meantime, the patch should probably be revised to make sure that
all platforms work the same rather than some supporting LDAP_NO_LIMIT and
other supporting LDAP_DEFAULT_SIZELIMIT.  The preference should be
LDAP_DEFAULT_SIZELIMIT (-1).

Brad

Remove dead defines in util_ldap.c

[David Jones]

Several defines in util_ldap for certificate file types are no longer used
and can be deleted, They were moved to apr_ldap_option.h with the APR_
prefix added

Here's a subset of the defines now in apr_ldap_option.h:
/** CA certificate type unknown */
#define APR_LDAP_CA_TYPE_UNKNOWN0
/** binary DER encoded CA certificate */
#define APR_LDAP_CA_TYPE_DER1




PATCH:
Index: util_ldap.c
===
--- util_ldap.c (revision 494665)
+++ util_ldap.c (working copy)
@@ -45,14 +45,6 @@
#include "unixd.h"
#endif

-/* defines for certificate file types
-*/
-#define LDAP_CA_TYPE_UNKNOWN0
-#define LDAP_CA_TYPE_DER1
-#define LDAP_CA_TYPE_BASE64 2
-#define LDAP_CA_TYPE_CERT7_DB   3
-
-
module AP_MODULE_DECLARE_DATA ldap_module;

#define LDAP_CACHE_LOCK() do {      \



--
David Jones
[EMAIL PROTECTED]

util_ldap.c use of hardcoded sizelimit on ldap_search_ext_s causing error

[David Jones]

Currently util_ldap.c has a hard coded -1 as the search limit value (meaning
infinite/no limit) on ldap_search_ext_s() calls.  Some platforms cannot
handle the -1, but need a 0.  Linux, zoS (and others) have a LDAP_NO_LIMIT
value in ldap.h.

Below is a patch, allows those who have LDAP_NO_LIMIT value to take
advantage of it, and others to continue using a -1 value.


Index: util_ldap.c
===
--- util_ldap.c (revision 494665)
+++ util_ldap.c (working copy)
@@ -52,6 +52,9 @@
#define LDAP_CA_TYPE_BASE64 2
#define LDAP_CA_TYPE_CERT7_DB   3

+#ifndef LDAP_NO_LIMIT
+#define LDAP_NO_LIMIT -1
+#endif

module AP_MODULE_DECLARE_DATA ldap_module;

@@ -657,7 +660,7 @@
/* search for reqdn */
if ((result = ldap_search_ext_s(ldc->ldap, (char *)reqdn,
LDAP_SCOPE_BASE,
"(objectclass=*)", NULL, 1,
-NULL, NULL, NULL, -1, &res))
+NULL, NULL, NULL, LDAP_NO_LIMIT, &res))
== LDAP_SERVER_DOWN)
{
ldc->reason = "DN Comparison ldap_search_ext_s() "
@@ -935,7 +938,7 @@
if ((result = ldap_search_ext_s(ldc->ldap,
(char *)basedn, scope,
(char *)filter, attrs, 0,
-NULL, NULL, NULL, -1, &res))
+NULL, NULL, NULL, LDAP_NO_LIMIT, &res))
== LDAP_SERVER_DOWN)
{
ldc->reason = "ldap_search_ext_s() for user failed with server
down";
@@ -1175,7 +1178,7 @@
if ((result = ldap_search_ext_s(ldc->ldap,
(char *)basedn, scope,
(char *)filter, attrs, 0,
-NULL, NULL, NULL, -1, &res))
+NULL, NULL, NULL, LDAP_NO_LIMIT, &res))
== LDAP_SERVER_DOWN)
{
ldc->reason = "ldap_search_ext_s() for user failed with server
down";



--
David Jones
[EMAIL PROTECTED]

Re: link failure from buildmark

[David Jones]

Sorry, not sure what happened, here is an unmangled diff:


svn diff Makefile.in <http://makefile.in/>

Index: Makefile.in <http://makefile.in/>
===
--- Makefile.in <http://makefile.in/> (revision 494665)
+++ Makefile.in <http://makefile.in/> (working copy)
@@ -4,8 +4,8 @@

PROGRAM_NAME = $(progname)
PROGRAM_SOURCES  = modules.c
-PROGRAM_LDADD= buildmark.o $(HTTPD_LDFLAGS) $(PROGRAM_DEPENDENCIES)
$(EXTRA_LIBS) $(AP_LIBS) $(LIBS)
-PROGRAM_PRELINK  = $(COMPILE) -c $(top_srcdir)/server/buildmark.c
+PROGRAM_LDADD= buildmark.lo $(HTTPD_LDFLAGS)
$(PROGRAM_DEPENDENCIES) $(EXTRA_LIBS) $(AP_LIBS) $(LIBS)
+PROGRAM_PRELINK  = $(LIBTOOL) --mode=compile $(COMPILE)  -c
$(top_srcdir)/server/buildmark.c
PROGRAM_DEPENDENCIES = \
  server/libmain.la \
  $(BUILTIN_LIBS) \

On 1/26/07, Greg Ames <[EMAIL PROTECTED]> wrote:


--- David Jones <[EMAIL PROTECTED]> wrote:

> zOS needs to compile with extra CFLAGS in order to link correctly.
> After revisions 153273/153266 to ./Makefile.in all compile and link
flags
> are lost as
> buildmark.c is made without them:

concept sounds fine but...

> --- Makefile.in.orig Wed Jan 17 14:00:11 2007
> +++ Makefile.in Thu Jan 18 09:17:17 2007
> @@ -4,8 +4,8 @@
>
> PROGRAM_NAME = $(progname)
> PROGRAM_SOURCES = modules.c
> -PROGRAM_LDADD = buildmark.o $(HTTPD_LDFLAGS) $(PROGRAM_DEPENDENCIES)
$()
> -PROGRAM_PRELINK = $(COMPILE) $(top_srcdir)/server/buildmark.c
> +PROGRAM_LDADD = buildmark.lo $(HTTPD_LDFLAGS) $(PROGRAM_DEPENDENCIES)
$)
 ^
what happened here?   >>-+

> +PROGRAM_PRELINK = $(LIBTOOL) --mode=compile $(COMPILE) -c
$(top_srcdir)/c

do we have a file named "c" in $(top_srcdir) ?  how does this do the job
with
buildmark?

maybe cut-n-pasto's or funky email clients explain both?

Greg





Yahoo! Music Unlimited
Access over 1 million songs.
http://music.yahoo.com/unlimited





Expecting? Get great news right away with email Auto-Check.
Try the Yahoo! Mail Beta.
http://advision.webevents.yahoo.com/mailbeta/newmail_tools.html

link failure from buildmark

[David Jones]

zOS needs to compile with extra CFLAGS in order to link correctly.
After revisions 153273/153266 to ./Makefile.in all compile and link flags
are lost as
buildmark.c is made without them: PROGRAM_PRELINK = $(COMPILE)
$(top_srcdir)/server/buildmark.c


zOS LIBTOOL does add the needed flags, so a solution for zOS is below. Seems
to work on linux, too.
Any comments for other/better/generic approaches?


--- Makefile.in.orig Wed Jan 17 14:00:11 2007
+++ Makefile.in Thu Jan 18 09:17:17 2007
@@ -4,8 +4,8 @@

PROGRAM_NAME = $(progname)
PROGRAM_SOURCES = modules.c
-PROGRAM_LDADD = buildmark.o $(HTTPD_LDFLAGS) $(PROGRAM_DEPENDENCIES) $()
-PROGRAM_PRELINK = $(COMPILE) $(top_srcdir)/server/buildmark.c
+PROGRAM_LDADD = buildmark.lo $(HTTPD_LDFLAGS) $(PROGRAM_DEPENDENCIES) $)
+PROGRAM_PRELINK = $(LIBTOOL) --mode=compile $(COMPILE) -c $(top_srcdir)/c
PROGRAM_DEPENDENCIES = \
server/libmain.la \
$(BUILTIN_LIBS) \

Note: Can't use $(LT_COMPILE) directly instead of $(LIBTOOL) as
rules.mkuses parms '-c $< && touch $@'
which get in the way.

--
David Jones
[EMAIL PROTECTED] <[EMAIL PROTECTED]>

Clarification on mod_cache request with max-age expired, 200 or 304?

[David Jones]

What is the proper status for a mod_cache request with both If-Modified-Since andCache-Control: max-age=n, when max-age indicates expiration?
Ex. Send a request with large max-age, receive an expected 304.Wait 5 secondsSend another request with If-Modified-Since and max-age=1 (delta seconds > max-age)Still receive a 304.
I was expecting a 200 since max-age < delta, what is supposed to happen?
Ran this at 2.2.3 with mod_mem_cache.
error_log:
[Fri Sep 22 10:32:34 2006] [debug] mod_cache.c(129): Adding CACHE_SAVE filter for /cache1.html[Fri Sep 22 10:32:34 2006] [debug] mod_cache.c(136): Adding CACHE_REMOVE_URL filter for /cache1.html[Fri Sep 22 10:32:34 2006] [debug] mod_cache.c(502): cache: /cache1.html not cached. Reason: HTTP Status 304 Not Modified
[Fri Sep 22 10:32:41 2006] [debug] cache_storage.c(272): Cached response for /cache1.html isn't fresh.  Adding/replacing conditional request headers.[Fri Sep 22 10:32:41 2006] [debug] mod_cache.c(129): Adding CACHE_SAVE filter for /cache1.html
[Fri Sep 22 10:32:41 2006] [debug] mod_cache.c(136): Adding CACHE_REMOVE_URL filter for /cache1.html[Fri Sep 22 10:32:41 2006] [debug] mod_cache.c(602): cache: Caching url: /cache1.html[Fri Sep 22 10:32:41 2006] [debug] mod_cache.c(608): cache: Removing CACHE_REMOVE_URL filter.
[Fri Sep 22 10:32:41 2006] [debug] mod_cache.c(651): cache: Added date header
Traces (this is from a 2.0.47 run so time stamps wont match above, but i get same results)
1st request/response==GET /cache1.html HTTP/1.1Cache-Control: max-age=1001If-Modified-Since: Thu, 21 Sep 2006 18:51:46 GMTUser-Agent: Jakarta Commons-HttpClient/3.0-rc2
Host: 127.0.0.1
HTTP/1.1 304 Not ModifiedDate: Thu, 21 Sep 2006 18:51:50 GMTETag: "9652c-13-41dfb3724d480"
2nd request/response==GET /cache1.html HTTP/1.1Cache-Control: max-age=1If-Modified-Since: Thu, 21 Sep 2006 18:51:46 GMTUser-Agent: Jakarta Commons-HttpClient/3.0-rc2
Host: 127.0.0.1
HTTP/1.1 304 Not ModifiedDate: Thu, 21 Sep 2006 18:51:55 GMTETag: "9652c-13-41dfb3724d480"
 
 
David Jones