Message ID | 20200820165050.GI4729@sunshine.barsnick.net |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel] libavformat/http, tls: honor http_proxy command line variable for HTTPS | expand |
Context | Check | Description |
---|---|---|
andriy/default | pending | |
andriy/make | success | Make finished |
andriy/make_fate | success | Make fate finished |
On Thu, 20 Aug 2020, Moritz Barsnick wrote: > Add the "http_proxy" option and its handling to the "tls" protocol, > pass the option from the "https" protocol. > > The "https" protocol already defines the "http_proxy" command line > option, like the "http" protocol does. The "http" protocol properly > honors that command line option in addition to the environment > variable. The "https" protocol doesn't, because the proxy is > evaluated in the underlying "tls" protocol, which doesn't have this > option, and thus only handles the environment variable, which it > has access to. > > Documentation for the "tls" protocol is not changed, as the new > option is basically only useful together with the "https" protocol. The patch looks fine in general, I think, but I don't agree with the last statement here. Even if you do e.g. a plain tls socket (or rtmps, or whatever), you may want to do the tls connection through a proxy server (be that a socks proxy or http proxy). For the case of rtmps, if you pass the http_proxy option, it should be passed to the rtmps protocol, which doesn't have such an option, from where it's passed along in the options dictionary down until the tls protocol consumes it. This just doesn't work for https, as the shared http/https options dictionary consumes the option from the dictionary, so you have to readd it like you do in your patch. // Martin
Hej Martin, On Thu, Aug 20, 2020 at 22:05:39 +0300, Martin Storsjö wrote: > Even if you do e.g. a plain tls socket (or rtmps, or whatever), you may > want to do the tls connection through a proxy server (be that a socks > proxy or http proxy). Actually, I forget that an HTTP proxy with CONNECT support cn transport any plain socket. I will amend documentation to doc/*.texi. > For the case of rtmps, if you pass the http_proxy option, it should be > passed to the rtmps protocol, which doesn't have such an option, from > where it's passed along in the options dictionary down until the tls > protocol consumes it. So you're saying it already works for rtmps, nothing needs to be changed there? Fine. Thanks for the review, will follow up with v2, Moritz
Hej, On Thu, 20 Aug 2020, Moritz Barsnick wrote: > Hej Martin, > > On Thu, Aug 20, 2020 at 22:05:39 +0300, Martin Storsjö wrote: > >> Even if you do e.g. a plain tls socket (or rtmps, or whatever), you may >> want to do the tls connection through a proxy server (be that a socks >> proxy or http proxy). > > Actually, I forget that an HTTP proxy with CONNECT support cn transport > any plain socket. I will amend documentation to doc/*.texi. > >> For the case of rtmps, if you pass the http_proxy option, it should be >> passed to the rtmps protocol, which doesn't have such an option, from >> where it's passed along in the options dictionary down until the tls >> protocol consumes it. > > So you're saying it already works for rtmps, nothing needs to be > changed there? Fine. I didn't test it, but from browsing the code, it looks like it would work. // Martin
diff --git a/libavformat/http.c b/libavformat/http.c index 6c39da1a8b..21584bdce9 100644 --- a/libavformat/http.c +++ b/libavformat/http.c @@ -213,6 +213,12 @@ static int http_open_cnx_internal(URLContext *h, AVDictionary **options) use_proxy = 0; if (port < 0) port = 443; + /* pass http_proxy to underlying protocol */ + if (s->http_proxy) { + err = av_dict_set(options, "http_proxy", s->http_proxy, 0); + if (err < 0) + return err; + } } if (port < 0) port = 80; diff --git a/libavformat/tls.c b/libavformat/tls.c index 10e0792e29..302c0f8d59 100644 --- a/libavformat/tls.c +++ b/libavformat/tls.c @@ -89,7 +89,7 @@ int ff_tls_open_underlying(TLSShared *c, URLContext *parent, const char *uri, AV if (!c->host && !(c->host = av_strdup(c->underlying_host))) return AVERROR(ENOMEM); - proxy_path = getenv("http_proxy"); + proxy_path = c->http_proxy ? c->http_proxy : getenv("http_proxy"); use_proxy = !ff_http_match_no_proxy(getenv("no_proxy"), c->underlying_host) && proxy_path && av_strstart(proxy_path, "http://", NULL); diff --git a/libavformat/tls.h b/libavformat/tls.h index beb19d6d55..0c52ff3be6 100644 --- a/libavformat/tls.h +++ b/libavformat/tls.h @@ -34,6 +34,7 @@ typedef struct TLSShared { int listen; char *host; + char *http_proxy; char underlying_host[200]; int numerichost; @@ -49,6 +50,7 @@ typedef struct TLSShared { {"cert_file", "Certificate file", offsetof(pstruct, options_field . cert_file), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL }, \ {"key_file", "Private key file", offsetof(pstruct, options_field . key_file), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL }, \ {"listen", "Listen for incoming connections", offsetof(pstruct, options_field . listen), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, .flags = TLS_OPTFL }, \ + {"http_proxy", "set proxy to tunnel through when using HTTPS", offsetof(pstruct, options_field . http_proxy), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL }, \ {"verifyhost", "Verify against a specific hostname", offsetof(pstruct, options_field . host), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL } int ff_tls_open_underlying(TLSShared *c, URLContext *parent, const char *uri, AVDictionary **options);