You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2002/01/25 09:16:30 UTC

Apache::Util porting issues

I've started porting Apache::Util, here is a summary of issues:

Now we have ModPerl::Util as well, so is this the right mapping:

ModPerl::Util:
--------------
- untaint
- exit
- size_string (has nothing to do with Apache
                or should it stay in Apache::Util?)

Apache::Util:
-------------
- escape_uri
- unescape_uri
- unescape_uri_info
- escape_html
- parsedate
- ht_time           (=> ap_ht_time)
- validate_password (=> apr_password_validate)

-----
if we move some functions from Apache::Util as in 1.x to
ModPerl::Util, we need to fix these up in compat.pm, right?

-----

Apache::Util has @EXPORT_OK for its all functions, should we keep it that
way? If so should I create xs/Apache/Util/Util_pm with:

use Exporter ();
@EXPORT_OK = qw(escape_html escape_uri unescape_uri unescape_uri_info
		parsedate ht_time size_string validate_password);
%EXPORT_TAGS = (all => \@EXPORT_OK);

-----

ht_time()'s API is not compatible with Apache::Util::ht_time with 1.x,
we cannot provide a fast wrapper since it now requires a pool object.
so should the code from 1.x be ported without going into APR land (no
pool required)?

or should ht_time be just like in APR and compat.pm to have the old
function in pure perl?

----

parsedate(). Should we use

   APU_DECLARE(apr_time_t) apr_date_parse_http(const char *date);

but it seem to return microsecs, so need to adjust this.

should it move into APR::Date now, with compat.pm adjusting for this
change. Or should we just drop the parsedate() name (and still have it
in compat.pm) and start using date_parse_http()?

also we have a new apr_date_parse_rfc in apr_functions.map, if we
expose it (we already do) it makes sense that date_parse_http will be
there too.

----

Is this a correct mapping?

- escape_uri          => ap_os_escape_path (needs pool!)
          shouldn't it be    ap_escape_url?

- unescape_uri        => ap_unescape_url

- unescape_uri_info   => port XS from Apache.xs in 1.x

should they still be called _uri or renamed to _url per ap_ functions?
in todo/api.txt, you say:

   Apache->unescape_url{_info}:
   not yet implemented.  should be moved to Apache::Util

you call these _url, not _uri.

also should we introduce these: ap_escape_path_segment, ap_escape_url

----

- escape_html         => ap_escape_html (needs pool!)

First of all what are we going to do about the issues raised by Robin
Berjon in the recent thread? I'm talking about UTF encodings not being
escaped.

Second, what to do with the functions requiring a pool object? todo
says:

   also need to default to current pool (pconf at startup, r->pool at
   request time)

but the 1.x API, doesn't have $r in the API. Can we have some global
variable that always points to the current pool? in the order r->pool,
s->pool, or pconf? This will solve many API adjustments problems. Or
s->is it a bad idea?

Third, todo/api.txt says:

   enhancements: consider jeff baker's more robust implementation of
   my_escape_html(), which should probably be made in apache-2.0 itself

where is it? is it utf compliant?

_____________________________________________________________________
Stas Bekman             JAm_pH      --   Just Another mod_perl Hacker
http://stason.org/      mod_perl Guide   http://perl.apache.org/guide
mailto:stas@stason.org  http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Apache::Util::size_string issues

Posted by Stas Bekman <st...@stason.org>.
Doug MacEachern wrote:

> On Tue, 29 Jan 2002, Stas Bekman wrote:
>  
> 
>>Since we have a potentially growing number of Util functions, may be 
>>it's better not to throw all the functions into one namespace? So in 
>>this case we could make something like Apache::StringUtil or 
>>Apache::UtilString?
>>
>>Because if for example you look at APR::Table, it's a util too.
>>
> 
> APR::Table all live in the apr_table_ namespace.  apr_strfsize doesn't
> really have a C namespace beyond apr_, the closest would be APR::String.
> probably should be apr_string_format_size(), but i'm tired of renaming apr
> functions.


