After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 726982 - [PATCH] It is not necessary to set the proxy from http_proxy environment variable
[PATCH] It is not necessary to set the proxy from http_proxy environment vari...
Status: RESOLVED FIXED
Product: rygel
Classification: Applications
Component: librygel-renderer
unspecified
Other Linux
: Normal normal
: ---
Assigned To: rygel-maint
rygel-maint
Depends on:
Blocks:
 
 
Reported: 2014-03-24 17:54 UTC by Alexander Kanavin
Modified: 2014-06-14 13:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
A patch that removes the usage of http_proxy environment variable. (1.37 KB, patch)
2014-03-24 17:58 UTC, Alexander Kanavin
committed Details | Review

Description Alexander Kanavin 2014-03-24 17:54:15 UTC
src/librygel-renderer/rygel-av-transport.vala has the following code:

        var proxy = Environment.get_variable ("http_proxy");
        if (proxy != null) {
            if (!proxy.has_prefix ("http://") &&
                !proxy.has_prefix ("https://")) {
                proxy = "http://" + proxy;
            }
            this.session = new Session.with_options (Soup.SESSION_PROXY_URI,
                                                     new Soup.URI (proxy));
        } else {
            this.session = new Session ();
        }


The above is not necessary, because libsoup now handles system proxy settings automatically (see https://developer.gnome.org/libsoup/stable/libsoup-session-porting.html ). Also, the code ignores the no_proxy environment variable, so for example playing files from localhost was not working for me. I'll attach a patch to fix this.
Comment 1 Alexander Kanavin 2014-03-24 17:58:08 UTC
Created attachment 272809 [details] [review]
A patch that removes the usage of http_proxy environment variable.
Comment 2 Jens Georg 2014-03-24 20:24:26 UTC
there was a reason for introducing http_proxy there. SoupSession already handled that when we introduced it. I have to check with Krzesmir if he remembers the reason.
Comment 3 Alexander Kanavin 2014-03-24 20:36:48 UTC
This was the original patch:

https://git.gnome.org/browse/rygel/commit/?id=cee9f398ba5d45187822b6a2f6d45c22cbead614

It adds support for a default proxy resolver (with older libsoup it had to be done explicitly) AND enforces the proxy from the environment variable. The latter is not necessary, and ignores no_proxy setting. I'm not sure how the default resolver works, but it's probably using the same environment variables anyway.
Comment 4 Jens Georg 2014-04-29 18:45:22 UTC
I think the point was to being able to override the system proxy with http_proxy env var.

I agree that ignoring the no_proxy variable is bad and that the system proxy might also set http_proxy
Comment 5 Alexander Kanavin 2014-04-29 19:25:21 UTC
I think the proxy resolver that libsoup is now using is based on libproxy, and that library has support for environment variables, including no_proxy.