At 01:57 AM 2/18/2003, Stas Bekman wrote:
>the uri unparse code wasn't properly ported from 1.0. The default scheme was
>dropped on the floor, now causing segfaults. In order to stay back compatible
>with 1.0 code the following patch should be applied.
>
>In order to avoid segfaults, and break backcompat, the code should at least do:
> /* Construct scheme://site string */
> if (uptr->hostname && uptr->scheme) {
>
>and not just if (uptr->hostname)
++1 to that. My comments about your patch are a little lower, but
I first decided to struggle through your suggestion above. I went
one step further, in violation of (but also in the spirit of) rfc2396,
and have committed the fix described here to avoid segfaults...
<scheme>: is not optional by rfc2396 (the violation I mentioned), but if
the user fails to provide one, who are we to argue? Now the question
becomes what to include if scheme is omitted but hostname (and
optionally port) are provided? My patch goes ahead and assembles
them, but I was left with the question of 'how?'.
I looked to this defintion;
absoluteURI = scheme ":" ( hier_part | opaque_part )
So the colon ties the scheme to the absolute URI, not "://". So my segfault
workaround simply drops "scheme:" if scheme is NULL. This eliminates the
segfault, and assembles the (heir_part) for a resources that include an
authority (sec 2.3 of the rfc) by prepending the '//' if the authority
(hostname)
is present.
Comments about that patch that's now committed?
***
Now to my comments about your patch below. As I was reviewing your
patch, I am convinced that the current apr_uri implementation is and was
intended to be scheme-agnostic. Your patch would break the agnostic
behavior of assigning an arbitrary scheme. I don't think that is in the
interest of forcing the API's users to be explicit. (I had the impression
we were defaulting the port to 80 when we were chatting on IRC, I now
see we don't make any such assumptions.)
A better solution to Apache::compat's implementation would be to test the
scheme and (if empty) fill it in with http: since that's *Apache::compat*'s
preference. But it really shouldn't be a universal preference. E.g. a pop
or nntp parser wouldn't appreciate those defaults, while Apache::compat
certainly likes that default.
If compat's job is to make everything look the same for the 1.3 module
porter, then do so ;-) But let's not twist this API to make it http-centric
again. Since it's simple for the user to fill in NULL members of the
apr_uri_t with their preferred defaults, let them do so.
At least we've quit segfaulting, which was the hardest debugging job for
the casual user. Now they get a string they should be unhappy with,
and can decide how to react and fix their code without gdb :-)
Bill
>Index: srclib/apr-util/include/apr_uri.h
>===================================================================
>RCS file: /home/cvspublic/apr-util/include/apr_uri.h,v
>retrieving revision 1.15
>diff -u -r1.15 apr_uri.h
>--- srclib/apr-util/include/apr_uri.h 1 Jan 2003 00:02:20 -0000 1.15
>+++ srclib/apr-util/include/apr_uri.h 18 Feb 2003 07:55:53 -0000
>@@ -101,6 +101,8 @@
> #define APR_URI_TIP_DEFAULT_PORT 3372 /**< default TIP port */
> #define APR_URI_SIP_DEFAULT_PORT 5060 /**< default SIP port */
>
>+#define APR_URI_DEFAULT_SCHEME "http"
>+
> /** Flags passed to unparse_uri_components(): */
> /** suppress "scheme://[EMAIL PROTECTED]:port" */
> #define APR_URI_UNP_OMITSITEPART (1U<<0)
>Index: srclib/apr-util/uri/apr_uri.c
>===================================================================
>RCS file: /home/cvspublic/apr-util/uri/apr_uri.c,v
>retrieving revision 1.16
>diff -u -r1.16 apr_uri.c
>--- srclib/apr-util/uri/apr_uri.c 1 Jan 2003 00:02:22 -0000 1.16
>+++ srclib/apr-util/uri/apr_uri.c 18 Feb 2003 07:55:53 -0000
>@@ -167,6 +167,10 @@
> rbrk = "]";
> }
>
>+ if (!uptr->scheme) {
>+ uptr->scheme = APR_URI_DEFAULT_SCHEME;
>+ }
>+
> is_default_port =
> (uptr->port_str == NULL ||
> uptr->port == 0 ||
>
>__________________________________________________________________
>Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
>http://stason.org/ mod_perl Guide ---> http://perl.apache.org
>mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
>http://modperlbook.org http://apache.org http://ticketmaster.com
>