;(


>>In any case I'll go with Apache::Util::format_size and later we can 
>>change it. Sounds OK?
>>
> 
> APR::


of course :)


> it should be APR::Util::strfsize or APR::String::format_size.

OK, I'll go with APR::String::format_size

Thanks Doug!

_____________________________________________________________________
Stas Bekman             JAm_pH      --   Just Another mod_perl Hacker
http://stason.org/      mod_perl Guide   http://perl.apache.org/guide
mailto:stas@stason.org  http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Apache::Util::size_string issues

Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 29 Jan 2002, Stas Bekman wrote:
 
> Since we have a potentially growing number of Util functions, may be 
> it's better not to throw all the functions into one namespace? So in 
> this case we could make something like Apache::StringUtil or 
> Apache::UtilString?
> 
> Because if for example you look at APR::Table, it's a util too.

APR::Table all live in the apr_table_ namespace.  apr_strfsize doesn't
really have a C namespace beyond apr_, the closest would be APR::String.
probably should be apr_string_format_size(), but i'm tired of renaming apr
functions.

> In any case I'll go with Apache::Util::format_size and later we can 
> change it. Sounds OK?

APR::

it should be APR::Util::strfsize or APR::String::format_size.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Apache::Util::size_string issues

Posted by Stas Bekman <st...@stason.org>.
Doug MacEachern wrote:

> On Tue, 29 Jan 2002, Stas Bekman wrote:
> 
>>like this?
>>
> 
> yeah, but i'm not sure it's worth it for such a small string.  in fact, i
> think it is a loss, see comment in sv.c:
> 	/*
> 	 * Check to see if we can just swipe the string.  If so, it's a
> 	 * possible small lose on short strings, but a big win on long ones.
> 	 * It might even be a win on short strings if SvPVX(dstr)
> 	 * has to be allocated and SvPVX(sstr) has to be freed.
> 	 */
> 
> probably best to stick with your original.


ok



>>>>Also apr_strfsize is in the APR::Strings module in the map file. Should 
>>>>we have APR::Strings::strfsize instead of Apache__Util_size_string.
>>>>
> 
> that is probably the right place.  but Apache::String::format_size has a
> better ring to it.  either that or just group it with APR::Util (as
> APR::Util::strfsize), where the current APR::Lib functions are supposed to
> move to.

Since we have a potentially growing number of Util functions, may be 
it's better not to throw all the functions into one namespace? So in 
this case we could make something like Apache::StringUtil or 
Apache::UtilString?

Because if for example you look at APR::Table, it's a util too.

In any case I'll go with Apache::Util::format_size and later we can 
change it. Sounds OK?

Apache::Util::size_string as a wrapper.

_____________________________________________________________________
Stas Bekman             JAm_pH      --   Just Another mod_perl Hacker
http://stason.org/      mod_perl Guide   http://perl.apache.org/guide
mailto:stas@stason.org  http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Apache::Util::size_string issues

Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 29 Jan 2002, Stas Bekman wrote:
> like this?

yeah, but i'm not sure it's worth it for such a small string.  in fact, i
think it is a loss, see comment in sv.c:
	/*
	 * Check to see if we can just swipe the string.  If so, it's a
	 * possible small lose on short strings, but a big win on long ones.
	 * It might even be a win on short strings if SvPVX(dstr)
	 * has to be allocated and SvPVX(sstr) has to be freed.
	 */

probably best to stick with your original.
 
> you mean, apr_strfsize, not apr_strftime

yup.
 
> OK, will ask.

might be worth checking what mod_autoindex outputs for a file of size
greater than what apr_off_t can hold too.
 
> what about this:
> 
> >>Also apr_strfsize is in the APR::Strings module in the map file. Should 
> >>we have APR::Strings::strfsize instead of Apache__Util_size_string.

that is probably the right place.  but Apache::String::format_size has a
better ring to it.  either that or just group it with APR::Util (as
APR::Util::strfsize), where the current APR::Lib functions are supposed to
move to.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Apache::Util::size_string issues

