From 9da117932325faee17e0bedb95ccf60dd516a91d Mon Sep 17 00:00:00 2001 From: DasSkelett Date: Fri, 25 Oct 2019 17:33:32 +0200 Subject: [PATCH 1/5] Enable header writeback for cURL downloads --- Core/Net/Curl.cs | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/Core/Net/Curl.cs b/Core/Net/Curl.cs index 29e337a66a..e4338e016b 100644 --- a/Core/Net/Curl.cs +++ b/Core/Net/Curl.cs @@ -33,7 +33,7 @@ public static void Init() /// /// Release any resources used by libcurl. NOT THREADSAFE AT ALL. - /// Do this after all other threads are done. + /// Do this after all other threads are done. /// public static void CleanUp() { @@ -46,11 +46,12 @@ public static void CleanUp() /// /// Creates a CurlEasy object that calls the given writeback function /// when data is received. + /// Can also write back the header. /// - /// The CurlEasy obect - /// + /// The CurlEasy object + /// /// Adapted from MultiDemo.cs in the curlsharp repo - public static CurlEasy CreateEasy(string url, CurlWriteCallback wf) + public static CurlEasy CreateEasy(string url, CurlWriteCallback wf, CurlHeaderCallback hwf = null) { if (!_initComplete) { @@ -62,21 +63,15 @@ public static CurlEasy CreateEasy(string url, CurlWriteCallback wf) easy.Url = url; easy.WriteData = null; easy.WriteFunction = wf; + if (hwf != null) + { + easy.HeaderFunction = hwf; + } easy.Encoding = "deflate, gzip"; easy.FollowLocation = true; // Follow redirects easy.UserAgent = Net.UserAgentString; easy.SslVerifyPeer = true; - // ksp.sarbian.com uses a SSL cert that libcurl can't - // verify, so we skip verification. Yeah, that sucks, I know, - // but this sucks less than our previous solution that disabled - // SSL checking entirely. - - if (url.StartsWith("https://ksp.sarbian.com/")) - { - easy.SslVerifyPeer = false; - } - var caBundle = ResolveCurlCaBundle(); if (caBundle != null) { @@ -88,15 +83,16 @@ public static CurlEasy CreateEasy(string url, CurlWriteCallback wf) /// /// Creates a CurlEasy object that writes to the given stream. + /// Can call a writeback function for the header. /// - public static CurlEasy CreateEasy(string url, FileStream stream) + public static CurlEasy CreateEasy(string url, FileStream stream, CurlHeaderCallback hwf = null) { // Let's make a happy closure around this stream! return CreateEasy(url, delegate(byte[] buf, int size, int nmemb, object extraData) { stream.Write(buf, 0, size * nmemb); return size * nmemb; - }); + }, hwf); } public static CurlEasy CreateEasy(Uri url, FileStream stream) From 651bef46f227cd974a358759f554ef36b6c06095 Mon Sep 17 00:00:00 2001 From: DasSkelett Date: Fri, 25 Oct 2019 17:39:06 +0200 Subject: [PATCH 2/5] Create new NativeAndCurlDownloadFailedKraken This Kraken should be used in those cases, where we use cURL as fallback when native downloads fail. It has some nice features: The Kraken holds the response (header + body), the uRL, the previous network exceptions and the returned status code. --- Core/Net/NetAsyncDownloader.cs | 2 +- Core/Types/Kraken.cs | 34 +++++++++++++++++++++++++ Netkan/Program.cs | 5 +--- Netkan/Transformers/CurseTransformer.cs | 2 +- 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/Core/Net/NetAsyncDownloader.cs b/Core/Net/NetAsyncDownloader.cs index 9dae9b6743..94febb7103 100644 --- a/Core/Net/NetAsyncDownloader.cs +++ b/Core/Net/NetAsyncDownloader.cs @@ -154,7 +154,7 @@ private void Download(ICollection targets) } /// - /// Download all our files using the native .NET hanlders. + /// Download all our files using the native .NET handlers. /// /// The native. private void DownloadNative() diff --git a/Core/Types/Kraken.cs b/Core/Types/Kraken.cs index 384f5ef330..f63d495a7a 100644 --- a/Core/Types/Kraken.cs +++ b/Core/Types/Kraken.cs @@ -274,7 +274,41 @@ public override string ToString() { return "Uh oh, the following things went wrong when downloading...\r\n\r\n" + String.Join("\r\n", exceptions); } + } + + /// + /// We often try downloading using native .NET methods and CurlSharp as a fallback. + /// If both downloads fail, use this Kraken to combine the exceptions. + /// It assumes that both methods got an equal response. + /// Has a nice ToString() method including the response header and content. + /// + public class NativeAndCurlDownloadFailedKraken : Kraken + { + public readonly List exceptions; + public readonly string URL; + public readonly string responseHeader; + public readonly string responseContent; + public readonly int responseStatus; + + public NativeAndCurlDownloadFailedKraken(List errors, string URL, string responseHeader, string responseContent, int responseStatus) + : this($"Native and cURL download failed downloading from {URL}, status {responseStatus}", errors, URL, responseHeader, responseContent, responseStatus) + {} + public NativeAndCurlDownloadFailedKraken(string message, List errors, string URL, string responseHeader, string responseContent, int responseStatus) + : base(message) + { + exceptions = errors; + this.URL = URL; + this.responseHeader = responseHeader; + this.responseContent = responseContent; + this.responseStatus = responseStatus; + } + + public override string ToString() + { + return $"Native and cURL download failed downloading from {URL}, status {responseStatus}:\r\n" + + $"{String.Join("\r\n\r\n", exceptions)}\r\n{responseHeader}\r\n{responseContent}"; + } } /// diff --git a/Netkan/Program.cs b/Netkan/Program.cs index 892e110894..2ecd246587 100644 --- a/Netkan/Program.cs +++ b/Netkan/Program.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Net; using System.Diagnostics; using System.IO; @@ -11,9 +10,7 @@ using Newtonsoft.Json; using Newtonsoft.Json.Linq; using CKAN.NetKAN.Model; -using CKAN.NetKAN.Services; using CKAN.NetKAN.Transformers; -using CKAN.NetKAN.Validators; using CKAN.NetKAN.Processors; namespace CKAN.NetKAN @@ -106,7 +103,7 @@ public static int Main(string[] args) } catch (Exception e) { - e = e.GetBaseException() ?? e; + e = e.GetBaseException(); Log.Fatal(e.Message); diff --git a/Netkan/Transformers/CurseTransformer.cs b/Netkan/Transformers/CurseTransformer.cs index 848d5fc4df..50b4ac428c 100755 --- a/Netkan/Transformers/CurseTransformer.cs +++ b/Netkan/Transformers/CurseTransformer.cs @@ -1,6 +1,6 @@ using System; using System.Collections.Generic; -using System.Linq; +using System.Linq; using System.Text.RegularExpressions; using log4net; using Newtonsoft.Json.Linq; From 260b934cd02ff5f39622a81225e3e42aa40e8f77 Mon Sep 17 00:00:00 2001 From: DasSkelett Date: Fri, 25 Oct 2019 18:19:32 +0200 Subject: [PATCH 3/5] Restructure Net.Download and Net.DownloadText Restructure Net.Download and Net.DownloadText to allow better logging and catching of download errors. Throw `NAtiveAndCurlDownloadFailedKraken`s, as well as log more of what's happening. --- Core/Net/Net.cs | 103 +++++++++++++++-------- Netkan/Sources/Curse/CurseApi.cs | 18 ++-- Netkan/Sources/Spacedock/SpaceDockApi.cs | 14 ++- 3 files changed, 94 insertions(+), 41 deletions(-) diff --git a/Core/Net/Net.cs b/Core/Net/Net.cs index 3e2a880cb7..e44fa46d39 100644 --- a/Core/Net/Net.cs +++ b/Core/Net/Net.cs @@ -105,16 +105,16 @@ public static string Download(string url, out string etag, string filename = nul } HttpWebResponse response = ex.Response as HttpWebResponse; - if (response.StatusCode != HttpStatusCode.Redirect) + if (response?.StatusCode != HttpStatusCode.Redirect) { throw; } - return Net.Download(response.GetResponseHeader("Location"), out etag, filename, user); + return Download(response.GetResponseHeader("Location"), out etag, filename, user); } - catch (Exception ex) + catch (Exception e) { - log.InfoFormat("Download failed, trying with curlsharp..."); + log.InfoFormat("Native download failed, trying with CurlSharp..."); etag = null; try @@ -122,16 +122,37 @@ public static string Download(string url, out string etag, string filename = nul Curl.Init(); using (FileStream stream = File.OpenWrite(filename)) - using (var curl = Curl.CreateEasy(url, stream)) { - CurlCode result = curl.Perform(); - if (result != CurlCode.Ok) + string header = string.Empty; + + var client = Curl.CreateEasy(url, stream, delegate(byte[] buf, int size, int nmemb, object extraData) { - throw new Kraken("curl download of " + url + " failed with CurlCode " + result); - } - else + header += Encoding.UTF8.GetString(buf); + return size * nmemb; + }); + + using (client) { - log.Debug("curlsharp download successful"); + var result = client.Perform(); + var returnCode = client.ResponseCode; + + if (result != CurlCode.Ok || returnCode >= 300) + { + // Always log if it's an error. + log.ErrorFormat("Response from {0}:\r\n\r\n{1}\r\n{2}", url, header, "Content not logged because it is likely a file."); + + WebException curlException = + new WebException($"Curl download failed with status {returnCode}."); + throw new NativeAndCurlDownloadFailedKraken( + new List {e, curlException}, + url.ToString(), header, null, returnCode + ); + } + else + { + // Only log if debug flag is set. + log.DebugFormat("Response from {0}:\r\n\r\n{1}\r\n{2}", url, header, "Content not logged because it is likely a file."); + } } } @@ -157,9 +178,9 @@ public static string Download(string url, out string etag, string filename = nul } // Look for an exception regarding the authentication. - if (Regex.IsMatch(ex.ToString(), "The authentication or decryption has failed.")) + if (Regex.IsMatch(e.ToString(), "The authentication or decryption has failed.")) { - throw new MissingCertificateKraken("Failed downloading " + url, ex); + throw new MissingCertificateKraken("Failed downloading " + url, e); } // Not the exception we were looking for! Throw it further upwards! @@ -241,39 +262,55 @@ public static string DownloadText(Uri url, string authToken = "") agent.Headers.Add("Authorization", $"token {authToken}"); } - return agent.DownloadString(url.OriginalString); + string content = agent.DownloadString(url.OriginalString); + string header = agent.ResponseHeaders.ToString(); + + log.DebugFormat("Response from {0}:\r\n\r\n{1}\r\n{2}", url, header, content); + + return content; } catch (Exception e) { - log.InfoFormat(e.ToString()); - log.InfoFormat("Download failed, trying with curlsharp..."); + log.InfoFormat("Native download failed, trying with CurlSharp..."); - var content = string.Empty; + string content = string.Empty; + string header = string.Empty; - var client = Curl.CreateEasy(url.OriginalString, delegate (byte[] buf, int size, int nmemb, object extraData) - { - content += Encoding.UTF8.GetString(buf); - return size * nmemb; - }); + var client = Curl.CreateEasy(url.OriginalString, + delegate (byte[] buf, int size, int nmemb, object extraData) + { + content += Encoding.UTF8.GetString(buf); + return size * nmemb; + }, + delegate(byte[] buf, int size, int nmemb, object extraData) + { + header += Encoding.UTF8.GetString(buf); + return size * nmemb; + } + ); - client.SetOpt(CurlOption.FailOnError, true); - using (client) { var result = client.Perform(); var returnCode = client.ResponseCode; - if (result != CurlCode.Ok) + if (result != CurlCode.Ok || returnCode >= 300 ) { - throw new WebException( - String.Format("Curl download failed with error {0} ({1})", result, returnCode), - e - ); + // Always log if it's an error. + log.ErrorFormat("Response from {0}:\r\n\r\n{1}\r\n{2}", url, header, content); + + WebException curlException = new WebException($"Curl download failed with status {returnCode}."); + throw new NativeAndCurlDownloadFailedKraken( + new List {e, curlException}, + url.ToString(), header, content, returnCode + ); + } + else + { + // Only log if debug flag is set + log.DebugFormat("Response from {0}:\r\n\r\n{1}\r\n{2}", url, header, content); + return content; } - - log.DebugFormat("Download from {0}:\r\n\r\n{1}", url, content); - - return content; } } } diff --git a/Netkan/Sources/Curse/CurseApi.cs b/Netkan/Sources/Curse/CurseApi.cs index 465a8f9d5d..ec6f0c7f66 100755 --- a/Netkan/Sources/Curse/CurseApi.cs +++ b/Netkan/Sources/Curse/CurseApi.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Net; using CKAN.NetKAN.Services; using log4net; @@ -22,15 +22,21 @@ public CurseApi(IHttpService http) public CurseMod GetMod(string nameOrId) { - var json = Call(nameOrId); + string json; + try + { + json = Call(nameOrId); + } + catch (NativeAndCurlDownloadFailedKraken e) + { + // CurseForge returns a valid json with an error message in some cases. + json = e.responseContent; + } // Check if the mod has been removed from Curse and if it corresponds to a KSP mod. var error = JsonConvert.DeserializeObject(json); if (!string.IsNullOrWhiteSpace(error.error)) { - throw new Kraken(string.Format( - "Could not get the mod from Curse, reason: {0}.", - error.message - )); + throw new Kraken($"Could not get the mod from Curse, reason: {error.message}."); } return CurseMod.FromJson(json); } diff --git a/Netkan/Sources/Spacedock/SpaceDockApi.cs b/Netkan/Sources/Spacedock/SpaceDockApi.cs index cafd2a25df..2743f025d1 100644 --- a/Netkan/Sources/Spacedock/SpaceDockApi.cs +++ b/Netkan/Sources/Spacedock/SpaceDockApi.cs @@ -1,4 +1,5 @@ using System; +using System.Net; using System.Text.RegularExpressions; using CKAN.NetKAN.Services; using log4net; @@ -22,14 +23,23 @@ public SpacedockApi(IHttpService http) public SpacedockMod GetMod(int modId) { - var json = Call("/mod/" + modId); + string json; + try + { + json = Call("/mod/" + modId); + } + catch (NativeAndCurlDownloadFailedKraken e) + { + // SpaceDock returns a valid json with an error message in case of non 200 codes. + json = e.responseContent; + } // Check if the mod has been removed from SD. var error = JsonConvert.DeserializeObject(json); if (error.error) { - var errorMessage = string.Format("Could not get the mod from SpaceDock, reason: {0}", error.reason); + var errorMessage = $"Could not get the mod from SpaceDock, reason: {error.reason}"; throw new Kraken(errorMessage); } From a5831853c5979393310378b233576f16480771af Mon Sep 17 00:00:00 2001 From: DasSkelett Date: Fri, 25 Oct 2019 18:22:47 +0200 Subject: [PATCH 4/5] Custom NetKAN error message for GitHub API rate limit violations --- Netkan/Sources/Github/GithubApi.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Netkan/Sources/Github/GithubApi.cs b/Netkan/Sources/Github/GithubApi.cs index d0bc8e05e4..afdb586f14 100644 --- a/Netkan/Sources/Github/GithubApi.cs +++ b/Netkan/Sources/Github/GithubApi.cs @@ -102,9 +102,12 @@ private string Call(string path) { return _http.DownloadText(url, _oauthToken); } - catch (WebException webEx) + catch (NativeAndCurlDownloadFailedKraken k) { - Log.ErrorFormat("WebException while accessing {0}: {1}", url, webEx); + if (k.responseStatus == 403 && k.responseHeader.Contains("X-RateLimit-Remaining: 0")) + { + throw new Kraken("GitHub API rate limit exceeded."); + } throw; } } From e9b9811a0e939753377502f5be33bfe2e1fc1b3b Mon Sep 17 00:00:00 2001 From: DasSkelett Date: Fri, 25 Oct 2019 18:26:02 +0200 Subject: [PATCH 5/5] Custom NetKAN error message when blocked by CurseForge --- Netkan/Sources/Curse/CurseApi.cs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/Netkan/Sources/Curse/CurseApi.cs b/Netkan/Sources/Curse/CurseApi.cs index ec6f0c7f66..60e8a8c9f7 100755 --- a/Netkan/Sources/Curse/CurseApi.cs +++ b/Netkan/Sources/Curse/CurseApi.cs @@ -1,4 +1,6 @@ -using System; +using System; +using System.Diagnostics.Eventing.Reader; +using System.IO; using System.Net; using CKAN.NetKAN.Services; using log4net; @@ -48,7 +50,24 @@ public static Uri ResolveRedirect(Uri url) HttpWebRequest request = (HttpWebRequest) WebRequest.Create(redirUrl); request.AllowAutoRedirect = false; request.UserAgent = Net.UserAgentString; - HttpWebResponse response = (HttpWebResponse) request.GetResponse(); + + HttpWebResponse response; + try + { + response = (HttpWebResponse) request.GetResponse(); + } + catch (WebException e) + { + if (e.Status == WebExceptionStatus.ProtocolError) + { + response = e.Response as HttpWebResponse; + if (response?.StatusCode == HttpStatusCode.Forbidden) + { + throw new Kraken("CKAN blocked by CurseForge"); + } + } + throw; + } response.Close(); while (response.Headers["Location"] != null) {