Hi,
Nice idea. An http patch was long overdue.
In the implementation it might be a little bit faster if you get the leading
'if' statement outside in the main loop and then call within the if
recover_absolute_uri. You would save an unnecessary function call and save a
gw_assert which already has been done in octstr_strip_blanks anyway.
Shouldn't you also check for port 443?
+1 overall
----- Original Message -----
From: "Stipe Tolj" <[email protected]>
To: "kannel_dev_mailinglist" <[email protected]>
Sent: Friday, January 23, 2009 7:23 PM
Subject: Re: [PATCH] Location header recovering to absoluteURI
Alejandro Guerrieri schrieb:
Apart from renaming "absulte" to "absolute", patch looks fine and I'm +1
with the idea of allowing this behavior.
ehmmm ;) ... yep right, here we go with the typo fixed.
Stipe
--
-------------------------------------------------------------------
Kφlner Landstrasse 419
40589 Dόsseldorf, NRW, Germany
tolj.org system architecture Kannel Software Foundation (KSF)
http://www.tolj.org/ http://www.kannel.org/
mailto:st_{at}_tolj.org mailto:stolj_{at}_kannel.org
-------------------------------------------------------------------
--------------------------------------------------------------------------------
### Eclipse Workspace Patch 1.0
#P gateway-cvs-head
Index: gwlib/http.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/http.c,v
retrieving revision 1.253
diff -u -r1.253 http.c
--- gwlib/http.c 12 Jan 2009 16:46:55 -0000 1.253
+++ gwlib/http.c 23 Jan 2009 17:21:57 -0000
@@ -928,6 +928,49 @@
}
+/*
+ * Recovers a Location header value of format URI /xyz to an
+ * absoluteURI format according to the protocol rules.
+ * This simply implies that we re-create the prefixed scheme,
+ * user/passwd (if any), host and port string and prepend it
+ * to the location URI.
+ */
+static void recover_absolute_uri(HTTPServer *trans, Octstr *loc)
+{
+ Octstr *os;
+
+ gw_assert(loc != NULL && trans != NULL);
+
+ /* we'll only accept locations with a leading / */
+ if (octstr_get_char(loc, 0) == '/') {
+
+ /* scheme */
+ os = trans->ssl ? octstr_create("https://";) :
+ octstr_create("http://";);
+
+ /* credentials, if any */
+ if (trans->username && trans->password) {
+ octstr_append(os, trans->username);
+ octstr_append_char(os, ':');
+ octstr_append(os, trans->password);
+ octstr_append_char(os, '@');
+ }
+
+ /* host */
+ octstr_append(os, trans->host);
+
+ /* port, only added if literally not default. */
+ if (trans->port != 80) {
+ octstr_format_append(os, ":%ld", trans->port);
+ }
+
+ /* prepend the created octstr to the loc, and destroy then. */
+ octstr_insert(loc, os, 0);
+ octstr_destroy(os);
+ }
+}
+
+
/*
* Read and parse the status response line from an HTTP server.
* Fill in trans->persistent and trans->status with the findings.
@@ -1117,9 +1160,30 @@
/*
* This is a redirected response, we have to follow.
- * Clean up all trans stuff for the next request we do.
+ *
+ * According to HTTP/1.1 (RFC 2616), section 14.30 any Location
+ * header value should be 'absoluteURI', which is defined in
+ * RFC 2616, section 3.2.1 General Syntax, and specifically in
+ * RFC 2396, section 3 URI Syntactic Components as
+ *
+ * absoluteURI = scheme ":" ( hier_part | opaque_part )
+ *
+ * Some HTTP servers 'interpret' a leading UDI / as that kind
+ * of absoluteURI, which is not correct, following the protocol
in
+ * detail. But we'll try to recover from that misleaded
+ * interpreation and try to convert the partly absoluteURI to a
+ * fully qualified absoluteURI.
+ *
+ * http_URL = "http:" "//" [ userid : password "@"] host
+ * [ ":" port ] [ abs_path [ "?" query ]]
+ *
*/
octstr_strip_blanks(h);
+ recover_absolute_uri(trans, h);
+
+ /*
+ * Clean up all trans stuff for the next request we do.
+ */
octstr_destroy(trans->url);
octstr_destroy(trans->host);
trans->port = 0;
