search

vslavik / winsparkle

App update framework for Windows, inspired by Sparkle for macOS
http://winsparkle.org
Other
1.31k stars 267 forks source link

Allow client code to pass proxy configuration #162

Open szogun1987 opened 6 years ago

szogun1987 commented 6 years ago

There should be way to pass proxy data into connection created by Sparkle (proxy server address, and credentials). DownloadFile method of download.cpp should use InternetSetOption to pass those settings.

vslavik commented 6 years ago

Perhaps you should elaborate on why you think it "should" do that. WinSparkle (not "Sparkle") of course uses system defaults; I really don't see why it should behave any differently from the rest of the OS.

If you disagree, by all means, please submit a well-reasoned PR and I'll gladly consider it. But coming here to declare what WinSparkle "should" do with zero supporting arguments is probably not the best way to contribute.

szogun1987 commented 6 years ago

Proxy servers are used for many reasons. In some companies there is no way to download anything without authenticating via proxy. Windows by default doesn't prompt user for proxy credentials. I'm not sure if by default system-level settings for proxy are used (e.g. .Net HttpClient doesn't use system Proxy settings by default).

vslavik commented 6 years ago

Proxy servers are used for many reasons. In some companies there is no way to download anything without authenticating via proxy.

None of that explains why you think this should be configurable per-app in app-specific ways. If anything, it's an argument for using system settings — that company IT can configure as it wants. Also, proxy discovery is a thing. I'm pretty sure WinInet uses that by default, but if you think otherwise, let's verify and fix if there's anything to fix.

I'm not sure if by default system-level settings for proxy are used

Perhaps that something you should have checked first.

(e.g. .Net HttpClient doesn't use system Proxy settings by default).

WinSparkle does not use .NET or WinHTTP. It uses WinInet for this very reason.

szogun1987 commented 6 years ago

Still there is a case of invalid credentials stored in credentials store. Our app gives active feedback (by prompting user for providing credentials). Sparkle just reports network failure. Our app is able to provide valid credentials because it already have it (it establish couple network connections before).

vslavik commented 6 years ago

Still there is a case of invalid credentials stored in credentials store.

This doesn't seem to have any relation to the subject of this issue and if anything, is even more bizarre as something you'd expect client code to handle. Either this makes no sense or I don't understand your reasoning.

WinSparkle (notice, again, the name) assumes — IMHO entirely reasonably — that the Windows network environment is correctly configured and the network works. I don't think it is the job of WinSparkle — or any general-purpose application using it — to include interface for configuring OS networking.

szogun1987 commented 6 years ago

Many apps stores proxy settings (especially credentials) internally e.g. internet browsers (excluding IE), Slack client or "Skype for business". They all prompt user for credentials and replay request if 407 is returned. Our app is expected to provide same behavior.

vslavik commented 6 years ago

All right, a browser (that doesn't use native stack) and a glorified webpage bundled with said browser are hardly representative examples of "normal" apps. I thought about this some more and I think that WinSparkle's assumption that the OS has working networking stack is a reasonable one. If normal WinInet connections don't work automatically, more things are probably broken.

They all prompt user for credentials and replay request if 407

My understanding is that WinInet may show UI for proxies authentication (it's something you can explicitly disallow). If I am wrong, then WinSparkle should automatically show auth UI ([1], [2]) and a PR to that effect would be great. I just don't think that this belongs in WinSparkle API.

dittodhole commented 6 years ago

Perhaps you should elaborate on why you think it "should" do that. WinSparkle (not "Sparkle") of course uses system defaults; I really don't see why it should behave any differently from the rest of the OS.

Scratched my head for a long time, but eventually got some input from our customers:

One customer's IT department uses different proxies for browsing (where special filters apply) vs update http-requests (where a different ruleset applies). They differ regarding url and credentials, which makes a per-app proxy-configuration mandatory.

... oh dear, learning something new from our customers every day 😀