Posted by Stas Bekman <st...@stason.org>.
Doug MacEachern wrote:

> On Mon, 28 Jan 2002, Stas Bekman wrote:
>  
> 
>>static MP_INLINE
>>SV *mpxs_Apache__Util_size_string(pTHX_ apr_off_t  size)
>>{
>>     char buff[5];
>>
>>     apr_strfsize(size, buff);
>>
>>     return newSVpvn(buff, 4);
>>}
>>
> 
> that'd be fine.  i'd probably use mpxs_set_targ instead, but that can be
> later.


like this?

static MP_INLINE void mpxs_apr_strfsize(pTHX_ SV *sv, SV *arg)
{
    apr_off_t size = mp_xs_sv2_apr_off_t(arg);
    char *ptr;
    
    mpxs_sv_grow(sv, 5);
    apr_strfsize(size, ptr);
    mpxs_sv_cur_set(sv, strlen(ptr)); /*XXX*/
}

static XS(MPXS_apr_strfsize)
{
    dXSARGS;

    mpxs_usage_items_1("size");

    mpxs_set_targ(mpxs_apr_strfsize, ST(0));
}


>>we want it to be perlish, right?
>>
>>there is a problem with apr_off_t since we typedef it as IV, therefore 
>>if you try:
>>
>>  Apache::Util::size_string(42_000_000_000);
>>
>>it looses data when converting from NV or PV to IV. Which means that we 
>>can use this function only up to 1G numbers or so. Doesn't sound good.
>>
>
> normally off_t is the same size as IV, you'd have to compile apache with
> largefile flags to make it bigger.  might be better to change apr_strftime
> to use a type that can always hold a bigger number.


you mean, apr_strfsize, not apr_strftime

OK, will ask.

what about this:

>>Also apr_strfsize is in the APR::Strings module in the map file. Should 
>>we have APR::Strings::strfsize instead of Apache__Util_size_string.
>>





-- 


_____________________________________________________________________
Stas Bekman             JAm_pH      --   Just Another mod_perl Hacker
http://stason.org/      mod_perl Guide   http://perl.apache.org/guide
mailto:stas@stason.org  http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Apache::Util::size_string issues

Posted by Doug MacEachern <do...@covalent.net>.
On Mon, 28 Jan 2002, Stas Bekman wrote:
 
> static MP_INLINE
> SV *mpxs_Apache__Util_size_string(pTHX_ apr_off_t  size)
> {
>      char buff[5];
> 
>      apr_strfsize(size, buff);
> 
>      return newSVpvn(buff, 4);
> }

that'd be fine.  i'd probably use mpxs_set_targ instead, but that can be
later.

> we want it to be perlish, right?
> 
> there is a problem with apr_off_t since we typedef it as IV, therefore 
> if you try:
> 
>   Apache::Util::size_string(42_000_000_000);
> 
> it looses data when converting from NV or PV to IV. Which means that we 
> can use this function only up to 1G numbers or so. Doesn't sound good.
> 
> Also apr_strfsize is in the APR::Strings module in the map file. Should 
> we have APR::Strings::strfsize instead of Apache__Util_size_string.

normally off_t is the same size as IV, you'd have to compile apache with
largefile flags to make it bigger.  might be better to change apr_strftime
to use a type that can always hold a bigger number.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Apache::Util::size_string issues

Posted by Stas Bekman <st...@stason.org>.
Doug MacEachern wrote:

> On Fri, 25 Jan 2002, Stas Bekman wrote:

>>- size_string (has nothing to do with Apache
>>                or should it stay in Apache::Util?)
>>
> 
> it used to be a string version of ap_send_size(), which no longer exists
> in favor of the new apr_strfsize() function.  we should use that in 2.0
> and add a compat wrapper for Apache::Util::size_string.

OK, how about this then:

static MP_INLINE
SV *mpxs_Apache__Util_size_string(pTHX_ apr_off_t  size)
{
     char buff[5];

     apr_strfsize(size, buff);

     return newSVpvn(buff, 4);
}

