Consistently use flatpak_load_http_uri()

Currently Flatpak has a few different implementations of helper
functions to download a URI using libsoup, but the best one seems to be
in common/flatpak-utils-http.c. So this commit deletes the others and
makes use of flatpak_load_http_uri() in place of download_uri() in a
few places. This has a couple consequences:
1) It means that we're now properly checking HTTP status codes rather
than assuming that the request was successful, in the install command,
remote-add command, and in FlatpakTransaction. This fixes an assertion
failure seen by a user when they tried to use a flatpakref URL that hit
a 404.
2) It means that in the places where we're using flatpak_load_http_uri()
we are only supporting http:// and https:// URLs not, say, file:// ones.
For the install and remote-add commands this was already being enforced.
For the handling of flatpakref files and bundles in FlatpakTransaction,
I believe it's just convention because it doesn't make much sense to
do anything else; this commit enforces that convention.

Also, add a unit test for the case of trying to install a flatpakref at
a URL that hits a 404 error.

Fixes https://github.com/flatpak/flatpak/issues/2727

Closes: #2740
Approved by: matthiasclasen
This commit is contained in:
Matthew Leeds
2019-03-05 13:04:51 -08:00
committed by Atomic Bot
parent 7a5c024695
commit 4e81789167
6 changed files with 35 additions and 102 deletions

View File

