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.

diff --git a/libsoup/soup-message.c b/libsoup/soup-message.c
index 93c2522..64aa42e 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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