You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Torsten Foertsch <to...@gmx.net> on 2005/05/22 19:18:58 UTC

APR::Base64 Bug

Hi,

there is something wrong with APR::Base64.

r2@opi:~> perl -MAPR::Base64 -MData::Dumper -e '$x=APR::Base64::encode( "x" ); print "$x\n".Dumper( [split "", $x] ), "length=".length($x)."\n";'
eA==
$VAR1 = [
          'e',
          'A',
          '=',
          '=',
          ''
        ];
length=5
r2@opi:~> perl -MAPR::Base64 -MData::Dumper -e '$x=APR::Base64::encode( "xxx" ); print "$x\n".Dumper( [split "", $x] ), "length=".length($x)."\n";'
eHh4
$VAR1 = [
          'e',
          'H',
          'h',
          '4',
          ''
        ];
length=5

In both cases length should be 4 and split("",$x) should return (qw(e a = =)).

Torsten

Re: APR::Base64 Bug

Posted by Torsten Foertsch <to...@gmx.net>.
On Sunday 22 May 2005 19:18, Torsten Foertsch wrote:
> there is something wrong with APR::Base64.
>
> r2@opi:~> perl -MAPR::Base64 -MData::Dumper -e '$x=APR::Base64::encode( "x"
> ); print "$x\n".Dumper( [split "", $x] ), "length=".length($x)."\n";' eA==
> $VAR1 = [
>           'e',
>           'A',
>           '=',
>           '=',
>           ''
>         ];
> length=5
> r2@opi:~> perl -MAPR::Base64 -MData::Dumper -e '$x=APR::Base64::encode(
> "xxx" ); print "$x\n".Dumper( [split "", $x] ), "length=".length($x)."\n";'
> eHh4
> $VAR1 = [
>           'e',
>           'H',
>           'h',
>           '4',
>           ''
>         ];
> length=5
>
> In both cases length should be 4 and split("",$x) should return (qw(e a =
> =)).

the attached t/lib/TestAPRlib/base64.pm fails on the bug:

t/apr-ext/base64....1..3
# Running under perl version 5.008005 for linux
# Current time local: Sun May 22 19:24:51 2005
# Current time GMT:   Sun May 22 17:24:51 2005
# Using Test.pm version 1.25
# Using Apache/Test.pm version 1.25
# encoded string: MTIzNDVxd2VydCFAIyQl
# testing : encode
# expected: MTIzNDVxd2VydCFAIyQl
# received: MTIzNDVxd2VydCFAIyQl
not ok 1
# Failed test 1 
in /usr/src/packages/BUILD/mod_perl-2.0.0/t/lib/TestAPRlib/base64.pm at line 
23
# testing : encoded length
# expected: 21
# received: 21
ok 2
# testing : decode
# expected: 12345qwert!@#$%
# received: 12345qwert!@#$%
ok 3
FAILED test 1

Re: APR::Base64 Bug

Posted by Stas Bekman <st...@stason.org>.
Torsten Foertsch wrote:
> On Monday 23 May 2005 22:57, Stas Bekman wrote:
> 
>>if it works for you, i'll commit it.
> 
> 
> The attachment contains a complete patch including the test.

Thanks, Torsten, committed.

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

Re: APR::Base64 Bug

Posted by Torsten Foertsch <to...@gmx.net>.
On Monday 23 May 2005 22:57, Stas Bekman wrote:
> if it works for you, i'll commit it.

The attachment contains a complete patch including the test.

All tests successful, 1 test skipped.
Files=229, Tests=2475, 81 wallclock secs (62.48 cusr +  6.47 csys = 68.95 CPU)

Torsten

Re: APR::Base64 Bug

Posted by Stas Bekman <st...@stason.org>.
Torsten Foertsch wrote:
> On Sunday 22 May 2005 19:18, Torsten Foertsch wrote:
> 
>>Hi,
>>
>>there is something wrong with APR::Base64.
>>
>>r2@opi:~> perl -MAPR::Base64 -MData::Dumper -e '$x=APR::Base64::encode( "x"
>>); print "$x\n".Dumper( [split "", $x] ), "length=".length($x)."\n";' eA==
>>$VAR1 = [
>>          'e',
>>          'A',
>>          '=',
>>          '=',
>>          ''
>>        ];
>>length=5
>>r2@opi:~> perl -MAPR::Base64 -MData::Dumper -e '$x=APR::Base64::encode(
>>"xxx" ); print "$x\n".Dumper( [split "", $x] ), "length=".length($x)."\n";'
>>eHh4
>>$VAR1 = [
>>          'e',
>>          'H',
>>          'h',
>>          '4',
>>          ''
>>        ];
>>length=5
>>
>>In both cases length should be 4 and split("",$x) should return (qw(e a =
>>=)).
> 
> 
> The problem is apr_base64_encode_len() returns the length including the 
> trailing \0 required to hold the encoded string.
> 
> This patch cures the problem:
> 
> ========================================================================
> --- xs/APR/Base64/APR__Base64.h 2005-04-26 20:58:34.000000000 +0200
> +++ xs/APR/Base64/APR__Base64.h 2005-05-23 22:05:53.416525123 +0200
> @@ -18,9 +18,9 @@
>      STRLEN len;
>      int encoded_len;
>      char *data = SvPV(arg, len);
> -    mpxs_sv_grow(sv, apr_base64_encode_len(len));
> +    mpxs_sv_grow(sv, apr_base64_encode_len(len)-1);
>      encoded_len = apr_base64_encode_binary(SvPVX(sv), data, len);
> -    mpxs_sv_cur_set(sv, encoded_len);
> +    mpxs_sv_cur_set(sv, encoded_len-1);
>  }
> 
>  static MP_INLINE void mpxs_apr_base64_decode(pTHX_ SV *sv, SV *arg)
> ========================================================================
> 
> but now the APR::Base64::encode_len test fails because it calls simply 
> apr_base64_encode_len().
> 
> To cure that a new function needs to be introduced in 
> xs/APR/Base64/APR__Base64.h ...
> 
> I'd do that but I don't know what to call instead of mpxs_sv_cur_set() if I 
> want to set an IV.

