BUG-1708: This change is just an improvement which writes the web-request output in case of errors. This should help to find the problem. Also cleaned the code a bit.

2025-07-16 10:03:44 -07:00 · 2014-11-28 16:48:05 +01:00 · 2014-11-28 16:48:05 +01:00 · b9e86d4af7
commit b9e86d4af7
parent 0e2d9cda61
4 changed files with 108 additions and 91 deletions
--- a/GreenshotPicasaPlugin/PicasaUtils.cs
+++ b/GreenshotPicasaPlugin/PicasaUtils.cs
@ -30,8 +30,9 @@ namespace GreenshotPicasaPlugin {
 	/// Description of PicasaUtils.
 	/// </summary>
 	public class PicasaUtils {
+		private const string GoogleAccountUri = "https://www.google.com/accounts/";
 		private static readonly log4net.ILog LOG = log4net.LogManager.GetLogger(typeof(PicasaUtils));
-		private static PicasaConfiguration config = IniConfig.GetIniSection<PicasaConfiguration>();
+		private static readonly PicasaConfiguration Config = IniConfig.GetIniSection<PicasaConfiguration>();

 		private PicasaUtils() {
 		}
@ -39,17 +40,20 @@ namespace GreenshotPicasaPlugin {
 		/// <summary>
 		/// Do the actual upload to Picasa
 		/// </summary>
-		/// <param name="imageData">byte[] with image data</param>
+		/// <param name="surfaceToUpload">Image to upload</param>
+		/// <param name="outputSettings"></param>
+		/// <param name="title"></param>
+		/// <param name="filename"></param>
 		/// <returns>PicasaResponse</returns>
 		public static string UploadToPicasa(ISurface surfaceToUpload, SurfaceOutputSettings outputSettings, string title, string filename) {
 			OAuthSession oAuth = new OAuthSession(PicasaCredentials.ConsumerKey, PicasaCredentials.ConsumerSecret);
 			oAuth.BrowserSize = new Size(1020, 590);
-			oAuth.AccessTokenUrl =  "https://www.google.com/accounts/OAuthGetAccessToken";
-			oAuth.AuthorizeUrl =	"https://www.google.com/accounts/OAuthAuthorizeToken";
-			oAuth.RequestTokenUrl = "https://www.google.com/accounts/OAuthGetRequestToken";
+			oAuth.AccessTokenUrl =  GoogleAccountUri + "OAuthGetAccessToken";
+			oAuth.AuthorizeUrl =	GoogleAccountUri + "OAuthAuthorizeToken";
+			oAuth.RequestTokenUrl = GoogleAccountUri + "OAuthGetRequestToken";
 			oAuth.LoginTitle = "Picasa authorization";
-			oAuth.Token = config.PicasaToken;
-			oAuth.TokenSecret = config.PicasaTokenSecret;
+			oAuth.Token = Config.PicasaToken;
+			oAuth.TokenSecret = Config.PicasaTokenSecret;
 			oAuth.RequestTokenParameters.Add("scope", "https://picasaweb.google.com/data/");
 			oAuth.RequestTokenParameters.Add("xoauth_displayname", "Greenshot");
 			if (string.IsNullOrEmpty(oAuth.Token)) {
@ -57,10 +61,10 @@ namespace GreenshotPicasaPlugin {
 					return null;
 				}
 				if (!string.IsNullOrEmpty(oAuth.Token)) {
-					config.PicasaToken = oAuth.Token;
+					Config.PicasaToken = oAuth.Token;
 				}
 				if (!string.IsNullOrEmpty(oAuth.TokenSecret)) {
-					config.PicasaTokenSecret = oAuth.TokenSecret;
+					Config.PicasaTokenSecret = oAuth.TokenSecret;
 				}
 				IniConfig.Save();
 			}
@ -74,10 +78,10 @@ namespace GreenshotPicasaPlugin {
 				throw;
 			} finally {
 				if (!string.IsNullOrEmpty(oAuth.Token)) {
-					config.PicasaToken = oAuth.Token;
+					Config.PicasaToken = oAuth.Token;
 				}
 				if (!string.IsNullOrEmpty(oAuth.TokenSecret)) {
-					config.PicasaTokenSecret = oAuth.TokenSecret;
+					Config.PicasaTokenSecret = oAuth.TokenSecret;
 				}
 			}
 		}
@ -93,13 +97,14 @@ namespace GreenshotPicasaPlugin {
 				if(nodes.Count > 0) {
 					string url = null;
 					foreach(XmlNode node in nodes) {
-						url = node.Attributes["href"].Value;
-						string rel = node.Attributes["rel"].Value;
-						// Pictures with rel="http://schemas.google.com/photos/2007#canonical" are the direct link
-						if (rel != null && rel.EndsWith("canonical")) {
-							break;
+						if (node.Attributes != null) {
+							url = node.Attributes["href"].Value;
+							string rel = node.Attributes["rel"].Value;
+							// Pictures with rel="http://schemas.google.com/photos/2007#canonical" are the direct link
+							if (rel != null && rel.EndsWith("canonical")) {
+								break;
+							}
 						}
-						
 					}
 					return url;
 				}
--- a/GreenshotPlugin/Core/NetworkHelper.cs
+++ b/GreenshotPlugin/Core/NetworkHelper.cs
@ -49,12 +49,12 @@ namespace GreenshotPlugin.Core {
 			};
 		}
 		/// <summary>
-		/// Download a url response as string
+		/// Download a uri response as string
 		/// </summary>
-		/// <param name=url">An Uri to specify the download location</param>
+		/// <param name="uri">An Uri to specify the download location</param>
 		/// <returns>string with the file content</returns>
-		public static string GetAsString(Uri url) {
-			HttpWebRequest webRequest = (HttpWebRequest)CreateWebRequest(url);
+		public static string GetAsString(Uri uri) {
+			HttpWebRequest webRequest = (HttpWebRequest)CreateWebRequest(uri);
 			webRequest.Method = "GET";
 			webRequest.KeepAlive = true;
 			webRequest.Credentials = CredentialCache.DefaultCredentials;
@ -84,7 +84,7 @@ namespace GreenshotPlugin.Core {
 		}

 		/// <summary>
-		/// Download the url to Bitmap
+		/// Download the uri to Bitmap
 		/// </summary>
 		/// <param name="baseUri"></param>
 		/// <returns>Bitmap</returns>
@ -212,7 +212,7 @@ namespace GreenshotPlugin.Core {
 		/// <returns>Dictionary<string, string></returns>
 		public static IDictionary<string, string> ParseQueryString(string s) {
 			IDictionary<string, string> parameters = new SortedDictionary<string, string>();
-			// remove anything other than query string from url
+			// remove anything other than query string from uri
 			if (s.Contains("?")) {
 				s = s.Substring(s.IndexOf('?') + 1);
 			}
@ -318,18 +318,30 @@ namespace GreenshotPlugin.Core {
 		/// </summary>
 		/// <param name="webRequest">The request object.</param>
 		/// <returns>The response data.</returns>
+		/// TODO: This method should handle the StatusCode better!
 		public static string GetResponse(HttpWebRequest webRequest) {
-			string responseData;
+			string responseData = null;
 			try {
 				HttpWebResponse response = (HttpWebResponse)webRequest.GetResponse();
-				using (StreamReader reader = new StreamReader(response.GetResponseStream(), true)) {
-					responseData = reader.ReadToEnd();
+				LOG.InfoFormat("Response status: {0}", response.StatusCode);
+				bool isHttpError = (int) response.StatusCode >= 300;
+				Stream responseStream = response.GetResponseStream();
+				if (responseStream != null) {
+					using (StreamReader reader = new StreamReader(responseStream, true)) {
+						responseData = reader.ReadToEnd();
+					}
+					if (isHttpError) {
+						LOG.ErrorFormat("HTTP error {0} with content: {1}", response.StatusCode, responseData);
+					}
 				}
-				LOG.DebugFormat("Response status: {0}", response.StatusCode);
 			} catch (WebException e) {
 				HttpWebResponse response = (HttpWebResponse)e.Response;
 				using (Stream responseStream = response.GetResponseStream()) {
-					LOG.ErrorFormat("HTTP error {0} with content: {1}", response.StatusCode, new StreamReader(responseStream, true).ReadToEnd());
+					if (responseStream != null) {
+						LOG.ErrorFormat("HTTP error {0} with content: {1}", response.StatusCode, new StreamReader(responseStream, true).ReadToEnd());
+					} else {
+						LOG.ErrorFormat("HTTP error {0}", response.StatusCode);
+					}
 				}
 				throw;
 			}
--- a/GreenshotPlugin/Core/OAuthHelper.cs
+++ b/GreenshotPlugin/Core/OAuthHelper.cs
@ -18,6 +18,7 @@
 * You should have received a copy of the GNU General Public License
 * along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
+
 using System;
 using System.Collections.Generic;
 using System.Drawing;
@ -72,14 +73,14 @@ namespace GreenshotPlugin.Core {

 		private string _userAgent = "Greenshot";
 		private string _callbackUrl = "http://getgreenshot.org";
-		private bool checkVerifier = true;
-		private bool useHTTPHeadersForAuthorization = true;
-		private IDictionary<string, string> accessTokenResponseParameters = null;
-		private IDictionary<string, string> requestTokenResponseParameters = null;
-		private IDictionary<string, object> requestTokenParameters = new Dictionary<string, object>();
+		private bool _checkVerifier = true;
+		private bool _useHttpHeadersForAuthorization = true;
+		private IDictionary<string, string> _accessTokenResponseParameters;
+		private IDictionary<string, string> _requestTokenResponseParameters;
+		private readonly IDictionary<string, object> _requestTokenParameters = new Dictionary<string, object>();
 		
 		public IDictionary<string, object> RequestTokenParameters {
-			get { return requestTokenParameters; }
+			get { return _requestTokenParameters; }
 		}

 		/// <summary>
@ -87,7 +88,7 @@ namespace GreenshotPlugin.Core {
 		/// </summary>
 		public IDictionary<string, string> AccessTokenResponseParameters {
 			get {
-				return accessTokenResponseParameters;
+				return _accessTokenResponseParameters;
 			}
 		}
 		/// <summary>
@ -95,15 +96,15 @@ namespace GreenshotPlugin.Core {
 		/// </summary>
 		public IDictionary<string, string> RequestTokenResponseParameters {
 			get {
-				return requestTokenResponseParameters;
+				return _requestTokenResponseParameters;
 			}
 		}
-		private string consumerKey;
-		private string consumerSecret;
+		private readonly string _consumerKey;
+		private readonly string _consumerSecret;

 		// default browser size
 		private Size _browserSize = new Size(864, 587);
-		private string loginTitle = "Authorize Greenshot access";
+		private string _loginTitle = "Authorize Greenshot access";

 		#region PublicProperties
 		public HTTPMethod RequestTokenMethod {
@ -162,10 +163,10 @@ namespace GreenshotPlugin.Core {
 		}
 		public bool CheckVerifier {
 			get {
-				return checkVerifier;
+				return _checkVerifier;
 			}
 			set {
-				checkVerifier = value;
+				_checkVerifier = value;
 			}
 		}

@ -180,18 +181,18 @@ namespace GreenshotPlugin.Core {

 		public string LoginTitle {
 			get {
-				return loginTitle;
+				return _loginTitle;
 			}
 			set {
-				loginTitle = value;
+				_loginTitle = value;
 			}
 		}
 		public bool UseHTTPHeadersForAuthorization {
 			get {
-				return useHTTPHeadersForAuthorization;
+				return _useHttpHeadersForAuthorization;
 			}
 			set {
-				useHTTPHeadersForAuthorization = value;
+				_useHttpHeadersForAuthorization = value;
 			}
 		}

@ -208,8 +209,8 @@ namespace GreenshotPlugin.Core {
 		/// <param name="consumerKey">"Public" key for the encoding. When using RSASHA1 this is the path to the private key file</param>
 		/// <param name="consumerSecret">"Private" key for the encoding. when usin RSASHA1 this is the password for the private key file</param>
 		public OAuthSession(string consumerKey, string consumerSecret) {
-			this.consumerKey = consumerKey;
-			this.consumerSecret = consumerSecret;
+			_consumerKey = consumerKey;
+			_consumerSecret = consumerSecret;
 			UseMultipartFormData = true;
 			RequestTokenMethod = HTTPMethod.GET;
 			AccessTokenMethod = HTTPMethod.GET;
@ -307,66 +308,63 @@ namespace GreenshotPlugin.Core {
 		/// <summary>
 		/// Get the request token using the consumer key and secret.  Also initializes tokensecret
 		/// </summary>
-		/// <returns>The request token.</returns>
-		private String getRequestToken() {
-			string ret = null;
+		private void GetRequestToken() {
 			IDictionary<string, object> parameters = new Dictionary<string, object>();
-			foreach(var value in requestTokenParameters) {
+			foreach(var value in _requestTokenParameters) {
 				parameters.Add(value);
 			}
 			Sign(RequestTokenMethod, RequestTokenUrl, parameters);
 			string response = MakeRequest(RequestTokenMethod, RequestTokenUrl, null, parameters, null);
-			if (response != null && response.Length > 0) {
+			if (!string.IsNullOrEmpty(response)) {
 				response = NetworkHelper.UrlDecode(response);
 				LOG.DebugFormat("Request token response: {0}", response);
-				requestTokenResponseParameters = NetworkHelper.ParseQueryString(response);
-				if (requestTokenResponseParameters.ContainsKey(OAUTH_TOKEN_KEY)) {
-					Token = requestTokenResponseParameters[OAUTH_TOKEN_KEY];
-					TokenSecret = requestTokenResponseParameters[OAUTH_TOKEN_SECRET_KEY];
-					ret = Token;
+				_requestTokenResponseParameters = NetworkHelper.ParseQueryString(response);
+				string value;
+				if (_requestTokenResponseParameters.TryGetValue(OAUTH_TOKEN_KEY, out value)) {
+					Token = value;
+					TokenSecret = _requestTokenResponseParameters[OAUTH_TOKEN_SECRET_KEY];
 				}
 			}
-			return ret;
 		}

 		/// <summary>
 		/// Authorize the token by showing the dialog
 		/// </summary>
 		/// <returns>The request token.</returns>
-		private String getAuthorizeToken() {
+		private String GetAuthorizeToken() {
 			if (string.IsNullOrEmpty(Token)) {
 				Exception e = new Exception("The request token is not set");
 				throw e;
 			}
-			LOG.DebugFormat("Opening AuthorizationLink: {0}", authorizationLink);
-			OAuthLoginForm oAuthLoginForm = new OAuthLoginForm(LoginTitle, BrowserSize, authorizationLink, CallbackUrl);
+			LOG.DebugFormat("Opening AuthorizationLink: {0}", AuthorizationLink);
+			OAuthLoginForm oAuthLoginForm = new OAuthLoginForm(LoginTitle, BrowserSize, AuthorizationLink, CallbackUrl);
 			oAuthLoginForm.ShowDialog();
 			if (oAuthLoginForm.isOk) {
 				if (oAuthLoginForm.CallbackParameters != null) {
-					if (oAuthLoginForm.CallbackParameters.ContainsKey(OAUTH_TOKEN_KEY)) {
-						Token = oAuthLoginForm.CallbackParameters[OAUTH_TOKEN_KEY];
+					string tokenValue;
+					if (oAuthLoginForm.CallbackParameters.TryGetValue(OAUTH_TOKEN_KEY, out tokenValue)) {
+						Token = tokenValue;
 					}
-					if (oAuthLoginForm.CallbackParameters.ContainsKey(OAUTH_VERIFIER_KEY)) {
-						Verifier = oAuthLoginForm.CallbackParameters[OAUTH_VERIFIER_KEY];
+					string verifierValue;
+					if (oAuthLoginForm.CallbackParameters.TryGetValue(OAUTH_VERIFIER_KEY, out verifierValue)) {
+						Verifier = verifierValue;
 					}
 				}
 			}
 			if (CheckVerifier) {
 				if (!string.IsNullOrEmpty(Verifier)) {
 					return Token;
-				} else {
-					return null;
 				}
-			} else {
-				return Token;
+				return null;
 			}
+			return Token;
 		}

 		/// <summary>
 		/// Get the access token
 		/// </summary>
 		/// <returns>The access token.</returns>		
-		private String getAccessToken() {
+		private String GetAccessToken() {
 			if (string.IsNullOrEmpty(Token) || (CheckVerifier && string.IsNullOrEmpty(Verifier))) {
 				Exception e = new Exception("The request token and verifier were not set");
 				throw e;
@ -375,15 +373,17 @@ namespace GreenshotPlugin.Core {
 			IDictionary<string, object> parameters = new Dictionary<string, object>();
 			Sign(AccessTokenMethod, AccessTokenUrl, parameters);
 			string response = MakeRequest(AccessTokenMethod, AccessTokenUrl, null, parameters, null);
-			if (response != null && response.Length > 0) {
+			if (!string.IsNullOrEmpty(response)) {
 				response = NetworkHelper.UrlDecode(response);
 				LOG.DebugFormat("Access token response: {0}", response);
-				accessTokenResponseParameters = NetworkHelper.ParseQueryString(response);
-				if (accessTokenResponseParameters.ContainsKey(OAUTH_TOKEN_KEY) && accessTokenResponseParameters[OAUTH_TOKEN_KEY] != null) {
-					Token = accessTokenResponseParameters[OAUTH_TOKEN_KEY];
+				_accessTokenResponseParameters = NetworkHelper.ParseQueryString(response);
+				string tokenValue;
+				if (_accessTokenResponseParameters.TryGetValue(OAUTH_TOKEN_KEY, out tokenValue) && tokenValue != null) {
+					Token = tokenValue;
 				}
-				if (accessTokenResponseParameters.ContainsKey(OAUTH_TOKEN_SECRET_KEY) && accessTokenResponseParameters[OAUTH_TOKEN_SECRET_KEY] != null) {
-					TokenSecret = accessTokenResponseParameters[OAUTH_TOKEN_SECRET_KEY];
+				string secretValue;
+				if (_accessTokenResponseParameters.TryGetValue(OAUTH_TOKEN_SECRET_KEY, out secretValue) && secretValue != null) {
+					TokenSecret = secretValue;
 				}
 			}

@ -400,18 +400,18 @@ namespace GreenshotPlugin.Core {
 			Verifier = null;
 			LOG.Debug("Creating Token");
 			try {
-				getRequestToken();
+				GetRequestToken();
 			} catch (Exception ex) {
 				LOG.Error(ex);
 				throw new NotSupportedException("Service is not available: " + ex.Message);
 			}
-			if (string.IsNullOrEmpty(getAuthorizeToken())) {
+			if (string.IsNullOrEmpty(GetAuthorizeToken())) {
 				LOG.Debug("User didn't authenticate!");
 				return false;
 			}
 			try {
 				Thread.Sleep(1000);
-				return getAccessToken() != null;
+				return GetAccessToken() != null;
 			} catch (Exception ex) {
 				LOG.Error(ex);
 				throw;
@ -422,7 +422,7 @@ namespace GreenshotPlugin.Core {
 		/// Get the link to the authorization page for this application.
 		/// </summary>
 		/// <returns>The url with a valid request token, or a null string.</returns>
-		private string authorizationLink {
+		private string AuthorizationLink {
 			get {
 				return AuthorizeUrl + "?" + OAUTH_TOKEN_KEY + "=" + Token + "&" + OAUTH_CALLBACK_KEY + "=" + UrlEncode3986(CallbackUrl);
 			}
@ -578,8 +578,8 @@ namespace GreenshotPlugin.Core {
 					parameters.Add(OAUTH_SIGNATURE_METHOD_KEY, HMACSHA1SignatureType);
 					break;
 			}
-			parameters.Add(OAUTH_CONSUMER_KEY_KEY, consumerKey);
-			if (CallbackUrl != null && RequestTokenUrl != null && requestURL.ToString().StartsWith(RequestTokenUrl)) {
+			parameters.Add(OAUTH_CONSUMER_KEY_KEY, _consumerKey);
+			if (CallbackUrl != null && RequestTokenUrl != null && requestURL.StartsWith(RequestTokenUrl)) {
 				parameters.Add(OAUTH_CALLBACK_KEY, CallbackUrl);
 			}
 			if (!string.IsNullOrEmpty(Verifier)) {
@ -590,13 +590,13 @@ namespace GreenshotPlugin.Core {
 			}
 			signatureBase.Append(UrlEncode3986(GenerateNormalizedParametersString(parameters)));
 			LOG.DebugFormat("Signature base: {0}", signatureBase);
-			string key = string.Format(CultureInfo.InvariantCulture, "{0}&{1}", UrlEncode3986(consumerSecret), string.IsNullOrEmpty(TokenSecret) ? string.Empty : UrlEncode3986(TokenSecret));
+			string key = string.Format(CultureInfo.InvariantCulture, "{0}&{1}", UrlEncode3986(_consumerSecret), string.IsNullOrEmpty(TokenSecret) ? string.Empty : UrlEncode3986(TokenSecret));
 			switch (SignatureType) {
 				case OAuthSignatureTypes.RSASHA1:
 					// Code comes from here: http://www.dotnetfunda.com/articles/article1932-rest-service-call-using-oauth-10-authorization-with-rsa-sha1.aspx
 					// Read the .P12 file to read Private/Public key Certificate
-					string certFilePath = consumerKey; // The .P12 certificate file path Example: "C:/mycertificate/MCOpenAPI.p12
-					string password = consumerSecret; // password to read certificate .p12 file
+					string certFilePath = _consumerKey; // The .P12 certificate file path Example: "C:/mycertificate/MCOpenAPI.p12
+					string password = _consumerSecret; // password to read certificate .p12 file
 					// Read the Certification from .P12 file.
 					X509Certificate2 cert = new X509Certificate2(certFilePath.ToString(), password);
 					// Retrieve the Private key from Certificate.
@ -644,7 +644,7 @@ namespace GreenshotPlugin.Core {
 			if (parameters == null) {
 				throw new ArgumentNullException("parameters");
 			}
-			IDictionary<string, object> requestParameters = null;
+			IDictionary<string, object> requestParameters;
 			// Add oAuth values as HTTP headers, if this is allowed
 			StringBuilder authHeader = null;
 			if (UseHTTPHeadersForAuthorization) {
@ -679,8 +679,8 @@ namespace GreenshotPlugin.Core {
 			webRequest.Timeout = 20000;

 			if (UseHTTPHeadersForAuthorization && authHeader != null) {
-				LOG.DebugFormat("Authorization: OAuth {0}", authHeader.ToString());
-				webRequest.Headers.Add("Authorization: OAuth " + authHeader.ToString());
+				LOG.DebugFormat("Authorization: OAuth {0}", authHeader);
+				webRequest.Headers.Add("Authorization: OAuth " + authHeader);
 			}

 			if (headers != null) {
@ -689,7 +689,7 @@ namespace GreenshotPlugin.Core {
 				}
 			}

-			if ((HTTPMethod.POST == method || HTTPMethod.PUT == method) && postData == null && requestParameters != null && requestParameters.Count > 0) {
+			if ((HTTPMethod.POST == method || HTTPMethod.PUT == method) && postData == null && requestParameters.Count > 0) {
 				if (UseMultipartFormData) {
 					NetworkHelper.WriteMultipartFormData(webRequest, requestParameters);
 				} else {
@ -718,7 +718,7 @@ namespace GreenshotPlugin.Core {
 				webRequest.ContentLength = 0;
 			}

-			string responseData = null;
+			string responseData;
 			try {
 				responseData = NetworkHelper.GetResponse(webRequest);
 				LOG.DebugFormat("Response: {0}", responseData);
--- a/GreenshotPlugin/IniFile/IniConfig.cs
+++ b/GreenshotPlugin/IniFile/IniConfig.cs
@ -209,7 +209,7 @@ namespace Greenshot.IniFile {
 			}
 			if (iniFilePath == null) {
 				// check if file is in the same location as started from, if this is the case
-				// we will use this file instead of the Applicationdate folder
+				// we will use this file instead of the ApplicationData folder
 				// Done for Feature Request #2741508
 				iniFilePath = Path.Combine(applicationStartupPath, configFilename);
 				if (!File.Exists(iniFilePath)) {