Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate network download code #1294

Merged
merged 1 commit into from
Jul 31, 2015
Merged

Consolidate network download code #1294

merged 1 commit into from
Jul 31, 2015

Conversation

dbent
Copy link
Member

@dbent dbent commented Jul 16, 2015

This consolidates much of our network download logic into Net, and standardizes behavior such that:

  • We always try to use a WebClient first
  • If that fails, we attempt to use cURL

Also, moves the cURL CA file resolution logic into Curl so that everything that uses it benefits from it.

@dbent dbent force-pushed the topic/net_consolidate branch from f08d280 to f32919b Compare July 30, 2015 20:21
@pjf
Copy link
Member

pjf commented Jul 31, 2015

Either I'm mis-reading the logs, or KerbalStuff is working fine with the default WebClient under Linux, which it didn't use to do and was one of the big motivators for having curlsharp support in the first place.

Trying to find more ways to test this. :)

@dbent
Copy link
Member Author

dbent commented Jul 31, 2015

I was slightly confused as well. What the code used to do was always try with CurlSharp, then catch a DllNotFoundException and then re-attempt with WebClient. So it is using CurlSharp on Linux, but silently failing on Windows (unless you have cURL installed) and being retried.

@dbent
Copy link
Member Author

dbent commented Jul 31, 2015

And to add, my changes standardizes the behavior so that it always attempts with WebClient first, then falls back to CurlSharp.

@pjf
Copy link
Member

pjf commented Jul 31, 2015

Looks like kerbalstuff supports a number of TLS 1.0 ciphers now, and I'm sure their absence was the reason we jumped to libcurl in the first place.

So I'm still testing, trying to find something that forces a failover to libcurl. :)

@pjf
Copy link
Member

pjf commented Jul 31, 2015

This seems to be working fine with me forcing errors. My only real concern is if mono's WebClient class ever results in a timeout due to bugs, which my head says shouldn't happen, but which my gut says has already happened.

However I can't find evidence to support my gut, so I'll be merging this shortly. :)

@pjf pjf merged commit f32919b into master Jul 31, 2015
pjf added a commit that referenced this pull request Jul 31, 2015
* origin/topic/net_consolidate:
  Consolidate network download code
pjf added a commit that referenced this pull request Jul 31, 2015
@pjf pjf removed the pull request label Jul 31, 2015
@dbent dbent deleted the topic/net_consolidate branch July 31, 2015 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants