Add support for a Proxy directive

Message ID 20221120234316.13316-1-briang@apache.org
State Rejected, archived
Headers show
Series Add support for a Proxy directive | expand

Commit Message

Brian Geffon Nov. 20, 2022, 11:43 p.m. UTC
Currently the only way to use a proxy is via XferCommand; however,
when doing so there is no support for parallel downloads. This
change introduces a new directive Proxy which supports whatever
proxy protocols are supported by libcurl.

Signed-off-by: Brian Geffon <briang@apache.org>
---
 doc/pacman.conf.5.asciidoc           |  7 +++++++
 etc/pacman.conf.in                   |  5 +++++
 lib/libalpm/alpm.h                   | 24 ++++++++++++++++++++++++
 lib/libalpm/dload.c                  |  3 +++
 lib/libalpm/handle.c                 | 22 ++++++++++++++++++++++
 lib/libalpm/handle.h                 |  1 +
 scripts/completion/zsh_completion.in |  1 +
 src/pacman/conf.c                    | 10 ++++++++++
 src/pacman/conf.h                    |  2 ++
 src/pacman/pacman-conf.c             |  4 +++-
 10 files changed, 78 insertions(+), 1 deletion(-)

Comments

Andrew Gregory Nov. 21, 2022, 12:09 a.m. UTC | #1
On 11/20/22 at 06:43pm, Brian Geffon wrote:
> Currently the only way to use a proxy is via XferCommand; however,

Not true; from man CURLOPT_PROXY:

    libcurl respects the proxy environment variables named http_proxy,
    ftp_proxy, sftp_proxy etc. If set, libcurl will use the specified proxy for
    that URL scheme. So  for  a  "FTP://"  URL,  the  ftp_proxy  is
    considered.  all_proxy is used if no protocol specific proxy was set.

> when doing so there is no support for parallel downloads. This
> change introduces a new directive Proxy which supports whatever
> proxy protocols are supported by libcurl.
Brian Geffon Nov. 21, 2022, 12:19 a.m. UTC | #2
On Sun, Nov 20, 2022 at 7:09 PM Andrew Gregory
<andrew.gregory.8@gmail.com> wrote:
>
> On 11/20/22 at 06:43pm, Brian Geffon wrote:
> > Currently the only way to use a proxy is via XferCommand; however,
>
> Not true; from man CURLOPT_PROXY:

I apologize, I stand corrected. Happy to reword the commit message to
state that this
allows the proxy to be configured at a system level. Otherwise I'm
also happy to just
withdraw the patch if that doesn't seem useful.

>
>     libcurl respects the proxy environment variables named http_proxy,
>     ftp_proxy, sftp_proxy etc. If set, libcurl will use the specified proxy for
>     that URL scheme. So  for  a  "FTP://"  URL,  the  ftp_proxy  is
>     considered.  all_proxy is used if no protocol specific proxy was set.
>
> > when doing so there is no support for parallel downloads. This
> > change introduces a new directive Proxy which supports whatever
> > proxy protocols are supported by libcurl.

Thanks for looking,
Brian
Andrew Gregory Nov. 21, 2022, 2:17 a.m. UTC | #3
On 11/20/22 at 07:19pm, Brian Geffon wrote:
> On Sun, Nov 20, 2022 at 7:09 PM Andrew Gregory
> <andrew.gregory.8@gmail.com> wrote:
> >
> > On 11/20/22 at 06:43pm, Brian Geffon wrote:
> > > Currently the only way to use a proxy is via XferCommand; however,
> >
> > Not true; from man CURLOPT_PROXY:
> 
> I apologize, I stand corrected. Happy to reword the commit message to
> state that this
> allows the proxy to be configured at a system level. Otherwise I'm
> also happy to just
> withdraw the patch if that doesn't seem useful.