No need to set IV. Here is the patch that does that and incorporates yours:

Index: xs/maps/apr_functions.map
===================================================================
--- xs/maps/apr_functions.map	(revision 171124)
+++ xs/maps/apr_functions.map	(working copy)
@@ -579,7 +579,7 @@
  MODULE=APR::Base64
   apr_base64_decode | MPXS_ | coded_src
   apr_base64_encode | MPXS_ | plain_src
- apr_base64_encode_len
+ int:DEFINE_encode_len | | int:len
  -apr_base64_decode_len
  -apr_base64_encode_binary
  -apr_base64_decode_binary
Index: xs/APR/Base64/APR__Base64.h
===================================================================
--- xs/APR/Base64/APR__Base64.h	(revision 171124)
+++ xs/APR/Base64/APR__Base64.h	(working copy)
@@ -13,14 +13,18 @@
   * limitations under the License.
   */

+/* apr_base64_encode_len and apr_base64_encode_binary give length that
+ * includes the terminating '\0' */
+#define mpxs_APR__Base64_encode_len(len) apr_base64_encode_len(len) - 1;
+
  static MP_INLINE void mpxs_apr_base64_encode(pTHX_ SV *sv, SV *arg)
  {
      STRLEN len;
      int encoded_len;
      char *data = SvPV(arg, len);
-    mpxs_sv_grow(sv, apr_base64_encode_len(len));
+    mpxs_sv_grow(sv, apr_base64_encode_len(len) - 1);
      encoded_len = apr_base64_encode_binary(SvPVX(sv), data, len);
-    mpxs_sv_cur_set(sv, encoded_len);
+    mpxs_sv_cur_set(sv, encoded_len - 1);
  }

  static MP_INLINE void mpxs_apr_base64_decode(pTHX_ SV *sv, SV *arg)

if it works for you, i'll commit it.

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

Re: APR::Base64 Bug

Posted by Torsten Foertsch <to...@gmx.net>.
On Sunday 22 May 2005 19:18, Torsten Foertsch wrote:
> Hi,
>
> there is something wrong with APR::Base64.
>
> r2@opi:~> perl -MAPR::Base64 -MData::Dumper -e '$x=APR::Base64::encode( "x"
> ); print "$x\n".Dumper( [split "", $x] ), "length=".length($x)."\n";' eA==
> $VAR1 = [
>           'e',
>           'A',
>           '=',
>           '=',
>           ''
>         ];
> length=5
> r2@opi:~> perl -MAPR::Base64 -MData::Dumper -e '$x=APR::Base64::encode(
> "xxx" ); print "$x\n".Dumper( [split "", $x] ), "length=".length($x)."\n";'
> eHh4
> $VAR1 = [
>           'e',
>           'H',
>           'h',
>           '4',
>           ''
>         ];
> length=5
>
> In both cases length should be 4 and split("",$x) should return (qw(e a =
> =)).

The problem is apr_base64_encode_len() returns the length including the 
trailing \0 required to hold the encoded string.

This patch cures the problem:

========================================================================
--- xs/APR/Base64/APR__Base64.h 2005-04-26 20:58:34.000000000 +0200
+++ xs/APR/Base64/APR__Base64.h 2005-05-23 22:05:53.416525123 +0200
@@ -18,9 +18,9 @@
     STRLEN len;
     int encoded_len;
     char *data = SvPV(arg, len);
-    mpxs_sv_grow(sv, apr_base64_encode_len(len));
+    mpxs_sv_grow(sv, apr_base64_encode_len(len)-1);
     encoded_len = apr_base64_encode_binary(SvPVX(sv), data, len);
-    mpxs_sv_cur_set(sv, encoded_len);
+    mpxs_sv_cur_set(sv, encoded_len-1);
 }

 static MP_INLINE void mpxs_apr_base64_decode(pTHX_ SV *sv, SV *arg)
========================================================================

but now the APR::Base64::encode_len test fails because it calls simply 
apr_base64_encode_len().

To cure that a new function needs to be introduced in 
xs/APR/Base64/APR__Base64.h ...

I'd do that but I don't know what to call instead of mpxs_sv_cur_set() if I 
want to set an IV.

Torsten