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?
1. The patching of segfault was the most important.
2. I actually like the weird //localhost/foo/bar as it'll force the user to re-adjust their code, whereas /foo/bar may just work.
***
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 :-)
I'll adjust the compat code to do as you suggest.
Thank you, Bill!
__________________________________________________________________ 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
