[libsoup] soup-message-server-io: handle unsatisfiable Range requests correctly



commit 8524d46134bce62ee6d8a534a56c24da90651b9c
Author: Dan Winship <danw gnome org>
Date:   Sun Aug 4 13:08:29 2013 -0400

    soup-message-server-io: handle unsatisfiable Range requests correctly
    
    soup_message_headers_get_ranges() doesn't check satisfiability (and
    can't without changing its API) so add a new
    soup_message_headers_get_ranges_internal(), and use that from
    soup-message-server-io so we can handle this properly. Test that it
    works from range-test.
    
    Pointed out on the mailing list by Kai Lüke.

 libsoup/soup-message-headers.c   |  114 +++++++++++++++++++++++++-------------
 libsoup/soup-message-server-io.c |   13 +++-
 libsoup/soup-misc-private.h      |    7 ++
 tests/range-test.c               |   85 ++++++++++++++++++++++++----
 4 files changed, 165 insertions(+), 54 deletions(-)
---
diff --git a/libsoup/soup-message-headers.c b/libsoup/soup-message-headers.c
index 876db61..f488c1c 100644
--- a/libsoup/soup-message-headers.c
+++ b/libsoup/soup-message-headers.c
@@ -9,6 +9,7 @@
 
 #include "soup-message-headers.h"
 #include "soup.h"