Given that a proxy is one of the few ways to influence the download process
that doesn't involve us adding dozens of knobs to the downloader, I'm not
necessarily opposed to adding it as an explicitly supported option (but I
haven't actually reviewed this patch yet).  I am curious how much utility this
actually has though.  I don't think I've ever actually heard from a user
needing to set a proxy specifically for pacman before so, to the best of my
knowledge, this is a pretty niche use case.
Brian Geffon Nov. 21, 2022, 5:12 p.m. UTC | #4
On Sun, Nov 20, 2022 at 9:17 PM Andrew Gregory
<andrew.gregory.8@gmail.com> wrote:
>
> On 11/20/22 at 07:19pm, Brian Geffon wrote:
> > On Sun, Nov 20, 2022 at 7:09 PM Andrew Gregory
> > <andrew.gregory.8@gmail.com> wrote:
> > >
> > > On 11/20/22 at 06:43pm, Brian Geffon wrote:
> > > > Currently the only way to use a proxy is via XferCommand; however,
> > >
> > > Not true; from man CURLOPT_PROXY:
> >
> > I apologize, I stand corrected. Happy to reword the commit message to
> > state that this
> > allows the proxy to be configured at a system level. Otherwise I'm
> > also happy to just
> > withdraw the patch if that doesn't seem useful.
>
> Given that a proxy is one of the few ways to influence the download process
> that doesn't involve us adding dozens of knobs to the downloader, I'm not
> necessarily opposed to adding it as an explicitly supported option (but I
> haven't actually reviewed this patch yet).  I am curious how much utility this
> actually has though.  I don't think I've ever actually heard from a user
> needing to set a proxy specifically for pacman before so, to the best of my
> knowledge, this is a pretty niche use case.

Yeah, let's just go ahead and drop this.

Thanks for taking the time to look!
Brian

Patch

diff --git a/doc/pacman.conf.5.asciidoc b/doc/pacman.conf.5.asciidoc
index 41f3ea03..657264e1 100644
--- a/doc/pacman.conf.5.asciidoc
+++ b/doc/pacman.conf.5.asciidoc
@@ -132,6 +132,13 @@  Options
 	HTTP/FTP support, or need the more advanced proxy support that comes with
 	utilities like wget.
 
+*Proxy =* https://host:port::
+	If set, use this endpoint as the proxy for downloads. The currently
+	supported set of protocols are http, https, socks4, socks4a, socks5,
+	and socks5h. If no port is specified it defaults to 1080. This option
+	only applies when using the built in downloader. When using XferCommand
+	any proxy settings must be passed as command line arguments to the command.
+
 *NoUpgrade =* file ...::
 	All files listed with a `NoUpgrade` directive will never be touched during
 	a package install/upgrade, and the new files will be installed with a
diff --git a/etc/pacman.conf.in b/etc/pacman.conf.in
index 1799efc7..6b2ef8b9 100644
--- a/etc/pacman.conf.in
+++ b/etc/pacman.conf.in
@@ -21,6 +21,11 @@  HoldPkg     = pacman glibc
 #CleanMethod = KeepInstalled
 Architecture = auto
 
+# The Proxy directive only applies when not using an XferCommand. When using
+# an XferCommand any proxy strings must be passed as arguments to the command.
+#Proxy       = socks5h://127.0.0.1:1080
+#Proxy       = https://127.0.0.1:8080
+
 # Pacman won't upgrade packages listed in IgnorePkg and members of IgnoreGroup
 #IgnorePkg   =
 #IgnoreGroup =
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 07e16b9f..37dd414f 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -2166,6 +2166,30 @@  int alpm_option_set_dbext(alpm_handle_t *handle, const char *dbext);
 /* End of dbext accessors */
 /** @} */
 
+/** @name Accessors for proxy server configuration
+ *
+ * This controls the proxy server that will be used for downloads.
+ * This may be necessary for computers that only have internet access
+ * via HTTP or Socks proxies.
+ *
+ * @{
+ */
+
+/** Gets the configured proxy.
+ * @param handle the context handle
+ * @return the configured proxy
+ */
+const char *alpm_option_get_proxy(alpm_handle_t *handle);
+
+/** Sets the proxy server.
+ * @param handle the context handle
+ * @param proxy the full proxy string to use.
+ * @return 0 on success, -1 on error (pm_errno is set accordingly)
+ */
+int alpm_option_set_proxy(alpm_handle_t *handle, const char *proxy);
+
+/* End of proxy accessors */
+/** @} */
 
 /** @name Accessors for the signature levels
  * @{
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 4fa17b35..171a802c 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -340,6 +340,9 @@  static void curl_set_handle_opts(CURL *curl, struct dload_payload *payload)
 	curl_easy_setopt(curl, CURLOPT_TCP_KEEPINTVL, 60L);
 	curl_easy_setopt(curl, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 	curl_easy_setopt(curl, CURLOPT_PRIVATE, (void *)payload);
+	if (handle->proxy) {
+		curl_easy_setopt(curl, CURLOPT_PROXY, handle->proxy);
+	}
 
 	_alpm_log(handle, ALPM_LOG_DEBUG, "%s: url is %s\n",
 		payload->remote_name, payload->fileurl);
diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
index d1eafeda..e72627ea 100644
--- a/lib/libalpm/handle.c
+++ b/lib/libalpm/handle.c
@@ -73,6 +73,7 @@  void _alpm_handle_free(alpm_handle_t *handle)
 	FREE(handle->root);
 	FREE(handle->dbpath);
 	FREE(handle->dbext);
+	FREE(handle->proxy);
 	FREELIST(handle->cachedirs);
 	FREELIST(handle->hookdirs);
 	FREE(handle->logfile);
@@ -330,6 +331,12 @@  const char SYMEXPORT *alpm_option_get_dbext(alpm_handle_t *handle)
 	return handle->dbext;
 }
 
+const char SYMEXPORT *alpm_option_get_proxy(alpm_handle_t *handle)
+{
+	CHECK_HANDLE(handle, return NULL);
+	return handle->proxy;
+}
+
 int SYMEXPORT alpm_option_get_parallel_downloads(alpm_handle_t *handle)
 {
 	CHECK_HANDLE(handle, return -1);
@@ -822,6 +829,21 @@  int SYMEXPORT alpm_option_set_dbext(alpm_handle_t *handle, const char *dbext)
 	return 0;
 }
 
+int SYMEXPORT alpm_option_set_proxy(alpm_handle_t *handle, const char *proxy)
+{
+	CHECK_HANDLE(handle, return -1);
+	ASSERT(proxy, RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1));
+
+	if(handle->proxy) {
+		FREE(handle->proxy);
+	}
+
+	STRDUP(handle->proxy, proxy, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
+
+	_alpm_log(handle, ALPM_LOG_DEBUG, "option 'proxy' = %s\n", handle->proxy);
+	return 0;
+}
+
 int SYMEXPORT alpm_option_set_default_siglevel(alpm_handle_t *handle,
 		int level)
 {
diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h
index 3a464689..32063ef9 100644
--- a/lib/libalpm/handle.h
+++ b/lib/libalpm/handle.h
@@ -107,6 +107,7 @@  struct _alpm_handle_t {
 	int usesyslog;           /* Use syslog instead of logfile? */ /* TODO move to frontend */
 	int checkspace;          /* Check disk space before installing */
 	char *dbext;             /* Sync DB extension */
+	char *proxy;		 /* Proxy server to use */
 	int siglevel;            /* Default signature verification level */
 	int localfilesiglevel;   /* Signature verification level for local file
 	                                       upgrade operations */
diff --git a/scripts/completion/zsh_completion.in b/scripts/completion/zsh_completion.in
index f65edeb2..69b19a18 100644
--- a/scripts/completion/zsh_completion.in
+++ b/scripts/completion/zsh_completion.in
@@ -516,6 +516,7 @@  _pacman_conf_general_directives=(
 	'DisableDownloadTimeout'
 	'NoProgressBar'
 	'ParallelDownloads'
+	'Proxy'
 	'CleanMethod'
 	'SigLevel'
 	'LocalFileSigLevel'
diff --git a/src/pacman/conf.c b/src/pacman/conf.c
index f9edf75b..37691519 100644
--- a/src/pacman/conf.c
+++ b/src/pacman/conf.c
@@ -154,6 +154,7 @@  int config_free(config_t *oldconfig)
 	free(oldconfig->rootdir);
 	free(oldconfig->dbpath);
 	free(oldconfig->logfile);
+	free(oldconfig->proxy);
 	free(oldconfig->gpgdir);
 	FREELIST(oldconfig->hookdirs);
 	FREELIST(oldconfig->cachedirs);
@@ -668,6 +669,11 @@  static int _parse_options(const char *key, char *value,
 				config->logfile = strdup(value);
 				pm_printf(ALPM_LOG_DEBUG, "config: logfile: %s\n", value);
 			}
+		} else if(strcmp(key, "Proxy") == 0) {
+			if(!config->proxy) {
+				config->proxy = strdup(value);
+				pm_printf(ALPM_LOG_DEBUG, "config: proxy: %s\n", value);
+			}
 		} else if(strcmp(key, "XferCommand") == 0) {
 			char **c;
 			if((config->xfercommand_argv = wordsplit(value)) == NULL) {
@@ -901,6 +907,10 @@  static int setup_libalpm(void)
 		pm_printf(ALPM_LOG_WARNING, _("no '%s' configured\n"), "XferCommand");
 	}
 
+	if (config->proxy) {
+		alpm_option_set_proxy(handle, config->proxy);
+	}
+
 	alpm_option_set_architectures(handle, config->architectures);
 	alpm_option_set_checkspace(handle, config->checkspace);
 	alpm_option_set_usesyslog(handle, config->usesyslog);
diff --git a/src/pacman/conf.h b/src/pacman/conf.h
index f7916ca9..c441cd80 100644
--- a/src/pacman/conf.h
+++ b/src/pacman/conf.h
@@ -115,6 +115,8 @@  typedef struct __config_t {
 	unsigned short verbosepkglists;
 	/* number of parallel download streams */
 	unsigned int parallel_downloads;
+	/* the proxy server to use */
+	char *proxy;
 	/* select -Sc behavior */
 	unsigned short cleanmethod;
 	alpm_list_t *holdpkg;
diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c
index a9d1f52b..baf3cebd 100644
--- a/src/pacman/pacman-conf.c
+++ b/src/pacman/pacman-conf.c
@@ -269,6 +269,7 @@  static void dump_config(void)
 	show_bool("ILoveCandy", config->chomp);
 	show_bool("NoProgressBar", config->noprogressbar);
 
+	show_str("Proxy", config->proxy);
 	show_int("ParallelDownloads", config->parallel_downloads);
 
 	show_cleanmethod("CleanMethod", config->cleanmethod);
@@ -384,7 +385,8 @@  static int list_directives(void)
 
 		} else if(strcasecmp(i->data, "ParallelDownloads") == 0) {
 			show_int("ParallelDownloads", config->parallel_downloads);
-
+		} else if(strcasecmp(i->data, "Proxy") == 0) {
+			show_str("Proxy", config->proxy);
 		} else if(strcasecmp(i->data, "CleanMethod") == 0) {
 			show_cleanmethod("CleanMethod", config->cleanmethod);