hughsie / appstream-glib

This library provides objects and helper methods to help reading and writing AppStream metadata.
GNU Lesser General Public License v2.1
65 stars 103 forks source link

Port from libsoup to libcurl #441

Closed hughsie closed 2 years ago

hughsie commented 2 years ago

The former bumped ABI, and all sorts of crazy happens when you link in libappstream-glib into a process with the 'other' ABI.

It seems the universe has settled on curl as a dep; do the same.

nanonyme commented 2 years ago

Sounds fine to me. It has been repeatedly raised libcurl doesn't support autoproxy but I'm not sure if it matters in modern world. Evaluating WPAD scripts is a significant attack vector.

ximion commented 2 years ago

I think this is a good choice - I switched from Soup to Curl in libappstream a long time ago as well, and that was a great choice in retrospect.

janbrummer commented 2 years ago

Please support libsoup as libcurl does not support pac proxy. This will be a big issue for our company

hughsie commented 2 years ago

This will be a big issue for our company

Ohh? What are you using appstream-glib for out of interest?

smcv commented 2 years ago

Please support libsoup as libcurl does not support pac proxy. This will be a big issue for our company

If this is a problem for you, I think the way to use a PAC-detected proxy with libcurl would be to open a pull request that does something like this for each outgoing request:

mcatanzaro commented 2 years ago

Yes, then rinse and repeat for every application that switches from libsoup to libcurl.

Shame you cannot do this in libcurl itself, since it doesn't depend on GLib or libproxy.

hughsie commented 2 years ago

Could we move the functionality from libsoup into GLib itself?

mcatanzaro commented 2 years ago

Well GProxyResolver is already in GLib. That's where all the proxy smarts live. libsoup uses it. libcurl does not.

If you want libcurl to respect GNOME proxy settings, you've got to manually force it to use the proxy provided by GProxyResolver, basically mapping from the GLib/libsoup's understanding of proxies to libcurl's. And you have to do this before every HTTP request you perform with libcurl, since a different proxy may be required depending on what URL you are loading. Simon's post explains how to do this in more detail. It looks annoying, but not hard.

I would have just upgraded libsoup 2 -> libsoup 3 and not worry about it, but whatever.

mcatanzaro commented 2 years ago

BTW I see you use GProxyResolver and curl with fwupd already, so either (a) you've already figured this out and can copy/paste, or (b) you're doing it wrong there.

nanonyme commented 2 years ago

@smc well, in order to use PAC correctly you have different proxy per URL. So eg during redirection chain proxy might change after every redirect. But no one in corporate environments really uses like this. Using a combination of https_proxy and no_proxy can model these environments just fine and is more secure than evaluating unsigned code downloaded from the network.

mcatanzaro commented 2 years ago

BTW I see you use GProxyResolver and curl with fwupd already, so either (a) you've already figured this out and can copy/paste, or (b) you're doing it wrong there.

And looks like it's mostly (a). Very quick check of this code looks like mostly correct usage to me. I don't think you're handling SOCKS properly, though. You might need to translate the SOCKS URLs as suggested my Simon above.

mcatanzaro commented 2 years ago

socks: -> either socks4:// , socks4a:// , socks5:// or socks5h:// whichever is most appropriate

Looks like GProxyResolver tries to do this already. If we're using the libproxy-based GProxyResolver, it translates socks:// from libproxy into socks5://, then socks4a://, then socks4://. It doesn't know about socks5h:// though. But the GProxyResolver that uses GNOME settings, and the one that uses environment variables, just pass along whatever is configured. I suppose being prepared for the unexpected certainly wouldn't hurt.

smcv commented 2 years ago

If we're using the libproxy-based GProxyResolver, it translates socks:// from libproxy into socks5://, then socks4a://, then socks4://

That sounds like the documentation for g_proxy_resolver_lookup() either disagrees with the implementation or is misleading, because it talks about how "\ could be http, rtsp, socks or other proxying protocol".

mcatanzaro commented 2 years ago

That sounds like the documentation for g_proxy_resolver_lookup() either disagrees with the implementation or is misleading, because it talks about how " could be http, rtsp, socks or other proxying protocol".

Drat, yes. :/ It's arguably OK if we interpret it as "expect anything, yolo," but misleading at the very least.

mcatanzaro commented 2 years ago

So the real documentation is in GSimpleProxyResolver which specifies the behavior explicitly: "if proxy starts with "socks://", #GSimpleProxyResolver will treat it as referring to all three of the socks5, socks4a, and socks4 proxy types."

The more generic GProxyResolver can theoretically do anything, I suppose. But in practice, reasonable implementations will match GSimpleProxyResolver.

hughsie commented 2 years ago

Fixed in https://github.com/hughsie/appstream-glib/pull/443 ?