On 11 jan 2009, at 18.41, Martin Stjernholm wrote:
> I'm cc'ing Jonas on this one, since he in roxen.pike rev 1.1001
> introduced the Standards.URI stuff that started stripping the standard
> port numbers.
True, but to be honest I didn't introduce Standards.URI aside from a
single place; it was already used in existing code, but not in a way
that preserved the IPv6 parsing capabilities that I was aiming for.
I'm fairly sure I recognized the dropping of port numbers for standard
ports but I may have forgotten to consider the calls in http.pike.
> To recap, what Stephen does in this patch is to use Standards.URI to
> canonicalize the incoming urls in the http protocol, so that they can
> match the Standards.URI-normalized urls registered for the port:
>
> --- a/server/protocols/http.pike
> +++ b/server/protocols/http.pike
> @@ -2516,8 +2516,8 @@ void got_data(mixed fooid, string s, void|int
> chained)
>
> if (misc->host) {
> conf =
> - port_obj->find_configuration_for_url(port_obj->url_prefix +
> - misc->host + raw_url,
> + port_obj->find_configuration_for_url(
> + (string)Standards.URI(port_obj->url_prefix + misc->host
> + raw_url),
> this_object());
> if (conf->query("default_server")) {
> cache_key = port_obj->name+"://"+misc->host+raw_url;
> @@ -2525,7 +2525,8 @@ void got_data(mixed fooid, string s, void|int
> chained)
> } else {
> conf =
> port_obj->find_configuration_for_url(port_obj->url_prefix +
> "*:" +
> - port_obj->port +
> raw_url,
> + (string)Standards.URI(port_obj->url_prefix +
> + "*:" + port_obj->port + raw_url),
> this_object());
> }
> }
>
> I don't think we want the overhead of fiddling with Standards.URI
> objects there - that code is among the hottest sections in the whole
> server. I think it's better to reinstate the old behavior where the
> port always is included in the urls registered in the Protocol
> objects.
>
> So a partial revert appears necessary, at least in register_url
> (normalize_url is afaik mostly used for viewing, so that one can
> probably continue to strip the standard port). Jonas' fix seems to
> have a wider scope though, so he better speak his mind on this one, in
> case there is an intentional design change to drop the port numbers.
Maybe an option in Standards.URI to coerce to a string form including
port numbers? Reverting my changes will likely break IPv6 adresses.
Otherwise dropping the port number when constructing the parameter to
find_configuration_for_url() could be good enough? I don't know if the
overhead in Standards.URI is significant or if IPv6 support requires
it in this particular place anyway.
-- Jonas
|