diff mbox series

[FFmpeg-devel,v3] avformat/http, tls: honor http_proxy command line variable for HTTPS

Message ID 20200823121459.GT4729@sunshine.barsnick.net
State New
Headers show
Series [FFmpeg-devel,v3] avformat/http, tls: honor http_proxy command line variable for HTTPS | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Moritz Barsnick Aug. 23, 2020, 12:14 p.m. UTC 
Hej igen,

On Fri, Aug 21, 2020 at 12:19:06 +0300, Martin Storsjö wrote:
> LGTM, with one small nit:
[...]
> >      {"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 }
>
> I'd remove the "when using HTTPS" bit here.

Done. I also chose to make the capitalization consistent, and move the
option down, as "verifyhost" is related to the certificate options (and
should thus perhaps even be above "listen").

I have come to realize that the httpproxy protocol could apply to any
protocol with underlying tcp, but that's a different story.

Thanks,
Moritz
From e2d93c4f5ad186a5277c8ae346948c588f2998ca Mon Sep 17 00:00:00 2001
From: Moritz Barsnick <barsnick@gmx.net>
Date: Sun, 23 Aug 2020 13:53:39 +0200
Subject: [PATCH] avformat/http,tls: honor http_proxy command line variable for
 HTTPS

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.

Fixes #7223.

Signed-off-by: Moritz Barsnick <barsnick@gmx.net>
---
 doc/protocols.texi | 4 ++++
 libavformat/http.c | 6 ++++++
 libavformat/tls.c  | 2 +-
 libavformat/tls.h  | 4 +++-
 4 files changed, 14 insertions(+), 2 deletions(-)

--
2.26.2

Comments

Martin Storsjö Aug. 23, 2020, 7:50 p.m. UTC | #1 
On Sun, 23 Aug 2020, Moritz Barsnick wrote:

> Hej igen,
>
> On Fri, Aug 21, 2020 at 12:19:06 +0300, Martin Storsjö wrote:
>> LGTM, with one small nit:
> [...]
>>>      {"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 }
>>
>> I'd remove the "when using HTTPS" bit here.
>
> Done. I also chose to make the capitalization consistent, and move the
> option down, as "verifyhost" is related to the certificate options (and
> should thus perhaps even be above "listen").

Thanks, this version LGTM.

// Martin
Moritz Barsnick March 14, 2021, 1:55 a.m. UTC | #2 
On Sun, Aug 23, 2020 at 22:50:18 +0300, Martin Storsjö wrote:
> > On Fri, Aug 21, 2020 at 12:19:06 +0300, Martin Storsjö wrote:
> > > LGTM, with one small nit:
> > [...]
> > > >      {"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 }
> > >
> > > I'd remove the "when using HTTPS" bit here.
> >
> > Done. I also chose to make the capitalization consistent, and move the
> > option down, as "verifyhost" is related to the certificate options (and
> > should thus perhaps even be above "listen").
>
> Thanks, this version LGTM.

Ping. Is this okay to apply? Could someone please?

Thanks,
Moritz
Marton Balint March 19, 2021, 9:46 p.m. UTC | #3 
On Sun, 14 Mar 2021, Moritz Barsnick wrote:

> On Sun, Aug 23, 2020 at 22:50:18 +0300, Martin Storsjö wrote:
>> > On Fri, Aug 21, 2020 at 12:19:06 +0300, Martin Storsjö wrote:
>> > > LGTM, with one small nit:
>> > [...]
>> > > >      {"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 }
>> > >
>> > > I'd remove the "when using HTTPS" bit here.
>> >
>> > Done. I also chose to make the capitalization consistent, and move the
>> > option down, as "verifyhost" is related to the certificate options (and
>> > should thus perhaps even be above "listen").
>>
>> Thanks, this version LGTM.
>
> Ping. Is this okay to apply? Could someone please?

Ok, will apply.

Thanks,
Marton
diff mbox series

Patch

diff --git a/doc/protocols.texi b/doc/protocols.texi
index 7b3df96fda..603943023c 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -1708,6 +1708,10 @@  A file containing the private key for the certificate.
 If enabled, listen for connections on the provided port, and assume
 the server role in the handshake instead of the client role.

+@item http_proxy
+The HTTP proxy to tunnel through, e.g. @code{http://example.com:1234}.
+The proxy must support the CONNECT method.
+
 @end table

 Example command lines:
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..6c6aa01a9a 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,7 +50,8 @@  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 }, \
-    {"verifyhost", "Verify against a specific hostname",  offsetof(pstruct, options_field . host),      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 }, \
+    {"http_proxy", "Set proxy to tunnel through",         offsetof(pstruct, options_field . http_proxy), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL }

 int ff_tls_open_underlying(TLSShared *c, URLContext *parent, const char *uri, AVDictionary **options);