Opened 4 years ago

Closed 4 months ago

#2129 closed defect (fixed)

no_proxy environment variable implemented wrong

Reported by: divVerent Owned by:
Priority: normal Component: avformat
Version: git-master Keywords: http
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: yes

Description

Summary of the bug:
no_proxy is defined as an exclusion list; however, to ffmpeg, this variable disables proxy support entirely

How to reproduce:

% strace -fe connect ffplay "http://192.0.2.42/foo.mp3"
[pid  4727] connect(6, {sa_family=AF_INET, sin_port=htons(80), sin_addr=inet_addr("192.0.2.42")}, 16) = -1 EINPROGRESS (Operation now in progress)
# no proxy is used. good

% env http_proxy="http://192.0.2.23:8080" strace -fe connect ffplay "http://192.0.2.42/foo.mp3"
[pid  4658] connect(6, {sa_family=AF_INET, sin_port=htons(8080), sin_addr=inet_addr("192.0.2.23")}, 16) = -1 EINPROGRESS (Operation now in progress)
# proxy is properly used, good

% env http_proxy="http://192.0.2.23:8080" no_proxy="192.0.2.42" strace -fe connect ffplay "http://192.0.2.42/foo.mp3"
[pid  4837] connect(6, {sa_family=AF_INET, sin_port=htons(80), sin_addr=inet_addr("192.0.2.42")}, 16) = -1 EINPROGRESS (Operation now in progress)
# proxy is not used, good

env http_proxy="http://192.0.2.23:8080" no_proxy="192.0.2.123" strace -fe connect ffplay "http://192.0.2.42/foo.mp3"
[pid  4894] connect(6, {sa_family=AF_INET, sin_port=htons(80), sin_addr=inet_addr("192.0.2.42")}, 16) = -1 EINPROGRESS (Operation now in progress)
# proxy is not used, BAD!

Cause in the source, to be found in libavformat/tls.c and libavformat/http.c:

    proxy_path = getenv("http_proxy");
    use_proxy = (proxy_path != NULL) && !getenv("no_proxy") &&
        av_strstart(proxy_path, "http://", NULL);

Actually, the no_proxy variable would need parsing and comparing to the URL!

See here:
http://www.w3.org/Daemon/User/Proxies/ProxyClients.html
http://lynx.isc.org/lynx2.8.6/lynx2-8-6/lynx_help/keystrokes/environments.html
curl(1)
wget(1)
http://www.gnu.org/software/emacs/manual/html_node/url/Proxies.html

Change History (4)

comment:1 Changed 4 years ago by divVerent

Also, in typical setups, one plays videos from external sources more likely than from internal sources.

Also, typically, no_proxy lists the internal networks you do not need/want/can use a proxy for, and anything not listed there is the "big and wide internet", from where the videos come.

So, NOT supporting no_proxy would actually do less damage than this "half" support for it...

As for an authoritative specification, I didn't find any yet, but generally the following things seem to be common between most implementations:

  • It is a comma separated list of domain names; whitespace after the commas is allowed.
  • Wildcards are allowed and match one or more DNS labels. So, listing "*.example.org" excludes anything ending with .example.org, even multiple labels. As a special case, "*" excludes all.
  • I did not check whether *.example.org tends to more likely include example.org itself or not.
  • IP addresses tend to be matched the same way as DNS labels. So "192.168.*" matches 192.168.0.1 but also 192.168.foo.bar.com. Many implementations also support network mask entries, like "192.168.0.0/16".
  • Many implementations also allow adding a port to entries, like in "example.org:1234". When no port is specified, all ports are to be matched.
  • I am not aware of a common library or a standard to do all this.

comment:2 Changed 4 years ago by divVerent

To list some other common implementations:

libcurl: quite simple

  • Comma and space are equally-worth separators
  • no_proxy="*" disables proxies entirely
  • any domain name "example.org" matches "examle.org" as well as "*.example.org"
  • IP addresses are matched the same way; that makes including IP ranges impossible

lynx: totally simple

  • Comma and space are equally-worth separators
  • anything else is a simple suffix match (so example.org also matches wtfexample.org for example)

comment:3 Changed 4 years ago by cehoyos

  • Analyzed by developer set
  • Status changed from new to open

comment:4 Changed 4 months ago by cehoyos

  • Resolution set to fixed
  • Status changed from open to closed

Martin Storsjö's patch for this issue was merged in 03678a32 (1.2).

Note: See TracTickets for help on using tickets.