Re: [PATCH] Eliminate 13 in modules/aaa/mod_authn_dbd.c / modules/aaa/mod_authnz_ldap.c

2007-09-02 Thread William A. Rowe, Jr.
Graham Leggett wrote:
 Martin Kraemer wrote:
 
 Here's a patch to eliminate the 13, and to improve portability to
 EBCDIC machines by using apr_toupper().
 
 Some of this fooness here revolves around charset issues, something I am
 not clear on for many platforms.
 
 The first question is, what is the charset of the names of environment
 variables on various platforms, the next is the charset of the names of
 LDAP attributes and database columns.


It doesn't matter; we should only perform c-locale case folding.  As soon
as you get beyond that, the same unicode characters fold differently in
different languages.

Bill


Re: [PATCH] Eliminate 13 in modules/aaa/mod_authn_dbd.c / modules/aaa/mod_authnz_ldap.c

2007-09-01 Thread Graham Leggett

Martin Kraemer wrote:


Here's a patch to eliminate the 13, and to improve portability to
EBCDIC machines by using apr_toupper().


Some of this fooness here revolves around charset issues, something I am 
not clear on for many platforms.


The first question is, what is the charset of the names of environment 
variables on various platforms, the next is the charset of the names of 
LDAP attributes and database columns.


The apr_toupper() macro uses toupper() in the underlying implementation, 
which implies a char - char mapping. According to the man pages on my 
Mac, towupper() gives a wide character mapping int - int, but APR 
doesn't support this yet.


Regards,
Graham
--


smime.p7s
Description: S/MIME Cryptographic Signature


[PATCH] Eliminate 13 in modules/aaa/mod_authn_dbd.c / modules/aaa/mod_authnz_ldap.c

2007-08-31 Thread Martin Kraemer
From the 2.2.x STATUS doc:
   * mod_authn_dbd: Export any additional columns queried in the SQL select
 into the environment with the name AUTHENTICATE_COLUMN. This brings
 mod_authn_dbd behaviour in line with mod_authnz_ldap.
 Trunk: http://svn.apache.org/viewvc?view=revrevision=466865
 +1: minfrin
 niq: This wants a little tidying.  Use of 13 as a magic number (for a
  strlen of something defined elsewhere) is nasty.  More importantly
  you need to document how it affects the module's directives.
  I'll +1 it when that's done.
 wrowe: ditto's - good concept.

Here's a patch to eliminate the 13, and to improve portability to
EBCDIC machines by using apr_toupper().

  Martin
-- 
[EMAIL PROTECTED]| Fujitsu Siemens
http://www.fujitsu-siemens.com/imprint.html | 81730  Munich,  Germany
Index: modules/aaa/mod_authn_dbd.c
===
--- modules/aaa/mod_authn_dbd.c (Revision 571317)
+++ modules/aaa/mod_authn_dbd.c (Arbeitskopie)
@@ -138,7 +138,7 @@
 char *str = apr_pstrcat(r-pool, AUTHN_PREFIX,
 name,
 NULL);
-int j = 13;
+int j = sizeof(AUTHN_PREFIX)-1; /* string length of 
AUTHENTICATE_, excluding the trailing NIL */
 while (str[j]) {
 if (!apr_isalnum(str[j])) {
 str[j] = '_';
@@ -222,7 +222,7 @@
 char *str = apr_pstrcat(r-pool, AUTHN_PREFIX,
 name,
 NULL);
-int j = 13;
+int j = sizeof(AUTHN_PREFIX)-1; /* string length of 
AUTHENTICATE_, excluding the trailing NIL */
 while (str[j]) {
 if (!apr_isalnum(str[j])) {
 str[j] = '_';
Index: modules/aaa/mod_authnz_ldap.c
===
--- modules/aaa/mod_authnz_ldap.c   (Revision 571317)
+++ modules/aaa/mod_authnz_ldap.c   (Arbeitskopie)
@@ -442,11 +442,9 @@
 int i = 0;
 while (sec-attributes[i]) {
 char *str = apr_pstrcat(r-pool, AUTHN_PREFIX, sec-attributes[i], 
NULL);
-int j = 13;
+int j = sizeof(AUTHN_PREFIX)-1; /* string length of 
AUTHENTICATE_, excluding the trailing NIL */
 while (str[j]) {
-if (str[j] = 'a'  str[j] = 'z') {
-str[j] = str[j] - ('a' - 'A');
-}
+str[j] = apr_toupper(str[j]);
 j++;
 }
 apr_table_setn(e, str, vals[i]);


pgpYtb4EVUr2M.pgp
Description: PGP signature


Re: [PATCH] Eliminate 13 in modules/aaa/mod_authn_dbd.c / modules/aaa/mod_authnz_ldap.c

2007-08-31 Thread Graham Leggett
On Fri, August 31, 2007 12:41 pm, Martin Kraemer wrote:

 From the 2.2.x STATUS doc:
* mod_authn_dbd: Export any additional columns queried in the SQL
 select
  into the environment with the name AUTHENTICATE_COLUMN. This brings
  mod_authn_dbd behaviour in line with mod_authnz_ldap.
  Trunk: http://svn.apache.org/viewvc?view=revrevision=466865
  +1: minfrin
  niq: This wants a little tidying.  Use of 13 as a magic number (for a
   strlen of something defined elsewhere) is nasty.  More
 importantly
   you need to document how it affects the module's directives.
   I'll +1 it when that's done.
  wrowe: ditto's - good concept.

 Here's a patch to eliminate the 13, and to improve portability to
 EBCDIC machines by using apr_toupper().

Thanks for this - the fooness really needed to be sorted out before it was
rolled out over the other authn mechanisms.

We have used the exposed extra columns to support autologin to bugzilla,
amongst other things. The next plan is to update the docs to better
explain how these work.

Regards,
Graham
--




Re: [PATCH] Eliminate 13 in modules/aaa/mod_authn_dbd.c / modules/aaa/mod_authnz_ldap.c

2007-08-31 Thread Martin Kraemer
On Fri, Aug 31, 2007 at 12:54:44PM +0200, Graham Leggett wrote:
  Here's a patch to eliminate the 13, and to improve portability to
  EBCDIC machines by using apr_toupper().
 
 Thanks for this - the fooness really needed to be sorted out before it was
 rolled out over the other authn mechanisms.

Should I commit, or do you?

 We have used the exposed extra columns to support autologin to bugzilla,
 amongst other things. The next plan is to update the docs to better
 explain how these work.
+1!

  Martin
-- 
[EMAIL PROTECTED]| Fujitsu Siemens
http://www.fujitsu-siemens.com/imprint.html | 81730  Munich,  Germany


Re: [PATCH] Eliminate 13 in modules/aaa/mod_authn_dbd.c / modules/aaa/mod_authnz_ldap.c

2007-08-31 Thread Martin Kraemer
On Fri, Aug 31, 2007 at 05:09:34PM +0200, Martin Kraemer wrote:
 
 Should I commit, or do you?

Forgot to mention that I meant: commit to trunk.
For 2.2.x, I'd prefer you do it.

   Martin
-- 
[EMAIL PROTECTED]| Fujitsu Siemens
http://www.fujitsu-siemens.com/imprint.html | 81730  Munich,  Germany