+#include "soup-misc-private.h"
 
 /**
  * SECTION:soup-message-headers
@@ -858,56 +859,40 @@ sort_ranges (gconstpointer a, gconstpointer b)
        return ra->start - rb->start;
 }
 
-/**
- * soup_message_headers_get_ranges:
- * @hdrs: a #SoupMessageHeaders
- * @total_length: the total_length of the response body
- * @ranges: (out): return location for an array of #SoupRange
- * @length: the length of the returned array
- *
- * Parses @hdrs's Range header and returns an array of the requested
- * byte ranges. The returned array must be freed with
- * soup_message_headers_free_ranges().
- *
- * If @total_length is non-0, its value will be used to adjust the
- * returned ranges to have explicit start and end values, and the
- * returned ranges will be sorted and non-overlapping. If
- * @total_length is 0, then some ranges may have an end value of -1,
- * as described under #SoupRange, and some of the ranges may be
- * redundant.
- *
- * Return value: %TRUE if @hdrs contained a "Range" header containing
- * byte ranges which could be parsed, %FALSE otherwise (in which case
- * @range and @length will not be set).
- *
- * Since: 2.26
- **/
-gboolean
-soup_message_headers_get_ranges (SoupMessageHeaders  *hdrs,
-                                goffset              total_length,
-                                SoupRange          **ranges,
-                                int                 *length)
+/* like soup_message_headers_get_ranges(), except it returns:
+ *   SOUP_STATUS_OK if there is no Range or it should be ignored.
+ *   SOUP_STATUS_PARTIAL_CONTENT if there is at least one satisfiable range.
+ *   SOUP_STATUS_REQUESTED_RANGE_NOT_SATISFIABLE if @check_satisfiable
+ *     is %TRUE and the request is not satisfiable given @total_length.
+ */
+guint
+soup_message_headers_get_ranges_internal (SoupMessageHeaders  *hdrs,
+                                         goffset              total_length,
+                                         gboolean             check_satisfiable,
+                                         SoupRange          **ranges,
+                                         int                 *length)
 {
        const char *range = soup_message_headers_get_one (hdrs, "Range");
        GSList *range_list, *r;
        GArray *array;
        char *spec, *end;
        int i;
+       guint status = SOUP_STATUS_OK;
 
        if (!range || strncmp (range, "bytes", 5) != 0)
-               return FALSE;
+               return status;
 
        range += 5;
        while (g_ascii_isspace (*range))
                range++;
        if (*range++ != '=')
-               return FALSE;
+               return status;
        while (g_ascii_isspace (*range))
                range++;
 
        range_list = soup_header_parse_list (range);
        if (!range_list)
-               return FALSE;
+               return status;
 
        array = g_array_new (FALSE, FALSE, sizeof (SoupRange));
        for (r = range_list; r; r = r->next) {
@@ -921,22 +906,34 @@ soup_message_headers_get_ranges (SoupMessageHeaders  *hdrs,
                        cur.start = g_ascii_strtoull (spec, &end, 10);
                        if (*end == '-')
                                end++;
-                       if (*end)
+                       if (*end) {
                                cur.end = g_ascii_strtoull (end, &end, 10);
-                       else
+                               if (cur.end < cur.start) {
+                                       status = SOUP_STATUS_OK;
+                                       break;
+                               }
+                       } else
                                cur.end = total_length - 1;
                }
                if (*end) {
-                       g_array_free (array, TRUE);
-                       soup_header_free_list (range_list);
-                       return FALSE;
+                       status = SOUP_STATUS_OK;
+                       break;
+               } else if (check_satisfiable && cur.start >= total_length) {
+                       if (status == SOUP_STATUS_OK)
+                               status = SOUP_STATUS_REQUESTED_RANGE_NOT_SATISFIABLE;
+                       continue;
                }
 
                g_array_append_val (array, cur);
+               status = SOUP_STATUS_PARTIAL_CONTENT;
        }
-
        soup_header_free_list (range_list);
 
+       if (status != SOUP_STATUS_PARTIAL_CONTENT) {
+               g_array_free (array, TRUE);
+               return status;
+       }
+
        if (total_length) {
                g_array_sort (array, sort_ranges);
                for (i = 1; i < array->len; i++) {
@@ -954,7 +951,46 @@ soup_message_headers_get_ranges (SoupMessageHeaders  *hdrs,
        *length = array->len;
 
        g_array_free (array, FALSE);
-       return TRUE;
+       return SOUP_STATUS_PARTIAL_CONTENT;
+}
+
+/**
+ * soup_message_headers_get_ranges:
+ * @hdrs: a #SoupMessageHeaders
+ * @total_length: the total_length of the response body
+ * @ranges: (out): return location for an array of #SoupRange
+ * @length: the length of the returned array
+ *
+ * Parses @hdrs's Range header and returns an array of the requested
+ * byte ranges. The returned array must be freed with
+ * soup_message_headers_free_ranges().
+ *
+ * If @total_length is non-0, its value will be used to adjust the
+ * returned ranges to have explicit start and end values, and the
+ * returned ranges will be sorted and non-overlapping. If
+ * @total_length is 0, then some ranges may have an end value of -1,
+ * as described under #SoupRange, and some of the ranges may be
+ * redundant.
+ *
+ * Beware that even if given a @total_length, this function does not
+ * check that the ranges are satisfiable.
+ *
+ * Return value: %TRUE if @hdrs contained a syntactically-valid
+ * "Range" header, %FALSE otherwise (in which case @range and @length
+ * will not be set).
+ *
+ * Since: 2.26
+ **/
+gboolean
+soup_message_headers_get_ranges (SoupMessageHeaders  *hdrs,
+                                goffset              total_length,
+                                SoupRange          **ranges,
+                                int                 *length)
+{
+       guint status;
+
+       status = soup_message_headers_get_ranges_internal (hdrs, total_length, FALSE, ranges, length);
+       return status == SOUP_STATUS_PARTIAL_CONTENT;
 }
 
 /**
diff --git a/libsoup/soup-message-server-io.c b/libsoup/soup-message-server-io.c
index e85896b..814ae7e 100644
--- a/libsoup/soup-message-server-io.c
+++ b/libsoup/soup-message-server-io.c
@@ -119,6 +119,7 @@ handle_partial_get (SoupMessage *msg)
        SoupRange *ranges;
        int nranges;
        SoupBuffer *full_response;
+       guint status;
 
        /* Make sure the message is set up right for us to return a
         * partial response; it has to be a GET, the status must be
@@ -137,9 +138,15 @@ handle_partial_get (SoupMessage *msg)
        /* Oh, and there has to have been a valid Range header on the
         * request, of course.
         */
-       if (!soup_message_headers_get_ranges (msg->request_headers,
-                                             msg->response_body->length,
-                                             &ranges, &nranges))
+       status = soup_message_headers_get_ranges_internal (msg->request_headers,
+                                                          msg->response_body->length,
+                                                          TRUE,
+                                                          &ranges, &nranges);
+       if (status == SOUP_STATUS_REQUESTED_RANGE_NOT_SATISFIABLE) {
+               soup_message_set_status (msg, status);
+               soup_message_body_truncate (msg->response_body);
+               return;
+       } else if (status != SOUP_STATUS_PARTIAL_CONTENT)
                return;
 
        full_response = soup_message_body_flatten (msg->response_body);
diff --git a/libsoup/soup-misc-private.h b/libsoup/soup-misc-private.h
index d03bc77..0276870 100644
--- a/libsoup/soup-misc-private.h
+++ b/libsoup/soup-misc-private.h
@@ -8,6 +8,7 @@
 #define SOUP_MISC_PRIVATE_H 1
 
 #include "soup-socket.h"
+#include "soup-message-headers.h"
 
 char *uri_decoded_copy (const char *str, int length, int *decoded_length);
 char *soup_uri_to_string_internal (SoupURI *uri, gboolean just_path_and_query,
@@ -47,4 +48,10 @@ GSource *soup_add_completion_reffed (GMainContext *async_context,
                                     GSourceFunc   function,
                                     gpointer      data);
 
+guint soup_message_headers_get_ranges_internal (SoupMessageHeaders  *hdrs,
+                                               goffset              total_length,
+                                               gboolean             check_satisfiable,
+                                               SoupRange          **ranges,
+                                               int                 *length);
+
 #endif /* SOUP_MISC_PRIVATE_H */
diff --git a/tests/range-test.c b/tests/range-test.c
index e7d01d2..00b8567 100644
--- a/tests/range-test.c
+++ b/tests/range-test.c
@@ -78,7 +78,7 @@ check_part (SoupMessageHeaders *headers, const char *body, gsize body_len,
 
 static void
 do_single_range (SoupSession *session, SoupMessage *msg,
-                int start, int end)
+                int start, int end, gboolean succeed)
 {
        const char *content_type;
 
@@ -87,11 +87,31 @@ do_single_range (SoupSession *session, SoupMessage *msg,
 
        soup_session_send_message (session, msg);
 
-       if (msg->status_code != SOUP_STATUS_PARTIAL_CONTENT) {
-               debug_printf (1, "    Unexpected status %d %s\n",
-                             msg->status_code, msg->reason_phrase);
+       if (succeed) {
+               if (msg->status_code != SOUP_STATUS_PARTIAL_CONTENT) {
+                       debug_printf (1, "    Unexpected status %d %s\n",
+                                     msg->status_code, msg->reason_phrase);
+                       g_object_unref (msg);
+                       errors++;
+                       return;
+               }
+       } else {
+               if (msg->status_code == SOUP_STATUS_REQUESTED_RANGE_NOT_SATISFIABLE) {
+                       debug_printf (1, "    Got expected %d %s\n",
+                                     msg->status_code, msg->reason_phrase);
+               } else {
+                       const char *content_range;
+
+                       debug_printf (1, "    Unexpected status %d %s\n",
+                                     msg->status_code, msg->reason_phrase);
+                       content_range = soup_message_headers_get_one (msg->response_headers,
+                                                                     "Content-Range");
+                       if (content_range)
+                               debug_printf (1, "    Content-Range: %s\n", content_range);
+                       errors++;
+               }
+
                g_object_unref (msg);
-               errors++;
                return;
        }
 
@@ -111,13 +131,13 @@ do_single_range (SoupSession *session, SoupMessage *msg,
 
 static void
 request_single_range (SoupSession *session, const char *uri,
-                     int start, int end)
+                     int start, int end, gboolean succeed)
 {
        SoupMessage *msg;
 
        msg = soup_message_new ("GET", uri);
        soup_message_headers_set_range (msg->request_headers, start, end);
-       do_single_range (session, msg, start, end);
+       do_single_range (session, msg, start, end, succeed);
 }
 
 static void
@@ -198,7 +218,8 @@ request_double_range (SoupSession *session, const char *uri,
        if (expected_return_ranges == 1) {
                do_single_range (session, msg,
                                 MIN (first_start, second_start),
-                                MAX (first_end, second_end));
+                                MAX (first_end, second_end),
+                                TRUE);
        } else
                do_multi_range (session, msg, expected_return_ranges);
 }
@@ -225,12 +246,34 @@ request_triple_range (SoupSession *session, const char *uri,
        if (expected_return_ranges == 1) {
                do_single_range (session, msg,
                                 MIN (first_start, MIN (second_start, third_start)),
-                                MAX (first_end, MAX (second_end, third_end)));
+                                MAX (first_end, MAX (second_end, third_end)),
+                                TRUE);
        } else
                do_multi_range (session, msg, expected_return_ranges);
 }
 
 static void
+request_semi_invalid_range (SoupSession *session, const char *uri,
+                           int first_good_start, int first_good_end,
+                           int bad_start, int bad_end,
+                           int second_good_start, int second_good_end)
+{
+       SoupMessage *msg;
+       SoupRange ranges[3];
+
+       msg = soup_message_new ("GET", uri);
+       ranges[0].start = first_good_start;
+       ranges[0].end = first_good_end;
+       ranges[1].start = bad_start;
+       ranges[1].end = bad_end;
+       ranges[2].start = second_good_start;
+       ranges[2].end = second_good_end;
+       soup_message_headers_set_ranges (msg->request_headers, ranges, 3);
+
+       do_multi_range (session, msg, 2);
+}
+
+static void
 do_range_test (SoupSession *session, const char *uri,
               gboolean expect_coalesce, gboolean expect_partial_coalesce)
 {
@@ -258,7 +301,8 @@ do_range_test (SoupSession *session, const char *uri,
        /* A: 0, simple request */
        debug_printf (1, "Requesting %d-%d\n", 0 * twelfths, 1 * twelfths);
        request_single_range (session, uri,
-                             0 * twelfths, 1 * twelfths);
+                             0 * twelfths, 1 * twelfths,
+                             TRUE);
 
        /* B: 11, end-relative request. These two are mostly redundant
         * in terms of data coverage, but they may still catch
@@ -266,10 +310,12 @@ do_range_test (SoupSession *session, const char *uri,
         */
        debug_printf (1, "Requesting %d-\n", 11 * twelfths);
        request_single_range (session, uri,
-                             11 * twelfths, -1);
+                             11 * twelfths, -1,
+                             TRUE);
        debug_printf (1, "Requesting -%d\n", 1 * twelfths);
        request_single_range (session, uri,
-                             -1 * twelfths, -1);
+                             -1 * twelfths, -1,
+                             TRUE);
 
        /* C: 2 and 5 */
        debug_printf (1, "Requesting %d-%d,%d-%d\n",
@@ -318,6 +364,21 @@ do_range_test (SoupSession *session, const char *uri,
                debug_printf (1, "\nfull_response and test_response don't match\n");
                errors++;
        }
+
+       debug_printf (1, "Requesting (invalid) %d-%d\n",
+                     (int) full_response->length + 1,
+                     (int) full_response->length + 100);
+       request_single_range (session, uri,
+                             full_response->length + 1, full_response->length + 100,
+                             FALSE);
+
+       debug_printf (1, "Requesting (semi-invalid) 1-10,%d-%d,20-30\n",
+                     (int) full_response->length + 1,
+                     (int) full_response->length + 100);
+       request_semi_invalid_range (session, uri,
+                                   1, 10,
+                                   full_response->length + 1, full_response->length + 100,
+                                   20, 30); 
 }
 
 static void


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