[libsoup] Warn if the user tries to load a URI containing a fragment.



commit 19cc4d0c0956ec92f06a0a3a746ad0434e47bc9c
Author: Dan Winship <danw gnome org>
Date:   Wed Aug 26 11:09:34 2009 -0400

    Warn if the user tries to load a URI containing a fragment.
    
    Previously fragments were silently ignored when doing direct HTTP, but
    were passed to the proxy when using a proxy (which would sometimes
    cause the request to fail, depending on the server configuration).
    Since fragments are not meaningful at the HTTP level, callers should
    not be passing URIs with fragments to SoupMessage, so warn if they do,
    and then consistently ignore the fragment after that.
    
    Also update SoupSession to fix up fragments in Location headers so
    that SoupMessage won't warn about them (since that's the server's
    fault, not the caller's), and add a test to redirect-test for that.
    
    See also https://bugs.webkit.org/show_bug.cgi?id=28687 for some
    additional discussion.

 libsoup/soup-message.c |   19 ++++++++++++++++++-
 libsoup/soup-session.c |    7 +++++++
 tests/redirect-test.c  |   10 ++++++++++
 3 files changed, 35 insertions(+), 1 deletions(-)
---
diff --git a/libsoup/soup-message.c b/libsoup/soup-message.c
index 93c2522..64aa42e 100644
--- a/libsoup/soup-message.c
+++ b/libsoup/soup-message.c
@@ -659,7 +659,11 @@ get_property (GObject *object, guint prop_id,
  * @method: the HTTP method for the created request
  * @uri_string: the destination endpoint (as a string)
  * 
- * Creates a new empty #SoupMessage, which will connect to @uri
+ * Creates a new empty #SoupMessage, which will connect to @uri.
+ *
+ * @uri should not include a fragment identifier (ie, a "#" followed
+ * by an HTML anchor name, etc); fragments are not meaningful at the
+ * HTTP level.
  *
  * Return value: the new #SoupMessage (or %NULL if @uri could not
  * be parsed).
@@ -693,6 +697,9 @@ soup_message_new (const char *method, const char *uri_string)
  * 
  * Creates a new empty #SoupMessage, which will connect to @uri
  *
+ * @uri should not include a fragment identifier; fragments are not
+ * meaningful at the HTTP level.
+ *
  * Return value: the new #SoupMessage
  */
 SoupMessage *
@@ -1410,6 +1417,8 @@ soup_message_is_keepalive (SoupMessage *msg)
  * Sets @msg's URI to @uri. If @msg has already been sent and you want
  * to re-send it with the new URI, you need to call
  * soup_session_requeue_message().
+ *
+ * @uri should not include a fragment identifier.
  **/
 void
 soup_message_set_uri (SoupMessage *msg, SoupURI *uri)
@@ -1427,6 +1436,14 @@ soup_message_set_uri (SoupMessage *msg, SoupURI *uri)
 	}
 	priv->uri = soup_uri_copy (uri);
 
+	if (priv->uri && priv->uri->fragment) {
+		char *uristr = soup_uri_to_string (priv->uri, FALSE);
+		g_warning ("soup_message_set_uri: stripping fragment identifier from URI '%s'",
+			   uristr);
+		g_free (uristr);
+		soup_uri_set_fragment (priv->uri, NULL);
+	}
+
 	g_object_notify (G_OBJECT (msg), SOUP_MESSAGE_URI);
 }
 
diff --git a/libsoup/soup-session.c b/libsoup/soup-session.c
index c6d8765..2428cfe 100644
--- a/libsoup/soup-session.c
+++ b/libsoup/soup-session.c
@@ -1123,6 +1123,13 @@ redirect_handler (SoupMessage *msg, gpointer user_data)
 		return;
 	}
 
+	/* URI fragments are also not allowed in Location, but again,
+	 * some sites do this. Strip it to keep soup_message_set_uri()
+	 * from warning about it. (If the application wants to obey
+	 * the fragment anyway it can reparse Location itself.)
+	 */
+	soup_uri_set_fragment (new_uri, NULL);
+
 	soup_message_set_uri (msg, new_uri);
 	soup_uri_free (new_uri);
 
diff --git a/tests/redirect-test.c b/tests/redirect-test.c
index cd6f1a5..b99aabf 100644
--- a/tests/redirect-test.c
+++ b/tests/redirect-test.c
@@ -100,6 +100,11 @@ static struct {
 	{ { { "POST", "/307", 307 },
 	    { NULL } }, 307 },
 
+	/* Test behavior with Location header containing URI fragment */
+	{ { { "GET", "/bad-with-fragment", 302 },
+	    { "GET", "/", 200 },
+	    { NULL } }, 200 },
+
 	/* Test behavior with recoverably-bad Location header */
 	{ { { "GET", "/bad", 302 },
 	    { "GET", "/bad%20with%20spaces", 200 },
@@ -239,6 +244,11 @@ server_callback (SoupServer *server, SoupMessage *msg,
 			soup_message_headers_replace (msg->response_headers,
 						      "Location",
 						      "about:blank");
+		} else if (!strcmp (path, "/bad-with-fragment")) {
+			soup_message_set_status (msg, SOUP_STATUS_FOUND);
+			soup_message_headers_replace (msg->response_headers,
+						      "Location",
+						      "/#fragment");
 		} else if (!strcmp (path, "/bad with spaces"))
 			soup_message_set_status (msg, SOUP_STATUS_OK);
 		else



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]