Re: APR::Base64 Bug

[prev] [thread] [next] [Date index for 2005/05/23]

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@xxxxxx.xxx http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com