Repository: streamlink/streamlink
Pull request #2536 (2019-07-09 10:38:59)

session: default https-proxy option to the same as http-proxy
This sets the default value of `https-proxy` to same as `http-proxy`, it can be overridden still by setting `https-proxy`. 

beardypig: Should we deprecate the `--https-proxy` option? For comparison, cURL only supports a single `--proxy` option via the command line, I don't know if the environment variables `HTTP_PROXY` and `HTTPS_PROXY` still work independently though... 
codecov[bot]: # [Codecov](https://codecov.io/gh/streamlink/streamlink/pull/2536?src=pr&el=h1) Report
> Merging [#2536](https://codecov.io/gh/streamlink/streamlink/pull/2536?src=pr&el=desc) into [master](https://codecov.io/gh/streamlink/streamlink/commit/0a3876229068a72a58d8ba79e2f7ee4dc262dd04?src=pr&el=desc) will **decrease** coverage by `0.27%`.
> The diff coverage is `0%`.


```diff
@@            Coverage Diff             @@
##           master    #2536      +/-   ##
==========================================
- Coverage   52.56%   52.29%   -0.28%     
==========================================
  Files         239      239              
  Lines       15118    15120       +2     
==========================================
- Hits         7947     7907      -40     
- Misses       7171     7213      +42
```

codecov[bot]: # [Codecov](https://codecov.io/gh/streamlink/streamlink/pull/2536?src=pr&el=h1) Report
> Merging [#2536](https://codecov.io/gh/streamlink/streamlink/pull/2536?src=pr&el=desc) into [master](https://codecov.io/gh/streamlink/streamlink/commit/0a3876229068a72a58d8ba79e2f7ee4dc262dd04?src=pr&el=desc) will **decrease** coverage by `0.27%`.
> The diff coverage is `0%`.


```diff
@@            Coverage Diff             @@
##           master    #2536      +/-   ##
==========================================
- Coverage   52.56%   52.29%   -0.28%     
==========================================
  Files         239      239              
  Lines       15118    15120       +2     
==========================================
- Hits         7947     7907      -40     
- Misses       7171     7213      +42
```

gravyboat: If we deprecate the old one we'll have to do a full dot release instead of an incremental. I don't have an issue with it if you want to go that way, but I don't think it's really needed here and don't see a reason not to support both options as long as it's noted in the code (and docs) that they operate in the same way and it exists for legacy support purposes and so it's clear to people just in case they try to set both so they won't waste time.
beardypig: Sounds good. I'm happy with the PR as is then. Unless you think there should be more in the docs to highlight it?
gravyboat: @beardypig Should a note be added to `--https-proxy` noting that you can only have one of them? It's a pretty big edge case, but if someone had settings for both and one was overwritten that could be confusing.
beardypig: Setting `--https-proxy` will still have the desired effect - it will set the proxy for `https`, but the default value will be the same as the proxy for `http`. `http-proxy` set the `https` proxy if it is not already set, and `https-proxy` sets it regardless. 

Lemme add some tests ... :)
back-to: > Should we deprecate the --https-proxy option? 

`--https-proxy` should probably be **hidden** on https://streamlink.github.io/latest/cli.html
so people should start using only `--http-proxy`
beardypig: Would we hide the option, but still have it function as it did? I think changing the help text to `argparse.SUPPRESS` would do that, the question is: is being able to set a separate https proxy useful? I’ve never found it useful... 
back-to: >  gravyboat: If we deprecate the old one we'll have to do a full dot release instead of an incremental.

maybe it could be unified to _one command_

so this won't happen for some user scripts

`streamlink: error: unrecognized arguments: --https-proxy`

> gravyboat: I don't think it's really needed here and don't see a reason not to support both options.

it is most of the time annoying to enter the same proxy with two commands,
I see this as a **cleanup** for streamlink.

>  beardypig: is being able to set a separate https proxy useful?

99% of the time not really

> beardypig: I think changing the help text to argparse.SUPPRESS

if there should be two commands, this would be an option
beardypig: True, the other option is to make https-proxy an alias of http-proxy.
back-to: or a new command `--proxy` and `--http-proxy` / `--https-proxy` as alias

beardypig: I think `--http-proxy` fits better with the other options for `http`, there might be proxies for other things - do we still support rtmp? :)
gravyboat: Restarted the [Travis build](https://travis-ci.org/streamlink/streamlink/builds/556247255) since it didn't report in for some reason even though it succeeded.
beardypig: My preference is as the PR is now, we don't lose any functionality but it makes it easier for users because you don't have to set both proxies. 
gravyboat: Broken thanks to some [Travis changes](https://travis-ci.community/t/known-issue-travis-ci-reports-expected-waiting-for-status-to-be-reported-on-the-github-status-api-but-the-status-never-arrives/1154). I've installed the Travis CI app for the org but I've only activated it on the main repo. @bastimeyer @beardypig @back-to just a heads up that I've done this in case you get an email about it.

I'm restarting the Travis build again in hopes that the check notated in that linked document will show up correctly in the branches interface because right now it's not there at all.

Patch:
diff --git a/src/streamlink/session.py b/src/streamlink/session.py
index a61ff988..93e89b5f 100644
--- a/src/streamlink/session.py
+++ b/src/streamlink/session.py
@@ -257,6 +257,9 @@ class Streamlink(object):
 
         if key == "http-proxy":
             self.http.proxies["http"] = update_scheme("http://", value)
+            if "https" not in self.http.proxies:
+                self.http.proxies["https"] = update_scheme("http://", value)
+
         elif key == "https-proxy":
             self.http.proxies["https"] = update_scheme("https://", value)
         elif key == "http-cookies":
diff --git a/src/streamlink_cli/argparser.py b/src/streamlink_cli/argparser.py
index 6351fa00..74218938 100644
--- a/src/streamlink_cli/argparser.py
+++ b/src/streamlink_cli/argparser.py
@@ -1070,7 +1070,8 @@ def build_parser():
         "--http-proxy",
         metavar="HTTP_PROXY",
         help="""
-        A HTTP proxy to use for all HTTP requests.
+        A HTTP proxy to use for all HTTP requests, by default this proxy
+        will be used for all HTTPS requests too.
 
         Example: "http://hostname:port/"
         """


Test patch:
diff --git a/tests/test_session.py b/tests/test_session.py
index e923ab53..8fcc9cd7 100644
--- a/tests/test_session.py
+++ b/tests/test_session.py
@@ -166,6 +166,37 @@ class TestSession(unittest.TestCase):
         self.assertEqual(session.localization.language.alpha2, "en")
         self.assertEqual(session.localization.language_code, "en_US")
 
+    def test_https_proxy_default(self):
+        session = Streamlink()
+        session.set_option("http-proxy", "http://testproxy.com")
+
+        self.assertEqual("http://testproxy.com", session.http.proxies['http'])
+        self.assertEqual("http://testproxy.com", session.http.proxies['https'])
+
+    def test_https_proxy_set_first(self):
+        session = Streamlink()
+        session.set_option("https-proxy", "https://testhttpsproxy.com")
+        session.set_option("http-proxy", "http://testproxy.com")
+
+        self.assertEqual("http://testproxy.com", session.http.proxies['http'])
+        self.assertEqual("https://testhttpsproxy.com", session.http.proxies['https'])
+
+    def test_https_proxy_default_override(self):
+        session = Streamlink()
+        session.set_option("http-proxy", "http://testproxy.com")
+        session.set_option("https-proxy", "https://testhttpsproxy.com")
+
+        self.assertEqual("http://testproxy.com", session.http.proxies['http'])
+        self.assertEqual("https://testhttpsproxy.com", session.http.proxies['https'])
+
+    def test_https_proxy_set_only(self):
+        session = Streamlink()
+        session.set_option("https-proxy", "https://testhttpsproxy.com")
+
+        self.assertFalse("http" in session.http.proxies)
+        self.assertEqual("https://testhttpsproxy.com", session.http.proxies['https'])
+
+
 
 if __name__ == "__main__":
     unittest.main()