we want it to be perlish, right?

there is a problem with apr_off_t since we typedef it as IV, therefore 
if you try:

  Apache::Util::size_string(42_000_000_000);

it looses data when converting from NV or PV to IV. Which means that we 
can use this function only up to 1G numbers or so. Doesn't sound good.

Also apr_strfsize is in the APR::Strings module in the map file. Should 
we have APR::Strings::strfsize instead of Apache__Util_size_string.

_____________________________________________________________________
Stas Bekman             JAm_pH      --   Just Another mod_perl Hacker
http://stason.org/      mod_perl Guide   http://perl.apache.org/guide
mailto:stas@stason.org  http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Apache::Util porting issues

Posted by Doug MacEachern <do...@covalent.net>.
On Mon, 28 Jan 2002, Stas Bekman wrote:
 
> here is the patch.

+1

> Shouldn't this:
> 
>      if (in == &PL_sv_undef) {
>          return NULL;
>      }
> 
> in modperl_xs_sv2request_rec() be removed too? This case is already 
> handled in the third if() and allows for the same trick of getting onto 
> the current request when passing undef.

+1



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Apache::Util porting issues

Posted by Stas Bekman <st...@stason.org>.
>>but the 1.x API, doesn't have $r in the API. Can we have some global
>>variable that always points to the current pool? in the order r->pool,
>>s->pool, or pconf? This will solve many API adjustments problems. Or
>>s->is it a bad idea?
>>
> 
> we should avoid that whenever possible.  but we could change
> modperl_sv2pool to allow &PL_sv_undef which means use
> modperl_global_request_rec_get()->pool if available else
> modperl_global_get_pconf()

here is the patch.

Index: src/modules/perl/modperl_util.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_util.c,v
retrieving revision 1.33
diff -u -r1.33 modperl_util.c
--- src/modules/perl/modperl_util.c     8 Jan 2002 01:29:23 -0000       1.33
+++ src/modules/perl/modperl_util.c     28 Jan 2002 08:01:35 -0000
@@ -174,6 +174,18 @@
      char *classname = NULL;
      IV ptr = 0;