@@ -206,7 +206,9 @@ install_from (FlatpakDir *dir,
if (g_str_has_prefix (filename, "http:") ||
g_str_has_prefix (filename, "https:"))
{
file_data = download_uri (filename, error);
g_autoptr(SoupSession) soup_session = NULL;
soup_session = flatpak_create_soup_session (PACKAGE_STRING);
file_data = flatpak_load_http_uri (soup_session, filename, 0, NULL, NULL, cancellable, error);
if (file_data == NULL)
{
g_prefix_error (error, "Can't load uri %s: ", filename);

View File

@@ -183,8 +183,10 @@ load_options (const char *filename,
{
const char *options_data;
gsize options_size;
g_autoptr(SoupSession) soup_session = NULL;
bytes = download_uri (filename, &error);
soup_session = flatpak_create_soup_session (PACKAGE_STRING);
bytes = flatpak_load_http_uri (soup_session, filename, 0, NULL, NULL, NULL, &error);
if (bytes == NULL)
{

View File

@@ -92,65 +92,6 @@ looks_like_branch (const char *branch)
return TRUE;
}
static SoupSession *
get_soup_session (void)
{
static SoupSession *soup_session = NULL;
if (soup_session == NULL)
{
const char *http_proxy;
soup_session = soup_session_new_with_options (SOUP_SESSION_USER_AGENT, "flatpak-builder ",
SOUP_SESSION_SSL_USE_SYSTEM_CA_FILE, TRUE,
SOUP_SESSION_USE_THREAD_CONTEXT, TRUE,
SOUP_SESSION_TIMEOUT, 60,
SOUP_SESSION_IDLE_TIMEOUT, 60,
NULL);
http_proxy = g_getenv ("http_proxy");
if (http_proxy)
{
g_autoptr(SoupURI) proxy_uri = soup_uri_new (http_proxy);
if (!proxy_uri)
g_warning ("Invalid proxy URI '%s'", http_proxy);
else
g_object_set (soup_session, SOUP_SESSION_PROXY_URI, proxy_uri, NULL);
}
}
return soup_session;
}
GBytes *
download_uri (const char *url,
GError **error)
{
SoupSession *session;
g_autoptr(SoupRequest) req = NULL;
g_autoptr(GInputStream) input = NULL;
g_autoptr(GOutputStream) out = NULL;
session = get_soup_session ();
req = soup_session_request (session, url, error);
if (req == NULL)
return NULL;
input = soup_request_send (req, NULL, error);
if (input == NULL)
return NULL;
out = g_memory_output_stream_new_resizable ();
if (!g_output_stream_splice (out,
input,
G_OUTPUT_STREAM_SPLICE_CLOSE_TARGET | G_OUTPUT_STREAM_SPLICE_CLOSE_SOURCE,
NULL,
error))
return NULL;
return g_memory_output_stream_steal_as_bytes (G_MEMORY_OUTPUT_STREAM (out));
}
FlatpakDir *
flatpak_find_installed_pref (const char *pref, FlatpakKinds kinds, const char *default_arch, const char *default_branch,
gboolean search_all, gboolean search_user, gboolean search_system, char **search_installations,

View File

@@ -52,8 +52,6 @@ RemoteDirPair * remote_dir_pair_new (const char *remote_name,
FlatpakDir *dir);
gboolean looks_like_branch (const char *branch);
GBytes * download_uri (const char *url,
GError **error);
GBytes * flatpak_load_gpg_keys (char **gpg_import,
GCancellable *cancellable,

View File

@@ -2351,37 +2351,6 @@ flatpak_transaction_get_installation (FlatpakTransaction *self)
return g_object_ref (priv->installation);
}
static GBytes *
download_uri (const char *url,
GError **error)
{
g_autoptr(SoupSession) session = NULL;
g_autoptr(SoupRequest) req = NULL;
g_autoptr(GInputStream) input = NULL;
g_autoptr(GOutputStream) out = NULL;
session = flatpak_create_soup_session (PACKAGE_STRING);
req = soup_session_request (session, url, error);
if (req == NULL)
return NULL;
input = soup_request_send (req, NULL, error);
if (input == NULL)
return NULL;
out = g_memory_output_stream_new_resizable ();
if (!g_output_stream_splice (out,
input,
G_OUTPUT_STREAM_SPLICE_CLOSE_TARGET | G_OUTPUT_STREAM_SPLICE_CLOSE_SOURCE,
NULL,
error))
return NULL;
return g_memory_output_stream_steal_as_bytes (G_MEMORY_OUTPUT_STREAM (out));
}
static gboolean
remote_is_already_configured (FlatpakTransaction *self,
const char *url,
@@ -2465,7 +2434,11 @@ handle_suggested_remote_name (FlatpakTransaction *self, GKeyFile *keyfile, GErro
}
static gboolean
handle_runtime_repo_deps (FlatpakTransaction *self, const char *id, const char *dep_url, GError **error)
handle_runtime_repo_deps (FlatpakTransaction *self,
const char *id,
const char *dep_url,
GCancellable *cancellable,
GError **error)
{
FlatpakTransactionPrivate *priv = flatpak_transaction_get_instance_private (self);
g_autoptr(GBytes) dep_data = NULL;
@@ -2480,6 +2453,7 @@ handle_runtime_repo_deps (FlatpakTransaction *self, const char *id, const char *
g_autofree char *group = NULL;
g_autoptr(GError) local_error = NULL;
g_autofree char *runtime_collection_id = NULL;
g_autoptr(SoupSession) soup_session = NULL;
char *t;
int i;
gboolean res;
@@ -2487,7 +2461,11 @@ handle_runtime_repo_deps (FlatpakTransaction *self, const char *id, const char *
if (priv->disable_deps)
return TRUE;
dep_data = download_uri (dep_url, error);
if (!g_str_has_prefix (dep_url, "http:") && !g_str_has_prefix (dep_url, "https:"))
return flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA, _("Flatpakrepo URL %s not HTTP or HTTPS"), dep_url);
soup_session = flatpak_create_soup_session (PACKAGE_STRING);
dep_data = flatpak_load_http_uri (soup_session, dep_url, 0, NULL, NULL, cancellable, error);
if (dep_data == NULL)
{
g_prefix_error (error, "Can't load dependent file %s", dep_url);
@@ -2556,7 +2534,10 @@ handle_runtime_repo_deps (FlatpakTransaction *self, const char *id, const char *
}
static gboolean
handle_runtime_repo_deps_from_keyfile (FlatpakTransaction *self, GKeyFile *keyfile, GError **error)
handle_runtime_repo_deps_from_keyfile (FlatpakTransaction *self,
GKeyFile *keyfile,
GCancellable *cancellable,
GError **error)
{
FlatpakTransactionPrivate *priv = flatpak_transaction_get_instance_private (self);
g_autofree char *dep_url = NULL;
@@ -2574,7 +2555,7 @@ handle_runtime_repo_deps_from_keyfile (FlatpakTransaction *self, GKeyFile *keyfi
if (name == NULL)
return TRUE;
return handle_runtime_repo_deps (self, name, dep_url, error);
return handle_runtime_repo_deps (self, name, dep_url, cancellable, error);
}
static gboolean
@@ -2595,7 +2576,7 @@ flatpak_transaction_resolve_flatpakrefs (FlatpakTransaction *self,
if (!handle_suggested_remote_name (self, flatpakref, error))
return FALSE;
if (!handle_runtime_repo_deps_from_keyfile (self, flatpakref, error))
if (!handle_runtime_repo_deps_from_keyfile (self, flatpakref, cancellable, error))
return FALSE;
if (!flatpak_dir_create_remote_for_ref_file (priv->dir, flatpakref, priv->default_arch,
@@ -2618,6 +2599,7 @@ flatpak_transaction_resolve_flatpakrefs (FlatpakTransaction *self,
static gboolean
handle_runtime_repo_deps_from_bundle (FlatpakTransaction *self,
GFile *file,
GCancellable *cancellable,
GError **error)
{
FlatpakTransactionPrivate *priv = flatpak_transaction_get_instance_private (self);
@@ -2645,7 +2627,7 @@ handle_runtime_repo_deps_from_bundle (FlatpakTransaction *self,
ref_parts = g_strsplit (ref, "/", -1);
return handle_runtime_repo_deps (self, ref_parts[1], dep_url, error);
return handle_runtime_repo_deps (self, ref_parts[1], dep_url, cancellable, error);
}
static gboolean
@@ -2665,7 +2647,7 @@ flatpak_transaction_resolve_bundles (FlatpakTransaction *self,
g_autofree char *metadata = NULL;
gboolean created_remote;
if (!handle_runtime_repo_deps_from_bundle (self, data->file, error))
if (!handle_runtime_repo_deps_from_bundle (self, data->file, cancellable, error))
return FALSE;
if (!flatpak_dir_ensure_repo (priv->dir, cancellable, error))

View File

@@ -23,7 +23,7 @@ set -euo pipefail
skip_without_bwrap
echo "1..27"
echo "1..28"
#Regular repo
setup_repo
@@ -169,6 +169,14 @@ fi
echo "ok missing remote name auto-corrects for install"
port=$(cat httpd-port-main)
if ${FLATPAK} ${U} install -y http://127.0.0.1:${port}/nonexistent.flatpakref 2> install-error-log; then
assert_not_reached "Should not be able to install a nonexistent flatpakref"
fi
assert_file_has_content install-error-log "Server returned status 404: Not Found"
echo "ok install fails gracefully for 404 URLs"
${FLATPAK} ${U} uninstall -y org.test.Platform org.test.Hello
if ${FLATPAK} ${U} install -y test-missing-gpg-repo org.test.Platform 2> install-error-log; then