From 407c683524bb362cbc39b8445015e4b06e5296a5 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 24 Sep 2020 17:49:48 +0100 Subject: [PATCH 1/5] libostree: Add support for ETag and Last-Modified headers Add support in the soup and curl fetchers to send the `If-None-Match` and `If-Modified-Since` request headers, and pass on the `ETag` and `Last-Modified` response headers. This currently introduces no functional changes, but once call sites provide the appropriate integration, this will allow HTTP caching to happen with requests (typically with metadata requests, where the data is not immutable due to being content-addressed). That should reduce bandwidth requirements. Signed-off-by: Philip Withnall --- src/libostree/ostree-fetcher-curl.c | 251 +++++++++++++++++++++++++++- src/libostree/ostree-fetcher-soup.c | 81 ++++++++- src/libostree/ostree-fetcher-util.c | 43 ++++- src/libostree/ostree-fetcher-util.h | 10 ++ src/libostree/ostree-fetcher.h | 10 ++ src/libostree/ostree-metalink.c | 5 +- src/libostree/ostree-repo-pull.c | 29 +++- 7 files changed, 410 insertions(+), 19 deletions(-) diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 0ce3ff002b..975508ebff 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -99,10 +99,16 @@ struct FetcherRequest { guint64 current_size; guint64 max_size; OstreeFetcherRequestFlags flags; + struct curl_slist *req_headers; + char *if_none_match; /* request ETag */ + guint64 if_modified_since; /* seconds since the epoch */ gboolean is_membuf; GError *caught_write_error; GLnxTmpfile tmpf; GString *output_buf; + gboolean out_not_modified; /* TRUE if the server gave a HTTP 304 Not Modified response, which we don’t propagate as an error */ + char *out_etag; /* response ETag */ + guint64 out_last_modified; /* response Last-Modified, seconds since the epoch */ CURL *easy; char error[CURL_ERROR_SIZE]; @@ -335,7 +341,17 @@ check_multi_info (OstreeFetcher *fetcher) else { curl_easy_getinfo (easy, CURLINFO_RESPONSE_CODE, &response); - if (!is_file && !(response >= 200 && response < 300)) + + if (!is_file && response == 304 && + (req->if_none_match != NULL || req->if_modified_since > 0)) + { + /* Version on the server is unchanged from the version we have + * cached locally; report this as an out-argument, a zero-length + * response buffer, and no error. */ + req->out_not_modified = TRUE; + } + + if (!is_file && !(response >= 200 && response < 300) && response != 304) { GIOErrorEnum giocode = _ostree_fetcher_http_status_code_to_io_error (response); @@ -575,6 +591,175 @@ write_cb (void *ptr, size_t size, size_t nmemb, void *data) return realsize; } +/* @buf must already be known to be long enough */ +static gboolean +parse_uint (const char *buf, + guint n_digits, + guint min, + guint max, + guint *out) +{ + guint64 number; + const char *end_ptr = NULL; + gint saved_errno = 0; + + g_return_val_if_fail (n_digits == 2 || n_digits == 4, FALSE); + g_return_val_if_fail (out != NULL, FALSE); + + errno = 0; + number = g_ascii_strtoull (buf, (gchar **)&end_ptr, 10); + saved_errno = errno; + + if (!g_ascii_isdigit (buf[0]) || + saved_errno != 0 || + end_ptr == NULL || + end_ptr != buf + n_digits || + number < min || + number > max) + return FALSE; + + *out = number; + return TRUE; +} + +/* Locale-independent parsing for RFC 2616 date/times. + * + * Reference: https://tools.ietf.org/html/rfc2616#section-3.3.1 + * + * Syntax: + * , :: GMT + * + * Note that this only accepts the full-year and GMT formats specified by + * RFC 1123. It doesn’t accept RFC 850 or asctime formats. + * + * Example: + * Wed, 21 Oct 2015 07:28:00 GMT + */ +static GDateTime * +parse_rfc2616_date_time (const char *buf, + size_t len) +{ + guint day_int, year_int, hour_int, minute_int, second_int; + const char *day_names[] = + { + "Mon", + "Tue", + "Wed", + "Thu", + "Fri", + "Sat", + "Sun", + }; + size_t day_name_index; + const char *month_names[] = + { + "Jan", + "Feb", + "Mar", + "Apr", + "May", + "Jun", + "Jul", + "Aug", + "Sep", + "Oct", + "Nov", + "Dec", + }; + size_t month_name_index; + + if (len != 29) + return NULL; + + const char *day_name = buf; + const char *day = buf + 5; + const char *month_name = day + 3; + const char *year = month_name + 4; + const char *hour = year + 5; + const char *minute = hour + 3; + const char *second = minute + 3; + const char *tz = second + 3; + + for (day_name_index = 0; day_name_index < G_N_ELEMENTS (day_names); day_name_index++) + { + if (strncmp (day_names[day_name_index], day_name, 3) == 0) + break; + } + if (day_name_index >= G_N_ELEMENTS (day_names)) + return NULL; + /* don’t validate whether the day_name matches the rest of the date */ + if (*(day_name + 3) != ',' || *(day_name + 4) != ' ') + return NULL; + if (!parse_uint (day, 2, 1, 31, &day_int)) + return NULL; + if (*(day + 2) != ' ') + return NULL; + for (month_name_index = 0; month_name_index < G_N_ELEMENTS (month_names); month_name_index++) + { + if (strncmp (month_names[month_name_index], month_name, 3) == 0) + break; + } + if (month_name_index >= G_N_ELEMENTS (month_names)) + return NULL; + if (*(month_name + 3) != ' ') + return NULL; + if (!parse_uint (year, 4, 0, 9999, &year_int)) + return NULL; + if (*(year + 4) != ' ') + return NULL; + if (!parse_uint (hour, 2, 0, 23, &hour_int)) + return NULL; + if (*(hour + 2) != ':') + return NULL; + if (!parse_uint (minute, 2, 0, 59, &minute_int)) + return NULL; + if (*(minute + 2) != ':') + return NULL; + if (!parse_uint (second, 2, 0, 60, &second_int)) /* allow leap seconds */ + return NULL; + if (*(second + 2) != ' ') + return NULL; + if (strncmp (tz, "GMT", 3) != 0) + return NULL; + + return g_date_time_new_utc (year_int, month_name_index + 1, day_int, + hour_int, minute_int, second_int); +} + +/* CURLOPT_HEADERFUNCTION */ +static size_t +response_header_cb (const char *buffer, size_t size, size_t n_items, void *user_data) +{ + const size_t real_size = size * n_items; + GTask *task = G_TASK (user_data); + FetcherRequest *req; + + /* libcurl says that @size is always 1, but let’s check + * See https://curl.haxx.se/libcurl/c/CURLOPT_HEADERFUNCTION.html */ + g_assert (size == 1); + + req = g_task_get_task_data (task); + + const char *etag_header = "ETag: "; + const char *last_modified_header = "Last-Modified: "; + + if (real_size > strlen (etag_header) && + strncasecmp (buffer, etag_header, strlen (etag_header)) == 0) + { + g_clear_pointer (&req->out_etag, g_free); + req->out_etag = g_strstrip (g_strdup (buffer + strlen (etag_header))); + } + else if (real_size > strlen (last_modified_header) && + strncasecmp (buffer, last_modified_header, strlen (last_modified_header)) == 0) + { + g_autofree char *lm_buf = g_strstrip (g_strdup (buffer + strlen (last_modified_header))); + g_autoptr(GDateTime) dt = parse_rfc2616_date_time (lm_buf, strlen (lm_buf)); + req->out_last_modified = (dt != NULL) ? g_date_time_to_unix (dt) : 0; + } + + return real_size; +} + /* CURLOPT_PROGRESSFUNCTION */ static int prog_cb (void *p, double dltotal, double dlnow, double ult, double uln) @@ -600,6 +785,9 @@ request_unref (FetcherRequest *req) glnx_tmpfile_clear (&req->tmpf); if (req->output_buf) g_string_free (req->output_buf, TRUE); + g_free (req->if_none_match); + g_free (req->out_etag); + g_clear_pointer (&req->req_headers, (GDestroyNotify)curl_slist_free_all); curl_easy_cleanup (req->easy); g_free (req); @@ -721,8 +909,29 @@ initiate_next_curl_request (FetcherRequest *req, curl_easy_setopt (req->easy, CURLOPT_USERAGENT, self->custom_user_agent ?: OSTREE_FETCHER_USERAGENT_STRING); - if (self->extra_headers) - curl_easy_setopt (req->easy, CURLOPT_HTTPHEADER, self->extra_headers); + + /* Set caching request headers */ + if (req->if_none_match != NULL) + { + g_autofree char *if_none_match = g_strconcat ("If-None-Match: ", req->if_none_match, NULL); + req->req_headers = curl_slist_append (req->req_headers, if_none_match); + } + + if (req->if_modified_since > 0) + { + g_autoptr(GDateTime) date_time = g_date_time_new_from_unix_utc (req->if_modified_since); + g_autofree char *mod_date = g_date_time_format (date_time, "If-Modified-Since: %a, %d %b %Y %H:%M:%S %Z"); + + req->req_headers = curl_slist_append (req->req_headers, mod_date); + } + + /* Append a copy of @extra_headers to @req_headers, as the former could change + * between requests or while a request is in flight */ + for (const struct curl_slist *l = self->extra_headers; l != NULL; l = l->next) + req->req_headers = curl_slist_append (req->req_headers, l->data); + + if (req->req_headers != NULL) + curl_easy_setopt (req->easy, CURLOPT_HTTPHEADER, req->req_headers); if (self->cookie_jar_path) { @@ -796,6 +1005,7 @@ initiate_next_curl_request (FetcherRequest *req, } curl_easy_setopt (req->easy, CURLOPT_WRITEFUNCTION, write_cb); + curl_easy_setopt (req->easy, CURLOPT_HEADERFUNCTION, response_header_cb); if (g_getenv ("OSTREE_DEBUG_HTTP")) curl_easy_setopt (req->easy, CURLOPT_VERBOSE, 1L); curl_easy_setopt (req->easy, CURLOPT_ERRORBUFFER, req->error); @@ -815,6 +1025,7 @@ initiate_next_curl_request (FetcherRequest *req, /* closure bindings -> task */ curl_easy_setopt (req->easy, CURLOPT_PRIVATE, task); curl_easy_setopt (req->easy, CURLOPT_WRITEDATA, task); + curl_easy_setopt (req->easy, CURLOPT_HEADERDATA, task); curl_easy_setopt (req->easy, CURLOPT_PROGRESSDATA, task); CURLMcode multi_rc = curl_multi_add_handle (self->multi, req->easy); @@ -826,6 +1037,8 @@ _ostree_fetcher_request_async (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, gboolean is_membuf, guint64 max_size, int priority, @@ -859,6 +1072,8 @@ _ostree_fetcher_request_async (OstreeFetcher *self, req->filename = g_strdup (filename); req->max_size = max_size; req->flags = flags; + req->if_none_match = g_strdup (if_none_match); + req->if_modified_since = if_modified_since; req->is_membuf = is_membuf; /* We'll allocate the tmpfile on demand, so we handle * file I/O errors just in the write func. @@ -882,13 +1097,16 @@ _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint64 max_size, int priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) { - _ostree_fetcher_request_async (self, mirrorlist, filename, flags, FALSE, + _ostree_fetcher_request_async (self, mirrorlist, filename, flags, + if_none_match, if_modified_since, FALSE, max_size, priority, cancellable, callback, user_data); } @@ -897,6 +1115,9 @@ gboolean _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self, GAsyncResult *result, GLnxTmpfile *out_tmpf, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GError **error) { g_return_val_if_fail (g_task_is_valid (result, self), FALSE); @@ -912,6 +1133,13 @@ _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self, *out_tmpf = req->tmpf; req->tmpf.initialized = FALSE; /* Transfer ownership */ + if (out_not_modified != NULL) + *out_not_modified = req->out_not_modified; + if (out_etag != NULL) + *out_etag = g_steal_pointer (&req->out_etag); + if (out_last_modified != NULL) + *out_last_modified = req->out_last_modified; + return TRUE; } @@ -920,13 +1148,16 @@ _ostree_fetcher_request_to_membuf (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint64 max_size, int priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) { - _ostree_fetcher_request_async (self, mirrorlist, filename, flags, TRUE, + _ostree_fetcher_request_async (self, mirrorlist, filename, flags, + if_none_match, if_modified_since, TRUE, max_size, priority, cancellable, callback, user_data); } @@ -935,6 +1166,9 @@ gboolean _ostree_fetcher_request_to_membuf_finish (OstreeFetcher *self, GAsyncResult *result, GBytes **out_buf, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GError **error) { GTask *task; @@ -955,6 +1189,13 @@ _ostree_fetcher_request_to_membuf_finish (OstreeFetcher *self, g_assert (out_buf); *out_buf = ret; + if (out_not_modified != NULL) + *out_not_modified = req->out_not_modified; + if (out_etag != NULL) + *out_etag = g_steal_pointer (&req->out_etag); + if (out_last_modified != NULL) + *out_last_modified = req->out_last_modified; + return TRUE; } diff --git a/src/libostree/ostree-fetcher-soup.c b/src/libostree/ostree-fetcher-soup.c index 970ac7a4b3..52fe5fc38d 100644 --- a/src/libostree/ostree-fetcher-soup.c +++ b/src/libostree/ostree-fetcher-soup.c @@ -90,9 +90,14 @@ typedef struct { gboolean is_membuf; OstreeFetcherRequestFlags flags; + char *if_none_match; /* request ETag */ + guint64 if_modified_since; /* seconds since the epoch */ GInputStream *request_body; GLnxTmpfile tmpf; GOutputStream *out_stream; + gboolean out_not_modified; /* TRUE if the server gave a HTTP 304 Not Modified response, which we don’t propagate as an error */ + char *out_etag; /* response ETag */ + guint64 out_last_modified; /* response Last-Modified, seconds since the epoch */ guint64 max_size; guint64 current_size; @@ -196,8 +201,10 @@ pending_uri_unref (OstreeFetcherPendingURI *pending) g_free (pending->filename); g_clear_object (&pending->request); g_clear_object (&pending->request_body); + g_free (pending->if_none_match); glnx_tmpfile_clear (&pending->tmpf); g_clear_object (&pending->out_stream); + g_free (pending->out_etag); g_free (pending); } @@ -431,6 +438,22 @@ create_pending_soup_request (OstreeFetcherPendingURI *pending, pending->request = soup_session_request_uri (pending->thread_closure->session, (SoupURI*)(uri ? uri : next_mirror), error); + + /* Add caching headers. */ + if (SOUP_IS_REQUEST_HTTP (pending->request) && pending->if_none_match != NULL) + { + glnx_unref_object SoupMessage *msg = soup_request_http_get_message ((SoupRequestHTTP*) pending->request); + soup_message_headers_append (msg->request_headers, "If-None-Match", pending->if_none_match); + } + + if (SOUP_IS_REQUEST_HTTP (pending->request) && pending->if_modified_since > 0) + { + glnx_unref_object SoupMessage *msg = soup_request_http_get_message ((SoupRequestHTTP*) pending->request); + + g_autoptr(GDateTime) date_time = g_date_time_new_from_unix_utc (pending->if_modified_since); + g_autofree char *mod_date = g_date_time_format (date_time, "%a, %d %b %Y %H:%M:%S %Z"); + soup_message_headers_append (msg->request_headers, "If-Modified-Since", mod_date); + } } static void @@ -1050,7 +1073,14 @@ on_request_sent (GObject *object, if (SOUP_IS_REQUEST_HTTP (object)) { msg = soup_request_http_get_message ((SoupRequestHTTP*) object); - if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) + if (msg->status_code == SOUP_STATUS_NOT_MODIFIED && + (pending->if_none_match != NULL || pending->if_modified_since > 0)) + { + /* Version on the server is unchanged from the version we have cached locally; + * report this as an out-argument, a zero-length response buffer, and no error */ + pending->out_not_modified = TRUE; + } + else if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) { /* is there another mirror we can try? */ if (pending->mirrorlist_idx + 1 < pending->mirrorlist->len) @@ -1124,6 +1154,21 @@ on_request_sent (GObject *object, } goto out; } + + /* Grab cache properties from the response */ + pending->out_etag = g_strdup (soup_message_headers_get_one (msg->response_headers, "ETag")); + pending->out_last_modified = 0; + + const char *last_modified_str = soup_message_headers_get_one (msg->response_headers, "Last-Modified"); + if (last_modified_str != NULL) + { + SoupDate *soup_date = soup_date_new_from_string (last_modified_str); + if (soup_date != NULL) + { + pending->out_last_modified = soup_date_to_time_t (soup_date); + soup_date_free (soup_date); + } + } } pending->state = OSTREE_FETCHER_STATE_DOWNLOADING; @@ -1154,6 +1199,8 @@ _ostree_fetcher_request_async (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, gboolean is_membuf, guint64 max_size, int priority, @@ -1175,6 +1222,8 @@ _ostree_fetcher_request_async (OstreeFetcher *self, pending->mirrorlist = g_ptr_array_ref (mirrorlist); pending->filename = g_strdup (filename); pending->flags = flags; + pending->if_none_match = g_strdup (if_none_match); + pending->if_modified_since = if_modified_since; pending->max_size = max_size; pending->is_membuf = is_membuf; @@ -1196,13 +1245,16 @@ _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint64 max_size, int priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) { - _ostree_fetcher_request_async (self, mirrorlist, filename, flags, FALSE, + _ostree_fetcher_request_async (self, mirrorlist, filename, flags, + if_none_match, if_modified_since, FALSE, max_size, priority, cancellable, callback, user_data); } @@ -1211,6 +1263,9 @@ gboolean _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self, GAsyncResult *result, GLnxTmpfile *out_tmpf, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GError **error) { GTask *task; @@ -1231,6 +1286,13 @@ _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self, *out_tmpf = pending->tmpf; pending->tmpf.initialized = FALSE; /* Transfer ownership */ + if (out_not_modified != NULL) + *out_not_modified = pending->out_not_modified; + if (out_etag != NULL) + *out_etag = g_steal_pointer (&pending->out_etag); + if (out_last_modified != NULL) + *out_last_modified = pending->out_last_modified; + return TRUE; } @@ -1239,13 +1301,16 @@ _ostree_fetcher_request_to_membuf (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint64 max_size, int priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) { - _ostree_fetcher_request_async (self, mirrorlist, filename, flags, TRUE, + _ostree_fetcher_request_async (self, mirrorlist, filename, flags, + if_none_match, if_modified_since, TRUE, max_size, priority, cancellable, callback, user_data); } @@ -1254,6 +1319,9 @@ gboolean _ostree_fetcher_request_to_membuf_finish (OstreeFetcher *self, GAsyncResult *result, GBytes **out_buf, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GError **error) { GTask *task; @@ -1274,6 +1342,13 @@ _ostree_fetcher_request_to_membuf_finish (OstreeFetcher *self, g_assert (out_buf); *out_buf = ret; + if (out_not_modified != NULL) + *out_not_modified = pending->out_not_modified; + if (out_etag != NULL) + *out_etag = g_steal_pointer (&pending->out_etag); + if (out_last_modified != NULL) + *out_last_modified = pending->out_last_modified; + return TRUE; } diff --git a/src/libostree/ostree-fetcher-util.c b/src/libostree/ostree-fetcher-util.c index 1c3f81042c..76f6bba14e 100644 --- a/src/libostree/ostree-fetcher-util.c +++ b/src/libostree/ostree-fetcher-util.c @@ -34,6 +34,9 @@ typedef struct { GBytes *result_buf; + gboolean result_not_modified; + char *result_etag; + guint64 result_last_modified; /* second since the epoch */ gboolean done; GError **error; } @@ -48,6 +51,8 @@ fetch_uri_sync_on_complete (GObject *object, (void)_ostree_fetcher_request_to_membuf_finish ((OstreeFetcher*)object, result, &data->result_buf, + &data->result_not_modified, + &data->result_etag, &data->result_last_modified, data->error); data->done = TRUE; } @@ -57,7 +62,12 @@ _ostree_fetcher_mirrored_request_to_membuf_once (OstreeFetcher *fe GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, GBytes **out_contents, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, guint64 max_size, GCancellable *cancellable, GError **error) @@ -79,7 +89,7 @@ _ostree_fetcher_mirrored_request_to_membuf_once (OstreeFetcher *fe data.error = error; _ostree_fetcher_request_to_membuf (fetcher, mirrorlist, filename, - flags, + flags, if_none_match, if_modified_since, max_size, OSTREE_FETCHER_DEFAULT_PRIORITY, cancellable, fetch_uri_sync_on_complete, &data); while (!data.done) @@ -94,6 +104,12 @@ _ostree_fetcher_mirrored_request_to_membuf_once (OstreeFetcher *fe g_clear_error (error); ret = TRUE; *out_contents = NULL; + if (out_not_modified != NULL) + *out_not_modified = FALSE; + if (out_etag != NULL) + *out_etag = NULL; + if (out_last_modified != NULL) + *out_last_modified = 0; } } goto out; @@ -101,10 +117,17 @@ _ostree_fetcher_mirrored_request_to_membuf_once (OstreeFetcher *fe ret = TRUE; *out_contents = g_steal_pointer (&data.result_buf); + if (out_not_modified != NULL) + *out_not_modified = data.result_not_modified; + if (out_etag != NULL) + *out_etag = g_steal_pointer (&data.result_etag); + if (out_last_modified != NULL) + *out_last_modified = data.result_last_modified; out: if (mainctx) g_main_context_pop_thread_default (mainctx); g_clear_pointer (&data.result_buf, (GDestroyNotify)g_bytes_unref); + g_clear_pointer (&data.result_etag, g_free); return ret; } @@ -113,8 +136,13 @@ _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint n_network_retries, GBytes **out_contents, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, guint64 max_size, GCancellable *cancellable, GError **error) @@ -127,7 +155,9 @@ _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher g_clear_error (&local_error); if (_ostree_fetcher_mirrored_request_to_membuf_once (fetcher, mirrorlist, filename, flags, - out_contents, max_size, + if_none_match, if_modified_since, + out_contents, out_not_modified, out_etag, + out_last_modified, max_size, cancellable, &local_error)) return TRUE; } @@ -143,8 +173,13 @@ gboolean _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, OstreeFetcherURI *uri, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint n_network_retries, GBytes **out_contents, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, guint64 max_size, GCancellable *cancellable, GError **error) @@ -152,7 +187,9 @@ _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, g_autoptr(GPtrArray) mirrorlist = g_ptr_array_new (); g_ptr_array_add (mirrorlist, uri); /* no transfer */ return _ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, NULL, flags, - n_network_retries, out_contents, max_size, + if_none_match, if_modified_since, + n_network_retries, out_contents, + out_not_modified, out_etag, out_last_modified, max_size, cancellable, error); } diff --git a/src/libostree/ostree-fetcher-util.h b/src/libostree/ostree-fetcher-util.h index 4693540276..2cbaf5fa9c 100644 --- a/src/libostree/ostree-fetcher-util.h +++ b/src/libostree/ostree-fetcher-util.h @@ -56,8 +56,13 @@ gboolean _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint n_network_retries, GBytes **out_contents, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, guint64 max_size, GCancellable *cancellable, GError **error); @@ -65,8 +70,13 @@ gboolean _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, gboolean _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, OstreeFetcherURI *uri, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint n_network_retries, GBytes **out_contents, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, guint64 max_size, GCancellable *cancellable, GError **error); diff --git a/src/libostree/ostree-fetcher.h b/src/libostree/ostree-fetcher.h index 32f3ea1be6..2065ee54d3 100644 --- a/src/libostree/ostree-fetcher.h +++ b/src/libostree/ostree-fetcher.h @@ -123,6 +123,8 @@ void _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint64 max_size, int priority, GCancellable *cancellable, @@ -132,12 +134,17 @@ void _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self, gboolean _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self, GAsyncResult *result, GLnxTmpfile *out_tmpf, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GError **error); void _ostree_fetcher_request_to_membuf (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint64 max_size, int priority, GCancellable *cancellable, @@ -147,6 +154,9 @@ void _ostree_fetcher_request_to_membuf (OstreeFetcher *self, gboolean _ostree_fetcher_request_to_membuf_finish (OstreeFetcher *self, GAsyncResult *result, GBytes **out_buf, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GError **error); diff --git a/src/libostree/ostree-metalink.c b/src/libostree/ostree-metalink.c index cb8a50e3ac..6381b31e5e 100644 --- a/src/libostree/ostree-metalink.c +++ b/src/libostree/ostree-metalink.c @@ -436,8 +436,10 @@ try_one_url (OstreeMetalinkRequest *self, if (!_ostree_fetcher_request_uri_to_membuf (self->metalink->fetcher, uri, 0, + NULL, 0, self->metalink->n_network_retries, &bytes, + NULL, NULL, NULL, self->metalink->max_size, self->cancellable, error)) @@ -618,8 +620,9 @@ _ostree_metalink_request_sync (OstreeMetalink *self, request.parser = g_markup_parse_context_new (&metalink_parser, G_MARKUP_PREFIX_ERROR_POSITION, &request, NULL); if (!_ostree_fetcher_request_uri_to_membuf (self->fetcher, self->uri, 0, + NULL, 0, self->n_network_retries, - &contents, self->max_size, + &contents, NULL, NULL, NULL, self->max_size, cancellable, error)) goto out; diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index f16ccec508..a0d983bb6e 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -458,8 +458,9 @@ fetch_mirrored_uri_contents_utf8_sync (OstreeFetcher *fetcher, g_autoptr(GBytes) bytes = NULL; if (!_ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, filename, OSTREE_FETCHER_REQUEST_NUL_TERMINATION, + NULL, 0, n_network_retries, - &bytes, + &bytes, NULL, NULL, NULL, OSTREE_MAX_METADATA_SIZE, cancellable, error)) return FALSE; @@ -965,7 +966,7 @@ content_fetch_on_complete (GObject *object, OstreeObjectType objtype; gboolean free_fetch_data = TRUE; - if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmpf, error)) + if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmpf, NULL, NULL, NULL, error)) goto out; ostree_object_name_deserialize (fetch_data->object, &checksum, &objtype); @@ -1105,7 +1106,7 @@ meta_fetch_on_complete (GObject *object, g_debug ("fetch of %s%s complete", checksum_obj, fetch_data->is_detached_meta ? " (detached)" : ""); - if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmpf, error)) + if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmpf, NULL, NULL, NULL, error)) { if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) { @@ -1282,7 +1283,7 @@ static_deltapart_fetch_on_complete (GObject *object, g_debug ("fetch static delta part %s complete", fetch_data->expected_checksum); - if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmpf, error)) + if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmpf, NULL, NULL, NULL, error)) goto out; /* Transfer ownership of the fd */ @@ -1994,7 +1995,7 @@ start_fetch (OtPullData *pull_data, if (!is_meta && pull_data->trusted_http_direct) flags |= OSTREE_FETCHER_REQUEST_LINKABLE; _ostree_fetcher_request_to_tmpfile (pull_data->fetcher, mirrorlist, - obj_subpath, flags, expected_max_size, + obj_subpath, flags, NULL, 0, expected_max_size, is_meta ? OSTREE_REPO_PULL_METADATA_PRIORITY : OSTREE_REPO_PULL_CONTENT_PRIORITY, pull_data->cancellable, @@ -2119,7 +2120,7 @@ start_fetch_deltapart (OtPullData *pull_data, g_assert_cmpint (pull_data->n_outstanding_deltapart_fetches, <=, _OSTREE_MAX_OUTSTANDING_DELTAPART_REQUESTS); _ostree_fetcher_request_to_tmpfile (pull_data->fetcher, pull_data->content_mirrorlist, - deltapart_path, 0, fetch->size, + deltapart_path, 0, NULL, 0, fetch->size, OSTREE_FETCHER_DEFAULT_PRIORITY, pull_data->cancellable, static_deltapart_fetch_on_complete, @@ -2494,6 +2495,7 @@ on_superblock_fetched (GObject *src, if (!_ostree_fetcher_request_to_membuf_finish ((OstreeFetcher*)src, res, &delta_superblock_data, + NULL, NULL, NULL, error)) { if (!g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) @@ -2570,6 +2572,7 @@ start_fetch_delta_superblock (OtPullData *pull_data, _ostree_fetcher_request_to_membuf (pull_data->fetcher, pull_data->content_mirrorlist, delta_name, OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, OSTREE_MAX_METADATA_SIZE, 0, pull_data->cancellable, on_superblock_fetched, @@ -3000,8 +3003,10 @@ _ostree_preload_metadata_file (OstreeRepo *self, { return _ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, filename, OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, n_network_retries, - out_bytes, OSTREE_MAX_METADATA_SIZE, + out_bytes, NULL, NULL, NULL, + OSTREE_MAX_METADATA_SIZE, cancellable, error); } } @@ -3858,8 +3863,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, "summary.sig", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, pull_data->n_network_retries, &bytes_sig, + NULL, NULL, NULL, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; @@ -3887,8 +3894,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, pull_data->n_network_retries, &bytes_summary, + NULL, NULL, NULL, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; @@ -3949,8 +3958,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, pull_data->meta_mirrorlist, "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, pull_data->n_network_retries, &bytes_summary, + NULL, NULL, NULL, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; @@ -4012,8 +4023,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, pull_data->meta_mirrorlist, "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, pull_data->n_network_retries, &bytes_summary, + NULL, NULL, NULL, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; @@ -5499,8 +5512,10 @@ find_remotes_cb (GObject *obj, mirrorlist, commit_filename, OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, data->n_network_retries, &commit_bytes, + NULL, NULL, NULL, 0, /* no maximum size */ cancellable, &error)) From 0e3add8d4057b920422bba6405de208cdead00ec Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 29 Sep 2020 10:51:26 +0100 Subject: [PATCH 2/5] lib/pull: Hook up HTTP caching headers for summary and summary.sig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As `summary` and `summary.sig` aren’t immutable, HTTP requests to download them can be optimised by sending the `If-None-Match` and `If-Modified-Since` headers to avoid unnecessarily re-downloading them if they haven’t changed since last being checked. Hook them up to the new support for that in the fetcher. The `ETag` and `Last-Modified` for each file in the cache are stored as the `user.etag` xattr and the mtime, respectively. For flatpak, for example, this affects the cached files in `~/.local/share/flatpak/repo/tmp/cache/summaries`. If xattrs aren’t supported, or if the server doesn’t support the caching headers, the pull behaviour is unchanged from before. Signed-off-by: Philip Withnall --- src/libostree/ostree-repo-pull-private.h | 4 + src/libostree/ostree-repo-pull.c | 270 +++++++++++++++++++++-- 2 files changed, 256 insertions(+), 18 deletions(-) diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h index 689118be69..a7ea85cd7e 100644 --- a/src/libostree/ostree-repo-pull-private.h +++ b/src/libostree/ostree-repo-pull-private.h @@ -72,7 +72,11 @@ typedef struct { gboolean has_tombstone_commits; GBytes *summary_data; + char *summary_etag; + guint64 summary_last_modified; /* seconds since the epoch */ GBytes *summary_data_sig; + char *summary_sig_etag; + guint64 summary_sig_last_modified; /* seconds since the epoch */ GVariant *summary; GHashTable *summary_deltas_checksums; GHashTable *ref_original_commits; /* Maps checksum to commit, used by timestamp checks */ diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index a0d983bb6e..ae8ad17f5f 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -46,6 +46,7 @@ #include #include +#include #ifdef HAVE_LIBSYSTEMD #include #endif @@ -2702,6 +2703,64 @@ _ostree_repo_verify_summary (OstreeRepo *self, return TRUE; } +static void +_ostree_repo_load_cache_summary_properties (OstreeRepo *self, + const char *filename, + const char *extension, + char **out_etag, + guint64 *out_last_modified) +{ + const char *file = glnx_strjoina (_OSTREE_SUMMARY_CACHE_DIR, "/", filename, extension); + glnx_autofd int fd = -1; + + if (self->cache_dir_fd == -1) + return; + + if (!glnx_openat_rdonly (self->cache_dir_fd, file, TRUE, &fd, NULL)) + return; + + if (out_etag != NULL) + { + g_autoptr(GBytes) etag_bytes = glnx_fgetxattr_bytes (fd, "user.etag", NULL); + if (etag_bytes != NULL) + { + const guint8 *buf; + gsize buf_len; + + buf = g_bytes_get_data (etag_bytes, &buf_len); + + /* Loosely validate against https://tools.ietf.org/html/rfc7232#section-2.3 + * by checking there are no embedded nuls. */ + for (gsize i = 0; i < buf_len; i++) + { + if (buf[i] == 0) + { + buf_len = 0; + break; + } + } + + /* Nul-terminate and return */ + if (buf_len > 0) + *out_etag = g_strndup ((const char *) buf, buf_len); + else + *out_etag = NULL; + } + else + *out_etag = NULL; + } + + if (out_last_modified != NULL) + { + struct stat statbuf; + + if (glnx_fstatat (fd, "", &statbuf, AT_EMPTY_PATH, NULL)) + *out_last_modified = statbuf.st_mtim.tv_sec; + else + *out_last_modified = 0; + } +} + static gboolean _ostree_repo_load_cache_summary_file (OstreeRepo *self, const char *filename, @@ -2777,11 +2836,38 @@ _ostree_repo_load_cache_summary_if_same_sig (OstreeRepo *self, return TRUE; } +static void +store_file_cache_properties (int dir_fd, + const char *filename, + const char *etag, + guint64 last_modified) +{ + glnx_autofd int fd = -1; + struct timespec time_vals[] = + { + { .tv_sec = last_modified, .tv_nsec = UTIME_OMIT }, /* access, leave unchanged */ + { .tv_sec = last_modified, .tv_nsec = 0 }, /* modification */ + }; + + if (!glnx_openat_rdonly (dir_fd, filename, TRUE, &fd, NULL)) + return; + + if (etag != NULL) + TEMP_FAILURE_RETRY (fsetxattr (fd, "user.etag", etag, strlen (etag), 0)); + else + TEMP_FAILURE_RETRY (fremovexattr (fd, "user.etag")); + + if (last_modified > 0) + TEMP_FAILURE_RETRY (futimens (fd, time_vals)); +} + static gboolean _ostree_repo_save_cache_summary_file (OstreeRepo *self, const char *filename, const char *extension, GBytes *data, + const char *etag, + guint64 last_modified, GCancellable *cancellable, GError **error) { @@ -2802,6 +2888,9 @@ _ostree_repo_save_cache_summary_file (OstreeRepo *self, cancellable, error)) return FALSE; + /* Store the caching properties. This is non-fatal on failure. */ + store_file_cache_properties (self->cache_dir_fd, file, etag, last_modified); + return TRUE; } @@ -2810,16 +2899,24 @@ static gboolean _ostree_repo_cache_summary (OstreeRepo *self, const char *remote, GBytes *summary, + const char *summary_etag, + guint64 summary_last_modified, GBytes *summary_sig, + const char *summary_sig_etag, + guint64 summary_sig_last_modified, GCancellable *cancellable, GError **error) { if (!_ostree_repo_save_cache_summary_file (self, remote, NULL, - summary, cancellable, error)) + summary, + summary_etag, summary_last_modified, + cancellable, error)) return FALSE; if (!_ostree_repo_save_cache_summary_file (self, remote, ".sig", - summary_sig, cancellable, error)) + summary_sig, + summary_sig_etag, summary_sig_last_modified, + cancellable, error)) return FALSE; return TRUE; @@ -2967,8 +3064,13 @@ _ostree_preload_metadata_file (OstreeRepo *self, GPtrArray *mirrorlist, const char *filename, gboolean is_metalink, + const char *if_none_match, + guint64 if_modified_since, guint n_network_retries, GBytes **out_bytes, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GCancellable *cancellable, GError **error) { @@ -3003,9 +3105,9 @@ _ostree_preload_metadata_file (OstreeRepo *self, { return _ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, filename, OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - NULL, 0, + if_none_match, if_modified_since, n_network_retries, - out_bytes, NULL, NULL, NULL, + out_bytes, out_not_modified, out_etag, out_last_modified, OSTREE_MAX_METADATA_SIZE, cancellable, error); } @@ -3365,6 +3467,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, { gboolean ret = FALSE; g_autoptr(GBytes) bytes_summary = NULL; + gboolean summary_not_modified = FALSE; + g_autofree char *summary_etag = NULL; + guint64 summary_last_modified = 0; g_autofree char *metalink_url_str = NULL; g_autoptr(GHashTable) requested_refs_to_fetch = NULL; /* (element-type OstreeCollectionRef utf8) */ g_autoptr(GHashTable) commits_to_fetch = NULL; @@ -3832,6 +3937,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, { g_autoptr(GBytes) bytes_sig = NULL; + gboolean summary_sig_not_modified = FALSE; + g_autofree char *summary_sig_etag = NULL; + guint64 summary_sig_last_modified = 0; gsize n; g_autoptr(GVariant) refs = NULL; g_autoptr(GVariant) deltas = NULL; @@ -3860,16 +3968,47 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!bytes_sig) { + g_autofree char *summary_sig_if_none_match = NULL; + guint64 summary_sig_if_modified_since = 0; + + /* Load the summary.sig from the network, but send its ETag and + * Last-Modified from the on-disk cache (if it exists) to reduce the + * download size if nothing’s changed. */ + _ostree_repo_load_cache_summary_properties (self, remote_name_or_baseurl, ".sig", + &summary_sig_if_none_match, &summary_sig_if_modified_since); + + g_clear_pointer (&summary_sig_etag, g_free); + summary_sig_last_modified = 0; if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, "summary.sig", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - NULL, 0, + summary_sig_if_none_match, summary_sig_if_modified_since, pull_data->n_network_retries, &bytes_sig, - NULL, NULL, NULL, + &summary_sig_not_modified, &summary_sig_etag, &summary_sig_last_modified, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; + + /* The server returned HTTP status 304 Not Modified, so we’re clear to + * load summary.sig from the cache. Also load summary, since + * `_ostree_repo_load_cache_summary_if_same_sig()` would just do that anyway. */ + if (summary_sig_not_modified) + { + g_clear_pointer (&bytes_sig, g_bytes_unref); + g_clear_pointer (&bytes_summary, g_bytes_unref); + if (!_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, ".sig", + &bytes_sig, + cancellable, error)) + goto out; + + if (!bytes_summary && + !pull_data->remote_repo_local && + !_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, NULL, + &bytes_summary, + cancellable, error)) + goto out; + } } if (bytes_sig && @@ -3891,16 +4030,35 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!pull_data->summary && !bytes_summary) { + g_autofree char *summary_if_none_match = NULL; + guint64 summary_if_modified_since = 0; + + _ostree_repo_load_cache_summary_properties (self, remote_name_or_baseurl, NULL, + &summary_if_none_match, &summary_if_modified_since); + + g_clear_pointer (&summary_etag, g_free); + summary_last_modified = 0; if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - NULL, 0, + summary_if_none_match, summary_if_modified_since, pull_data->n_network_retries, &bytes_summary, - NULL, NULL, NULL, + &summary_not_modified, &summary_etag, &summary_last_modified, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; + + /* The server returned HTTP status 304 Not Modified, so we’re clear to + * load summary from the cache. */ + if (summary_not_modified) + { + g_clear_pointer (&bytes_summary, g_bytes_unref); + if (!_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, NULL, + &bytes_summary, + cancellable, error)) + goto out; + } } #ifndef OSTREE_DISABLE_GPGME @@ -3939,7 +4097,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, { if (summary_from_cache) { - /* The cached summary doesn't match, fetch a new one and verify again */ + /* The cached summary doesn't match, fetch a new one and verify again. + * Don’t set the cache headers in the HTTP request, to force a + * full download. */ if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_INVALID_CACHE) > 0) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, @@ -3954,14 +4114,16 @@ ostree_repo_pull_with_options (OstreeRepo *self, summary_from_cache = FALSE; g_clear_pointer (&bytes_summary, (GDestroyNotify)g_bytes_unref); + g_clear_pointer (&summary_etag, g_free); + summary_last_modified = 0; if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - NULL, 0, + NULL, 0, /* no cache headers */ pull_data->n_network_retries, &bytes_summary, - NULL, NULL, NULL, + &summary_not_modified, &summary_etag, &summary_last_modified, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; @@ -4004,7 +4166,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, { if (summary_from_cache) { - /* The cached summary doesn't match, fetch a new one and verify again */ + /* The cached summary doesn't match, fetch a new one and verify again. + * Don’t set the cache headers in the HTTP request, to force a + * full download. */ if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_INVALID_CACHE) > 0) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, @@ -4019,14 +4183,16 @@ ostree_repo_pull_with_options (OstreeRepo *self, summary_from_cache = FALSE; g_clear_pointer (&bytes_summary, (GDestroyNotify)g_bytes_unref); + g_clear_pointer (&summary_etag, g_free); + summary_last_modified = 0; if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - NULL, 0, + NULL, 0, /* no cache headers */ pull_data->n_network_retries, &bytes_summary, - NULL, NULL, NULL, + &summary_not_modified, &summary_etag, &summary_last_modified, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; @@ -4046,6 +4212,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (bytes_summary) { pull_data->summary_data = g_bytes_ref (bytes_summary); + pull_data->summary_etag = g_strdup (summary_etag); + pull_data->summary_last_modified = summary_last_modified; pull_data->summary = g_variant_new_from_bytes (OSTREE_SUMMARY_GVARIANT_FORMAT, bytes_summary, FALSE); if (!g_variant_is_normal_form (pull_data->summary)) @@ -4063,7 +4231,11 @@ ostree_repo_pull_with_options (OstreeRepo *self, } if (bytes_sig) - pull_data->summary_data_sig = g_bytes_ref (bytes_sig); + { + pull_data->summary_data_sig = g_bytes_ref (bytes_sig); + pull_data->summary_sig_etag = g_strdup (summary_sig_etag); + pull_data->summary_sig_last_modified = summary_sig_last_modified; + } } if (!summary_from_cache && bytes_summary && bytes_sig) @@ -4072,7 +4244,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, !_ostree_repo_cache_summary (self, remote_name_or_baseurl, bytes_summary, + summary_etag, summary_last_modified, bytes_sig, + summary_sig_etag, summary_sig_last_modified, cancellable, error)) goto out; @@ -4507,6 +4681,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, cancellable, error)) goto out; + store_file_cache_properties (pull_data->repo->repo_dir_fd, "summary", + pull_data->summary_etag, pull_data->summary_last_modified); + if (pull_data->summary_data_sig) { buf = g_bytes_get_data (pull_data->summary_data_sig, &len); @@ -4514,6 +4691,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, buf, len, replaceflag, cancellable, error)) goto out; + + store_file_cache_properties (pull_data->repo->repo_dir_fd, "summary.sig", + pull_data->summary_sig_etag, pull_data->summary_sig_last_modified); } } @@ -4700,7 +4880,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_clear_pointer (&pull_data->meta_mirrorlist, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&pull_data->content_mirrorlist, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&pull_data->summary_data, (GDestroyNotify) g_bytes_unref); + g_clear_pointer (&pull_data->summary_etag, g_free); g_clear_pointer (&pull_data->summary_data_sig, (GDestroyNotify) g_bytes_unref); + g_clear_pointer (&pull_data->summary_sig_etag, g_free); g_clear_pointer (&pull_data->summary, (GDestroyNotify) g_variant_unref); g_clear_pointer (&pull_data->static_delta_superblocks, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&pull_data->commit_to_depth, (GDestroyNotify) g_hash_table_unref); @@ -6129,6 +6311,16 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, g_autoptr(GPtrArray) mirrorlist = NULL; const char *append_user_agent = NULL; guint n_network_retries = DEFAULT_N_NETWORK_RETRIES; + gboolean summary_sig_not_modified = FALSE; + g_autofree char *summary_sig_if_none_match = NULL; + g_autofree char *summary_sig_etag = NULL; + gboolean summary_not_modified = FALSE; + g_autofree char *summary_if_none_match = NULL; + g_autofree char *summary_etag = NULL; + guint64 summary_sig_if_modified_since = 0; + guint64 summary_sig_last_modified = 0; + guint64 summary_if_modified_since = 0; + guint64 summary_last_modified = 0; g_return_val_if_fail (OSTREE_REPO (self), FALSE); g_return_val_if_fail (name != NULL, FALSE); @@ -6174,22 +6366,48 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, &mirrorlist, cancellable, error)) return FALSE; - /* FIXME: Send the ETag from the cache with the request for summary.sig to + /* Send the ETag from the cache with the request for summary.sig to * avoid downloading summary.sig unnecessarily. This won’t normally provide - * any benefits (but won’t do any harm) since summary.sig is typically 500B - * in size. But if a repository has multiple keys, the signature file will + * much benefit since summary.sig is typically 590B in size (vs a 0B HTTP 304 + * response). But if a repository has multiple keys, the signature file will * grow and this optimisation may be useful. */ + _ostree_repo_load_cache_summary_properties (self, name, ".sig", + &summary_sig_if_none_match, &summary_sig_if_modified_since); + _ostree_repo_load_cache_summary_properties (self, name, NULL, + &summary_if_none_match, &summary_if_modified_since); + if (!_ostree_preload_metadata_file (self, fetcher, mirrorlist, "summary.sig", metalink_url_string ? TRUE : FALSE, + summary_sig_if_none_match, summary_sig_if_modified_since, n_network_retries, &signatures, + &summary_sig_not_modified, &summary_sig_etag, &summary_sig_last_modified, cancellable, error)) return FALSE; + /* The server returned HTTP status 304 Not Modified, so we’re clear to + * load summary.sig from the cache. Also load summary, since + * `_ostree_repo_load_cache_summary_if_same_sig()` would just do that anyway. */ + if (summary_sig_not_modified) + { + g_clear_pointer (&signatures, g_bytes_unref); + g_clear_pointer (&summary, g_bytes_unref); + if (!_ostree_repo_load_cache_summary_file (self, name, ".sig", + &signatures, + cancellable, error)) + return FALSE; + + if (!summary && + !_ostree_repo_load_cache_summary_file (self, name, NULL, + &summary, + cancellable, error)) + return FALSE; + } + if (signatures) { if (!_ostree_repo_load_cache_summary_if_same_sig (self, @@ -6210,11 +6428,25 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, mirrorlist, "summary", metalink_url_string ? TRUE : FALSE, + summary_if_none_match, summary_if_modified_since, n_network_retries, &summary, + &summary_not_modified, &summary_etag, &summary_last_modified, cancellable, error)) return FALSE; + + /* The server returned HTTP status 304 Not Modified, so we’re clear to + * load summary.sig from the cache. Also load summary, since + * `_ostree_repo_load_cache_summary_if_same_sig()` would just do that anyway. */ + if (summary_not_modified) + { + g_clear_pointer (&summary, g_bytes_unref); + if (!_ostree_repo_load_cache_summary_file (self, name, NULL, + &summary, + cancellable, error)) + return FALSE; + } } if (!_ostree_repo_verify_summary (self, name, @@ -6230,7 +6462,9 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, if (!_ostree_repo_cache_summary (self, name, summary, + summary_etag, summary_last_modified, signatures, + summary_sig_etag, summary_sig_last_modified, cancellable, &temp_error)) { From a522bf76283eeb88fcab99a5ee9e54aa0e13f041 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 9 Oct 2020 18:34:55 +0100 Subject: [PATCH 3/5] tests: Add simple test for summary file caching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This test would have actually passed before the summary file caching changes (in the previous few commits) were added, as the `summary.sig` essentially acted as the ETag for the summary file, and itself wasn’t updated on disk if it didn’t change when querying the server. Actually testing that the HTTP caching headers are working to reduce HTTP traffic would require test hooks into the pull code or the trivial-httpd server, neither of which I have the time to add at the moment. Signed-off-by: Philip Withnall --- Makefile-tests.am | 1 + tests/test-pull-summary-caching.sh | 66 ++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100755 tests/test-pull-summary-caching.sh diff --git a/Makefile-tests.am b/Makefile-tests.am index 3fbc94bfcf..570f83890d 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -82,6 +82,7 @@ _installed_or_uninstalled_test_scripts = \ tests/test-pull-mirror-summary.sh \ tests/test-pull-large-metadata.sh \ tests/test-pull-metalink.sh \ + tests/test-pull-summary-caching.sh \ tests/test-pull-summary-sigs.sh \ tests/test-pull-resume.sh \ tests/test-pull-basicauth.sh \ diff --git a/tests/test-pull-summary-caching.sh b/tests/test-pull-summary-caching.sh new file mode 100755 index 0000000000..9671199a11 --- /dev/null +++ b/tests/test-pull-summary-caching.sh @@ -0,0 +1,66 @@ +#!/bin/bash +# +# Copyright © 2020 Endless OS Foundation LLC +# +# SPDX-License-Identifier: LGPL-2.0+ +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the +# Free Software Foundation, Inc., 59 Temple Place - Suite 330, +# Boston, MA 02111-1307, USA. +# +# Authors: +# - Philip Withnall + +set -euo pipefail + +. $(dirname $0)/libtest.sh + +if ! has_gpgme; then + echo "1..0 #SKIP no gpg support compiled in" + exit 0 +fi + +COMMIT_SIGN="--gpg-homedir=${TEST_GPG_KEYHOME} --gpg-sign=${TEST_GPG_KEYID_1}" + +echo "1..1" + +setup_fake_remote_repo2 "archive" "${COMMIT_SIGN}" + +# Create a few branches and update the summary file (and sign it) +mkdir ${test_tmpdir}/ostree-srv/other-files +cd ${test_tmpdir}/ostree-srv/other-files +echo 'hello world another object' > hello-world +${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit ${COMMIT_SIGN} -b other -s "A commit" -m "Another Commit body" + +mkdir ${test_tmpdir}/ostree-srv/yet-other-files +cd ${test_tmpdir}/ostree-srv/yet-other-files +echo 'hello world yet another object' > yet-another-hello-world +${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit ${COMMIT_SIGN} -b yet-another -s "A commit" -m "Another Commit body" + +${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo summary -u ${COMMIT_SIGN} + +# Test that pulling twice in a row doesn’t re-download the summary file or its signature +cd ${test_tmpdir} +rm -rf repo +ostree_repo_init repo --mode=archive +${OSTREE} --repo=repo remote add --set=gpg-verify-summary=true origin $(cat httpd-address)/ostree/gnomerepo +${OSTREE} --repo=repo pull origin other +assert_has_file repo/tmp/cache/summaries/origin +assert_has_file repo/tmp/cache/summaries/origin.sig +summary_inode="$(stat -c '%i' repo/tmp/cache/summaries/origin)" +summary_sig_inode="$(stat -c '%i' repo/tmp/cache/summaries/origin.sig)" +${OSTREE} --repo=repo pull origin other +assert_streq "$(stat -c '%i' repo/tmp/cache/summaries/origin)" "${summary_inode}" +assert_streq "$(stat -c '%i' repo/tmp/cache/summaries/origin.sig)" "${summary_sig_inode}" +echo "ok pull caches the summary files" From 92215025aa0eb65f575700d0f3c286d594397e4f Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 9 Oct 2020 18:46:06 +0100 Subject: [PATCH 4/5] ostree/trivial-httpd: Add Last-Modified/ETag support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is basic support for the Last-Modified/ETag/If-Modified-Since/If-None-Match headers. It’s not high performance, and doesn’t support all of the related caching features (like the If-Match header, etc.). Signed-off-by: Philip Withnall --- src/ostree/ostree-trivial-httpd.c | 98 +++++++++++++++++++++++++------ 1 file changed, 80 insertions(+), 18 deletions(-) diff --git a/src/ostree/ostree-trivial-httpd.c b/src/ostree/ostree-trivial-httpd.c index a38abbea61..d8bc055681 100644 --- a/src/ostree/ostree-trivial-httpd.c +++ b/src/ostree/ostree-trivial-httpd.c @@ -206,6 +206,15 @@ close_socket (SoupMessage *msg, gpointer user_data) #endif } +/* Returns the ETag including the surrounding quotes */ +static gchar * +calculate_etag (GMappedFile *mapping) +{ + g_autoptr(GBytes) bytes = g_mapped_file_get_bytes (mapping); + g_autofree gchar *checksum = g_compute_checksum_for_bytes (G_CHECKSUM_SHA256, bytes); + return g_strconcat ("\"", checksum, "\"", NULL); +} + static void do_get (OtTrivialHttpd *self, SoupServer *server, @@ -367,31 +376,41 @@ do_get (OtTrivialHttpd *self, soup_message_set_status (msg, SOUP_STATUS_FORBIDDEN); goto out; } + + glnx_autofd int fd = openat (self->root_dfd, path, O_RDONLY | O_CLOEXEC); + if (fd < 0) + { + soup_message_set_status (msg, SOUP_STATUS_INTERNAL_SERVER_ERROR); + goto out; + } + + g_autoptr(GMappedFile) mapping = g_mapped_file_new_from_fd (fd, FALSE, NULL); + if (!mapping) + { + soup_message_set_status (msg, SOUP_STATUS_INTERNAL_SERVER_ERROR); + goto out; + } + (void) close (fd); fd = -1; + + /* Send caching headers */ + g_autoptr(GDateTime) last_modified = g_date_time_new_from_unix_utc (stbuf.st_mtim.tv_sec); + if (last_modified != NULL) + { + g_autofree gchar *formatted = g_date_time_format (last_modified, "%a, %d %b %Y %H:%M:%S GMT"); + soup_message_headers_append (msg->response_headers, "Last-Modified", formatted); + } + + g_autofree gchar *etag = calculate_etag (mapping); + if (etag != NULL) + soup_message_headers_append (msg->response_headers, "ETag", etag); if (msg->method == SOUP_METHOD_GET) { - glnx_autofd int fd = -1; - g_autoptr(GMappedFile) mapping = NULL; gsize buffer_length, file_size; SoupRange *ranges; int ranges_length; gboolean have_ranges; - fd = openat (self->root_dfd, path, O_RDONLY | O_CLOEXEC); - if (fd < 0) - { - soup_message_set_status (msg, SOUP_STATUS_INTERNAL_SERVER_ERROR); - goto out; - } - - mapping = g_mapped_file_new_from_fd (fd, FALSE, NULL); - if (!mapping) - { - soup_message_set_status (msg, SOUP_STATUS_INTERNAL_SERVER_ERROR); - goto out; - } - (void) close (fd); fd = -1; - file_size = g_mapped_file_get_length (mapping); have_ranges = soup_message_headers_get_ranges(msg->request_headers, file_size, &ranges, &ranges_length); if (opt_force_ranges && !have_ranges && g_strrstr (path, "/objects") != NULL) @@ -447,7 +466,50 @@ do_get (OtTrivialHttpd *self, soup_message_headers_append (msg->response_headers, "Content-Length", length); } - soup_message_set_status (msg, SOUP_STATUS_OK); + + /* Check client’s caching headers. */ + const gchar *if_modified_since = soup_message_headers_get_one (msg->request_headers, + "If-Modified-Since"); + const gchar *if_none_match = soup_message_headers_get_one (msg->request_headers, + "If-None-Match"); + + if (if_none_match != NULL && etag != NULL) + { + if (g_strcmp0 (etag, if_none_match) == 0) + { + soup_message_set_status (msg, SOUP_STATUS_NOT_MODIFIED); + soup_message_body_truncate (msg->response_body); + } + else + { + soup_message_set_status (msg, SOUP_STATUS_OK); + } + } + else if (if_modified_since != NULL && last_modified != NULL) + { + SoupDate *if_modified_since_sd = soup_date_new_from_string (if_modified_since); + g_autoptr(GDateTime) if_modified_since_dt = NULL; + + if (if_modified_since_sd != NULL) + if_modified_since_dt = g_date_time_new_from_unix_utc (soup_date_to_time_t (if_modified_since_sd)); + + if (if_modified_since_dt != NULL && + g_date_time_compare (last_modified, if_modified_since_dt) <= 0) + { + soup_message_set_status (msg, SOUP_STATUS_NOT_MODIFIED); + soup_message_body_truncate (msg->response_body); + } + else + { + soup_message_set_status (msg, SOUP_STATUS_OK); + } + + g_clear_pointer (&if_modified_since_sd, soup_date_free); + } + else + { + soup_message_set_status (msg, SOUP_STATUS_OK); + } } out: { From 0974a7faf174988c93832e0b4693a11e3a42821e Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 16 Oct 2020 17:05:54 +0100 Subject: [PATCH 5/5] tests: Split RFC 2616 date parsing code out and add tests This makes it testable, and increases its test coverage too 100% of lines, as measured by `make coverage`. Signed-off-by: Philip Withnall --- Makefile-libostree.am | 2 + Makefile-tests.am | 9 +- src/libostree/ostree-date-utils-private.h | 38 +++++ src/libostree/ostree-date-utils.c | 166 ++++++++++++++++++++++ src/libostree/ostree-fetcher-curl.c | 138 +----------------- tests/.gitignore | 1 + tests/test-rfc2616-dates.c | 123 ++++++++++++++++ 7 files changed, 340 insertions(+), 137 deletions(-) create mode 100644 src/libostree/ostree-date-utils-private.h create mode 100644 src/libostree/ostree-date-utils.c create mode 100644 tests/test-rfc2616-dates.c diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 96b9249b97..eeb0b6c60f 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -68,6 +68,8 @@ libostree_1_la_SOURCES = \ src/libostree/ostree-cmdprivate.c \ src/libostree/ostree-core-private.h \ src/libostree/ostree-core.c \ + src/libostree/ostree-date-utils.c \ + src/libostree/ostree-date-utils-private.h \ src/libostree/ostree-dummy-enumtypes.c \ src/libostree/ostree-checksum-input-stream.c \ src/libostree/ostree-checksum-input-stream.h \ diff --git a/Makefile-tests.am b/Makefile-tests.am index 570f83890d..257b4a5d87 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -273,7 +273,8 @@ endif _installed_or_uninstalled_test_programs = tests/test-varint tests/test-ot-unix-utils tests/test-bsdiff tests/test-mutable-tree \ tests/test-keyfile-utils tests/test-ot-opt-utils tests/test-ot-tool-util \ tests/test-checksum tests/test-lzma tests/test-rollsum \ - tests/test-basic-c tests/test-sysroot-c tests/test-pull-c tests/test-repo tests/test-include-ostree-h tests/test-kargs + tests/test-basic-c tests/test-sysroot-c tests/test-pull-c tests/test-repo tests/test-include-ostree-h tests/test-kargs \ + tests/test-rfc2616-dates if USE_GPGME _installed_or_uninstalled_test_programs += \ @@ -390,6 +391,12 @@ tests_test_lzma_SOURCES = src/libostree/ostree-lzma-common.c src/libostree/ostre tests_test_lzma_CFLAGS = $(TESTS_CFLAGS) $(OT_DEP_LZMA_CFLAGS) tests_test_lzma_LDADD = $(TESTS_LDADD) $(OT_DEP_LZMA_LIBS) +tests_test_rfc2616_dates_SOURCES = \ + src/libostree/ostree-date-utils.c \ + tests/test-rfc2616-dates.c +tests_test_rfc2616_dates_CFLAGS = $(TESTS_CFLAGS) +tests_test_rfc2616_dates_LDADD = $(TESTS_LDADD) + if USE_GPGME tests_test_gpg_verify_result_SOURCES = \ src/libostree/ostree-gpg-verify-result-private.h \ diff --git a/src/libostree/ostree-date-utils-private.h b/src/libostree/ostree-date-utils-private.h new file mode 100644 index 0000000000..f9b8b3e086 --- /dev/null +++ b/src/libostree/ostree-date-utils-private.h @@ -0,0 +1,38 @@ +/* + * Copyright © 2020 Endless OS Foundation LLC + * + * SPDX-License-Identifier: LGPL-2.0+ + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + * + * Authors: + * - Philip Withnall + */ + +#pragma once + +#ifndef __GI_SCANNER__ + +#include + +G_BEGIN_DECLS + +GDateTime *_ostree_parse_rfc2616_date_time (const char *buf, + size_t len); + +G_END_DECLS + +#endif diff --git a/src/libostree/ostree-date-utils.c b/src/libostree/ostree-date-utils.c new file mode 100644 index 0000000000..8076e084cb --- /dev/null +++ b/src/libostree/ostree-date-utils.c @@ -0,0 +1,166 @@ +/* + * Copyright © 2020 Endless OS Foundation LLC + * + * SPDX-License-Identifier: LGPL-2.0+ + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + * + * Authors: + * - Philip Withnall + */ + +#include "config.h" + +#include +#include +#include + +#include "ostree-date-utils-private.h" + +/* @buf must already be known to be long enough */ +static gboolean +parse_uint (const char *buf, + guint n_digits, + guint min, + guint max, + guint *out) +{ + guint64 number; + const char *end_ptr = NULL; + gint saved_errno = 0; + + g_return_val_if_fail (n_digits == 2 || n_digits == 4, FALSE); + g_return_val_if_fail (out != NULL, FALSE); + + errno = 0; + number = g_ascii_strtoull (buf, (gchar **)&end_ptr, 10); + saved_errno = errno; + + if (!g_ascii_isdigit (buf[0]) || + saved_errno != 0 || + end_ptr == NULL || + end_ptr != buf + n_digits || + number < min || + number > max) + return FALSE; + + *out = number; + return TRUE; +} + +/* Locale-independent parsing for RFC 2616 date/times. + * + * Reference: https://tools.ietf.org/html/rfc2616#section-3.3.1 + * + * Syntax: + * , :: GMT + * + * Note that this only accepts the full-year and GMT formats specified by + * RFC 1123. It doesn’t accept RFC 850 or asctime formats. + * + * Example: + * Wed, 21 Oct 2015 07:28:00 GMT + */ +GDateTime * +_ostree_parse_rfc2616_date_time (const char *buf, + size_t len) +{ + guint day_int, year_int, hour_int, minute_int, second_int; + const char *day_names[] = + { + "Mon", + "Tue", + "Wed", + "Thu", + "Fri", + "Sat", + "Sun", + }; + size_t day_name_index; + const char *month_names[] = + { + "Jan", + "Feb", + "Mar", + "Apr", + "May", + "Jun", + "Jul", + "Aug", + "Sep", + "Oct", + "Nov", + "Dec", + }; + size_t month_name_index; + + if (len != 29) + return NULL; + + const char *day_name = buf; + const char *day = buf + 5; + const char *month_name = day + 3; + const char *year = month_name + 4; + const char *hour = year + 5; + const char *minute = hour + 3; + const char *second = minute + 3; + const char *tz = second + 3; + + for (day_name_index = 0; day_name_index < G_N_ELEMENTS (day_names); day_name_index++) + { + if (strncmp (day_names[day_name_index], day_name, 3) == 0) + break; + } + if (day_name_index >= G_N_ELEMENTS (day_names)) + return NULL; + /* don’t validate whether the day_name matches the rest of the date */ + if (*(day_name + 3) != ',' || *(day_name + 4) != ' ') + return NULL; + if (!parse_uint (day, 2, 1, 31, &day_int)) + return NULL; + if (*(day + 2) != ' ') + return NULL; + for (month_name_index = 0; month_name_index < G_N_ELEMENTS (month_names); month_name_index++) + { + if (strncmp (month_names[month_name_index], month_name, 3) == 0) + break; + } + if (month_name_index >= G_N_ELEMENTS (month_names)) + return NULL; + if (*(month_name + 3) != ' ') + return NULL; + if (!parse_uint (year, 4, 0, 9999, &year_int)) + return NULL; + if (*(year + 4) != ' ') + return NULL; + if (!parse_uint (hour, 2, 0, 23, &hour_int)) + return NULL; + if (*(hour + 2) != ':') + return NULL; + if (!parse_uint (minute, 2, 0, 59, &minute_int)) + return NULL; + if (*(minute + 2) != ':') + return NULL; + if (!parse_uint (second, 2, 0, 60, &second_int)) /* allow leap seconds */ + return NULL; + if (*(second + 2) != ' ') + return NULL; + if (strncmp (tz, "GMT", 3) != 0) + return NULL; + + return g_date_time_new_utc (year_int, month_name_index + 1, day_int, + hour_int, minute_int, second_int); +} diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 975508ebff..129e6988e6 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -45,6 +45,7 @@ #define CURLPIPE_MULTIPLEX 0 #endif +#include "ostree-date-utils-private.h" #include "ostree-fetcher.h" #include "ostree-fetcher-util.h" #include "ostree-enumtypes.h" @@ -591,141 +592,6 @@ write_cb (void *ptr, size_t size, size_t nmemb, void *data) return realsize; } -/* @buf must already be known to be long enough */ -static gboolean -parse_uint (const char *buf, - guint n_digits, - guint min, - guint max, - guint *out) -{ - guint64 number; - const char *end_ptr = NULL; - gint saved_errno = 0; - - g_return_val_if_fail (n_digits == 2 || n_digits == 4, FALSE); - g_return_val_if_fail (out != NULL, FALSE); - - errno = 0; - number = g_ascii_strtoull (buf, (gchar **)&end_ptr, 10); - saved_errno = errno; - - if (!g_ascii_isdigit (buf[0]) || - saved_errno != 0 || - end_ptr == NULL || - end_ptr != buf + n_digits || - number < min || - number > max) - return FALSE; - - *out = number; - return TRUE; -} - -/* Locale-independent parsing for RFC 2616 date/times. - * - * Reference: https://tools.ietf.org/html/rfc2616#section-3.3.1 - * - * Syntax: - * , :: GMT - * - * Note that this only accepts the full-year and GMT formats specified by - * RFC 1123. It doesn’t accept RFC 850 or asctime formats. - * - * Example: - * Wed, 21 Oct 2015 07:28:00 GMT - */ -static GDateTime * -parse_rfc2616_date_time (const char *buf, - size_t len) -{ - guint day_int, year_int, hour_int, minute_int, second_int; - const char *day_names[] = - { - "Mon", - "Tue", - "Wed", - "Thu", - "Fri", - "Sat", - "Sun", - }; - size_t day_name_index; - const char *month_names[] = - { - "Jan", - "Feb", - "Mar", - "Apr", - "May", - "Jun", - "Jul", - "Aug", - "Sep", - "Oct", - "Nov", - "Dec", - }; - size_t month_name_index; - - if (len != 29) - return NULL; - - const char *day_name = buf; - const char *day = buf + 5; - const char *month_name = day + 3; - const char *year = month_name + 4; - const char *hour = year + 5; - const char *minute = hour + 3; - const char *second = minute + 3; - const char *tz = second + 3; - - for (day_name_index = 0; day_name_index < G_N_ELEMENTS (day_names); day_name_index++) - { - if (strncmp (day_names[day_name_index], day_name, 3) == 0) - break; - } - if (day_name_index >= G_N_ELEMENTS (day_names)) - return NULL; - /* don’t validate whether the day_name matches the rest of the date */ - if (*(day_name + 3) != ',' || *(day_name + 4) != ' ') - return NULL; - if (!parse_uint (day, 2, 1, 31, &day_int)) - return NULL; - if (*(day + 2) != ' ') - return NULL; - for (month_name_index = 0; month_name_index < G_N_ELEMENTS (month_names); month_name_index++) - { - if (strncmp (month_names[month_name_index], month_name, 3) == 0) - break; - } - if (month_name_index >= G_N_ELEMENTS (month_names)) - return NULL; - if (*(month_name + 3) != ' ') - return NULL; - if (!parse_uint (year, 4, 0, 9999, &year_int)) - return NULL; - if (*(year + 4) != ' ') - return NULL; - if (!parse_uint (hour, 2, 0, 23, &hour_int)) - return NULL; - if (*(hour + 2) != ':') - return NULL; - if (!parse_uint (minute, 2, 0, 59, &minute_int)) - return NULL; - if (*(minute + 2) != ':') - return NULL; - if (!parse_uint (second, 2, 0, 60, &second_int)) /* allow leap seconds */ - return NULL; - if (*(second + 2) != ' ') - return NULL; - if (strncmp (tz, "GMT", 3) != 0) - return NULL; - - return g_date_time_new_utc (year_int, month_name_index + 1, day_int, - hour_int, minute_int, second_int); -} - /* CURLOPT_HEADERFUNCTION */ static size_t response_header_cb (const char *buffer, size_t size, size_t n_items, void *user_data) @@ -753,7 +619,7 @@ response_header_cb (const char *buffer, size_t size, size_t n_items, void *user_ strncasecmp (buffer, last_modified_header, strlen (last_modified_header)) == 0) { g_autofree char *lm_buf = g_strstrip (g_strdup (buffer + strlen (last_modified_header))); - g_autoptr(GDateTime) dt = parse_rfc2616_date_time (lm_buf, strlen (lm_buf)); + g_autoptr(GDateTime) dt = _ostree_parse_rfc2616_date_time (lm_buf, strlen (lm_buf)); req->out_last_modified = (dt != NULL) ? g_date_time_to_unix (dt) : 0; } diff --git a/tests/.gitignore b/tests/.gitignore index f5e95e49f6..938c169f4b 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -21,5 +21,6 @@ test-repo test-repo-finder-avahi test-repo-finder-config test-repo-finder-mount +test-rfc2616-dates test-rollsum-cli test-kargs diff --git a/tests/test-rfc2616-dates.c b/tests/test-rfc2616-dates.c new file mode 100644 index 0000000000..d3f2073ec6 --- /dev/null +++ b/tests/test-rfc2616-dates.c @@ -0,0 +1,123 @@ +/* + * Copyright © 2020 Endless OS Foundation LLC + * + * SPDX-License-Identifier: LGPL-2.0+ + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + * + * Authors: + * - Philip Withnall + */ + +#include "config.h" + +#include + +#include "ostree-date-utils-private.h" + +static void +test_ostree_parse_rfc2616_date_time (void) +{ +#if GLIB_CHECK_VERSION(2, 62, 0) +G_GNUC_BEGIN_IGNORE_DEPRECATIONS + const struct + { + const char *rfc2616; + const char *expected_iso8601; /* (nullable) if parsing is expected to fail */ + } + tests[] = + { + { "Wed, 21 Oct 2015 07:28:00 GMT", "2015-10-21T07:28:00Z" }, + { "Wed, 21 Oct 2015 07:28:00", NULL }, /* too short */ + { "Wed, 21 Oct 2015 07:28:00 CEST", NULL }, /* too long; not GMT */ + { "Cat, 21 Oct 2015 07:28:00 GMT", NULL }, /* invalid day */ + { "Wed 21 Oct 2015 07:28:00 GMT", NULL }, /* no comma */ + { "Wed,21 Oct 2015 07:28:00 GMT ", NULL }, /* missing space */ + { "Wed, xx Oct 2015 07:28:00 GMT", NULL }, /* no day-of-month */ + { "Wed, 011Oct 2015 07:28:00 GMT", NULL }, /* overlong day-of-month */ + { "Wed, 00 Oct 2015 07:28:00 GMT", NULL }, /* day-of-month underflow */ + { "Wed, 32 Oct 2015 07:28:00 GMT", NULL }, /* day-of-month overflow */ + { "Wed, 21,Oct 2015 07:28:00 GMT", NULL }, /* missing space */ + { "Wed, 21 Cat 2015 07:28:00 GMT", NULL }, /* invalid month */ + { "Wed, 21 Oct,2015 07:28:00 GMT", NULL }, /* missing space */ + { "Wed, 21 Oct xxxx 07:28:00 GMT", NULL }, /* no year */ + { "Wed, 21 Oct 0201507:28:00 GMT", NULL }, /* overlong year */ + { "Wed, 21 Oct 0000 07:28:00 GMT", NULL }, /* year underflow */ + { "Wed, 21 Oct 10000 07:28:00 GM", NULL }, /* year overflow */ + { "Wed, 21 Oct 2015,07:28:00 GMT", NULL }, /* missing space */ + { "Wed, 21 Oct 2015 07 28:00 GMT", NULL }, /* missing colon */ + { "Wed, 21 Oct 2015 007:28:00 GM", NULL }, /* overlong hour */ + { "Wed, 21 Oct 2015 xx:28:00 GMT", NULL }, /* missing hour */ + { "Wed, 21 Oct 2015 -1:28:00 GMT", NULL }, /* hour underflow */ + { "Wed, 21 Oct 2015 24:28:00 GMT", NULL }, /* hour overflow */ + { "Wed, 21 Oct 2015 07:28 00 GMT", NULL }, /* missing colon */ + { "Wed, 21 Oct 2015 07:028:00 GM", NULL }, /* overlong minute */ + { "Wed, 21 Oct 2015 07:xx:00 GMT", NULL }, /* missing minute */ + { "Wed, 21 Oct 2015 07:-1:00 GMT", NULL }, /* minute underflow */ + { "Wed, 21 Oct 2015 07:60:00 GMT", NULL }, /* minute overflow */ + { "Wed, 21 Oct 2015 07:28:00CEST", NULL }, /* missing space */ + { "Wed, 21 Oct 2015 07:28:000 GM", NULL }, /* overlong second */ + { "Wed, 21 Oct 2015 07:28:xx GMT", NULL }, /* missing second */ + { "Wed, 21 Oct 2015 07:28:-1 GMT", NULL }, /* seconds underflow */ + { "Wed, 21 Oct 2015 07:28:61 GMT", NULL }, /* seconds overflow */ + { "Wed, 21 Oct 2015 07:28:00 UTC", NULL }, /* invalid timezone (only GMT is allowed) */ + { "Thu, 01 Jan 1970 00:00:00 GMT", "1970-01-01T00:00:00Z" }, /* extreme but valid date */ + { "Mon, 31 Dec 9999 23:59:59 GMT", "9999-12-31T23:59:59Z" }, /* extreme but valid date */ + }; + + for (gsize i = 0; i < G_N_ELEMENTS (tests); i++) + { + g_test_message ("Test %" G_GSIZE_FORMAT ": %s", i, tests[i].rfc2616); + + /* Parse once with a trailing nul */ + g_autoptr(GDateTime) dt1 = _ostree_parse_rfc2616_date_time (tests[i].rfc2616, strlen (tests[i].rfc2616)); + if (tests[i].expected_iso8601 == NULL) + g_assert_null (dt1); + else + { + g_assert_nonnull (dt1); + g_autofree char *iso8601 = g_date_time_format_iso8601 (dt1); + g_assert_cmpstr (iso8601, ==, tests[i].expected_iso8601); + } + + /* And parse again with no trailing nul */ + g_autofree char *rfc2616_no_nul = g_malloc (strlen (tests[i].rfc2616)); + memcpy (rfc2616_no_nul, tests[i].rfc2616, strlen (tests[i].rfc2616)); + g_autoptr(GDateTime) dt2 = _ostree_parse_rfc2616_date_time (rfc2616_no_nul, strlen (tests[i].rfc2616)); + if (tests[i].expected_iso8601 == NULL) + g_assert_null (dt2); + else + { + g_assert_nonnull (dt2); + g_autofree char *iso8601 = g_date_time_format_iso8601 (dt2); + g_assert_cmpstr (iso8601, ==, tests[i].expected_iso8601); + } + } +G_GNUC_END_IGNORE_DEPRECATIONS +#else + /* GLib 2.62 is needed for g_date_time_format_iso8601(). */ + g_test_skip ("RFC 2616 date parsing test needs GLib ≥ 2.62.0"); +#endif +} + +int +main (int argc, + char **argv) +{ + g_test_init (&argc, &argv, NULL); + g_test_add_func ("/ostree_parse_rfc2616_date_time", test_ostree_parse_rfc2616_date_time); + return g_test_run (); +}