+    /* get the pool from the current request if applicable */
+    if (obj == &PL_sv_undef) {
+        request_rec *r = NULL;
+        (void)modperl_tls_get_request_rec(&r);
+
+        if (r) {
+            return r->pool;
+        }
+
+        return NULL;
+    }
+
      if ((SvROK(obj) && (SvTYPE(SvRV(obj)) == SVt_PVMG))) {
          ptr = SvObjIV(obj);
          classname = SvCLASS(obj);


Shouldn't this:

     if (in == &PL_sv_undef) {
         return NULL;
     }

in modperl_xs_sv2request_rec() be removed too? This case is already 
handled in the third if() and allows for the same trick of getting onto 
the current request when passing undef.

_____________________________________________________________________
Stas Bekman             JAm_pH      --   Just Another mod_perl Hacker
http://stason.org/      mod_perl Guide   http://perl.apache.org/guide
mailto:stas@stason.org  http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Apache::Util porting issues

Posted by Doug MacEachern <do...@covalent.net>.
On Fri, 25 Jan 2002, Stas Bekman wrote:

> I've started porting Apache::Util, here is a summary of issues:
> 
> Now we have ModPerl::Util as well, so is this the right mapping:
> 
> ModPerl::Util:
> --------------
> - untaint
> - exit
> - size_string (has nothing to do with Apache
>                 or should it stay in Apache::Util?)

it used to be a string version of ap_send_size(), which no longer exists
in favor of the new apr_strfsize() function.  we should use that in 2.0
and add a compat wrapper for Apache::Util::size_string.
 
> Apache::Util:
> -------------
> - escape_uri
> - unescape_uri
> - unescape_uri_info
> - escape_html
> - parsedate
> - ht_time           (=> ap_ht_time)
> - validate_password (=> apr_password_validate)
> 
> -----
> if we move some functions from Apache::Util as in 1.x to
> ModPerl::Util, we need to fix these up in compat.pm, right?

in general we should keep the 2.0 version as close to the C functions as
possible, in terms of names and arguments, including those that take a
pool.  then add compat.pm wrappers around the ones that have changed.

> -----
> 
> Apache::Util has @EXPORT_OK for its all functions, should we keep it that
> way? If so should I create xs/Apache/Util/Util_pm with:
> 
> use Exporter ();
> @EXPORT_OK = qw(escape_html escape_uri unescape_uri unescape_uri_info
> 		parsedate ht_time size_string validate_password);
> %EXPORT_TAGS = (all => \@EXPORT_OK);

let's get all the new functions and wrappers written first, then figure
out which exports belong in Apache::Util and which should be in compat.pm
 
> -----
> 
> ht_time()'s API is not compatible with Apache::Util::ht_time with 1.x,
> we cannot provide a fast wrapper since it now requires a pool object.
> so should the code from 1.x be ported without going into APR land (no
> pool required)?

1.x ht_time also requires a pool, we used the util_pool() function
underneath.  in 2.0 we should be able to optionally pass in a pool,
defaulting to something for compat.  either that or move ht_time to a new
package (like Apache::{Date|Time}) that require a pool argument and then
compat.pm has the Apache::Util::ht_time wrapper with default pool.
 
> parsedate(). Should we use
> 
>    APU_DECLARE(apr_time_t) apr_date_parse_http(const char *date);
> 
> but it seem to return microsecs, so need to adjust this.
> 
> should it move into APR::Date now, with compat.pm adjusting for this
> change. Or should we just drop the parsedate() name (and still have it
> in compat.pm) and start using date_parse_http()?
> 
> also we have a new apr_date_parse_rfc in apr_functions.map, if we
> expose it (we already do) it makes sense that date_parse_http will be
> there too.

yes, those should go in APR::Date
 
> ----
> 
> Is this a correct mapping?
> 
> - escape_uri          => ap_os_escape_path (needs pool!)
>           shouldn't it be    ap_escape_url?

same as 1.x, apache has (in both 1.x and 2.0) #define ap_escape_uri
 
> - unescape_uri        => ap_unescape_url
> 
> - unescape_uri_info   => port XS from Apache.xs in 1.x
> 
> should they still be called _uri or renamed to _url per ap_ functions?
> in todo/api.txt, you say:
> 
>    Apache->unescape_url{_info}:
>    not yet implemented.  should be moved to Apache::Util
> 
> you call these _url, not _uri.

i guess there is confusion because apache has:
#define ap_escape_uri
and
int ap_unescape_url

i don't care which we use so long as there is compat somewhere.
 
> also should we introduce these: ap_escape_path_segment, ap_escape_url

don't know what ap_escape_path_segment does.  there is no ap_escape_url,
just the #define ap_escape_uri.  dunno if it should be renamed or not.
 
> ----
> 
> - escape_html         => ap_escape_html (needs pool!)
> 
> First of all what are we going to do about the issues raised by Robin
> Berjon in the recent thread? I'm talking about UTF encodings not being
> escaped.

haven't read that.  but i hope this would be handled in apache and not
special by us.
 
> Second, what to do with the functions requiring a pool object? todo
> says:
> 
>    also need to default to current pool (pconf at startup, r->pool at
>    request time)

the 2.0 functions should take an APR::Pool argument.

> but the 1.x API, doesn't have $r in the API. Can we have some global
> variable that always points to the current pool? in the order r->pool,
> s->pool, or pconf? This will solve many API adjustments problems. Or
> s->is it a bad idea?

we should avoid that whenever possible.  but we could change
modperl_sv2pool to allow &PL_sv_undef which means use
modperl_global_request_rec_get()->pool if available else
modperl_global_get_pconf()
 
> Third, todo/api.txt says:
> 
>    enhancements: consider jeff baker's more robust implementation of
>    my_escape_html(), which should probably be made in apache-2.0 itself
> 
> where is it? is it utf compliant?

i don't remember, ask jeff.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org