- fixed: windows http get exceptions stop cleanup.

- made premium auth errors more tidy.

src/gui/src/PremiumAuth.cpp

@@ -24,7 +24,7 @@
 // we use syntool to authenticate because Qt's http library is very
 // unreliable, and since we're writing platform specific code, use the
 // synergy code since there we can use integ tests.
-QString PremiumAuth::auth(const QString& email, const QString& password)
+QString PremiumAuth::request(const QString& email, const QString& password)
 {
 	QString program(QCoreApplication::applicationDirPath() + "/syntool");
 	QStringList args("--premium-auth");
@@ -40,7 +40,7 @@ QString PremiumAuth::auth(const QString& email, const QString& password)
 		process.write(credentials.toStdString().c_str());
 
 		if (process.waitForFinished()) {
-			return process.readLine();
+			return process.readAll();
 		}
 	}
 

src/gui/src/PremiumAuth.h

@@ -22,5 +22,5 @@
 class PremiumAuth
 {
 public:
-	QString auth(const QString& email, const QString& password);
+	QString request(const QString& email, const QString& password);
 };

src/gui/src/SetupWizard.cpp

@@ -253,13 +253,17 @@ bool SetupWizard::isPremiumLoginValid(QMessageBox& message)
 	QString password = m_pLineEditPremiumPassword->text();
 
 	PremiumAuth auth;
-	QString responseJson = auth.auth(email, password);
+	QString responseJson = auth.request(email, password);
 
-	// this feels like a lot of work, but its cheaper than getting a json
+	if (responseJson.trimmed() == "") {
-	// parsing library involved.
+		message.setText(tr("Login failed, could not communicate with server."));
-	QRegExp regex(".*\"result\":\\s*([^,}\\s]+).*");
+		message.exec();
-	if (regex.exactMatch(responseJson)) {
+		return false;
-		QString boolString = regex.cap(1);
+	}
+
+	QRegExp resultRegex(".*\"result\".*:.*(true|false).*");
+	if (resultRegex.exactMatch(responseJson)) {
+		QString boolString = resultRegex.cap(1);
 		if (boolString == "true") {
 			return true;
 		}
@@ -269,8 +273,20 @@ bool SetupWizard::isPremiumLoginValid(QMessageBox& message)
 			return false;
 		}
 	}
+	else {
+		QRegExp errorRegex(".*\"error\".*:.*\"(.+)\".*");
+		if (errorRegex.exactMatch(responseJson)) {
 
-	message.setText(tr("Login failed, an error occurred.\n\nServer response:\n\n%1").arg(responseJson.trimmed()));
+			// replace "\n" with real new lines.
+			QString error = errorRegex.cap(1).replace("\\n", "\n");
+
+			message.setText(tr("Login failed, an error occurred.\n\n%1").arg(error));
+			message.exec();
+			return false;
+		}
+	}
+
+	message.setText(tr("Login failed, an error occurred.\n\nServer response:\n\n%1").arg(responseJson));
 	message.exec();
 	return false;
 }

src/lib/arch/CArchInternetWindows.cpp

@@ -19,61 +19,91 @@
 #include "CArch.h"
 #include "Version.h"
 #include "XArchWindows.h"
+
 #include <sstream>
 #include <Wininet.h>
 
+struct CWinINetUrl {
+	CString				m_scheme;
+	CString				m_host;
+	CString				m_path;
+	INTERNET_PORT		m_port;
+	DWORD				m_flags;
+};
+
+class CWinINetRequest {
+public:
+	CWinINetRequest(const CString& url);
+	~CWinINetRequest();
+
+	CString				send();
+	void				openSession();
+	void				connect();
+	void				openRequest();
+
+private:
+	HINTERNET			m_session;
+	HINTERNET			m_connect;
+	HINTERNET			m_request;
+	CWinINetUrl			m_url;
+	bool				m_used;
+};
+
+//
+// CArchInternetWindows
+//
+
 CString
 CArchInternetWindows::get(const CString& url)
 {
-	std::stringstream userAgent;
+	CWinINetRequest request(url);
-	userAgent << "Synergy ";
+	return request.send();
-	userAgent << kVersion;
+}
 
-    HINTERNET session = InternetOpen(
+//
-		userAgent.str().c_str(),
+// CWinINetRequest
-		INTERNET_OPEN_TYPE_PRECONFIG,
+//
-		NULL, NULL, NULL);
 
-	if (session == NULL) {
+static CWinINetUrl parseUrl(const CString& url);
-		throw XArch(new XArchEvalWindows());
+
+CWinINetRequest::CWinINetRequest(const CString& url) :
+	m_session(NULL),
+	m_connect(NULL),
+	m_request(NULL),
+	m_used(false),
+	m_url(parseUrl(url))
+{
+}
+
+CWinINetRequest::~CWinINetRequest()
+{
+	if (m_request != NULL) {
+		InternetCloseHandle(m_request);
 	}
 
-	// InternetCrackUrl didn't seem to work too well, this isn't quite
+	if (m_connect != NULL) {
-	// as robust, but it should do just fine for basic URLs.
+		InternetCloseHandle(m_connect);
-	size_t schemeEnd = url.find("://");
-	size_t hostEnd = url.find('/', schemeEnd + 3);
-	CString scheme = url.substr(0, schemeEnd);
-	CString host = url.substr(schemeEnd + 3, hostEnd - (schemeEnd + 3));
-	CString path = url.substr(hostEnd);
-
-	INTERNET_PORT port = INTERNET_DEFAULT_HTTP_PORT;
-	DWORD requestFlags = 0;
-
-	if (scheme.find("https") != CString::npos) {
-		port = INTERNET_DEFAULT_HTTPS_PORT;
-		requestFlags = INTERNET_FLAG_SECURE;
 	}
 
-	HINTERNET connect = InternetConnect(
+	if (m_session != NULL) {
-		session, host.c_str(), port, NULL, NULL,
+		InternetCloseHandle(m_session);
-		INTERNET_SERVICE_HTTP, NULL, NULL);
+	}
+}
+
+CString
+CWinINetRequest::send()
+{
+	if (m_used) {
+		throw XArch("class is one time use.");
+	}
+	m_used = true;
+
+	openSession();
+	connect();
+	openRequest();
 	
-	if (connect == NULL) {
-		throw XArch(new XArchEvalWindows());
-	}
-
-	HINTERNET request = HttpOpenRequest(
-		connect, "GET", path.c_str(),
-		HTTP_VERSION, NULL,
-		NULL, requestFlags, NULL);
-
-	if (request == NULL) {
-		throw XArch(new XArchEvalWindows());
-	}
-
 	CString headers("Content-Type: text/html");
-	if (!HttpSendRequest(request, headers.c_str(), (DWORD)headers.length(), NULL, NULL)) {
+	if (!HttpSendRequest(m_request, headers.c_str(), (DWORD)headers.length(), NULL, NULL)) {
-		int error = GetLastError();
 		throw XArch(new XArchEvalWindows());
 	}
 	
@@ -81,15 +111,92 @@ CArchInternetWindows::get(const CString& url)
 	CHAR buffer[1025];
     DWORD read = 0;
 
-	while (InternetReadFile(request, buffer, sizeof(buffer) - 1, &read) && (read != 0)) {
+	while (InternetReadFile(m_request, buffer, sizeof(buffer) - 1, &read) && (read != 0)) {
 		buffer[read] = 0;
 		result << buffer;
 		read = 0;
 	}
 
-    InternetCloseHandle(request);
-    InternetCloseHandle(connect);
-    InternetCloseHandle(session);
-
 	return result.str();
 }
+
+void
+CWinINetRequest::openSession()
+{
+	std::stringstream userAgent;
+	userAgent << "Synergy ";
+	userAgent << kVersion;
+
+    m_session = InternetOpen(
+		userAgent.str().c_str(),
+		INTERNET_OPEN_TYPE_PRECONFIG,
+		NULL,
+		NULL,
+		NULL);
+
+	if (m_session == NULL) {
+		throw XArch(new XArchEvalWindows());
+	}
+}
+
+void
+CWinINetRequest::connect()
+{
+	m_connect = InternetConnect(
+		m_session,
+		m_url.m_host.c_str(),
+		m_url.m_port,
+		NULL,
+		NULL,
+		INTERNET_SERVICE_HTTP,
+		NULL,
+		NULL);
+	
+	if (m_connect == NULL) {
+		throw XArch(new XArchEvalWindows());
+	}
+}
+
+void
+CWinINetRequest::openRequest()
+{
+	m_request = HttpOpenRequest(
+		m_connect,
+		"GET",
+		m_url.m_path.c_str(),
+		HTTP_VERSION,
+		NULL,
+		NULL,
+		m_url.m_flags,
+		NULL);
+
+	if (m_request == NULL) {
+		throw XArch(new XArchEvalWindows());
+	}
+}
+
+// nb: i tried to use InternetCrackUrl here, but couldn't quite get that to
+// work. here's some (less robust) code to split the url into components.
+// this works fine with simple urls, but doesn't consider the full url spec.
+static CWinINetUrl
+parseUrl(const CString& url)
+{
+	CWinINetUrl parsed;
+
+	size_t schemeEnd = url.find("://");
+	size_t hostEnd = url.find('/', schemeEnd + 3);
+
+	parsed.m_scheme = url.substr(0, schemeEnd);
+	parsed.m_host = url.substr(schemeEnd + 3, hostEnd - (schemeEnd + 3));
+	parsed.m_path = url.substr(hostEnd);
+
+	parsed.m_port = INTERNET_DEFAULT_HTTP_PORT;
+	parsed.m_flags = 0;
+
+	if (parsed.m_scheme.find("https") != CString::npos) {
+		parsed.m_port = INTERNET_DEFAULT_HTTPS_PORT;
+		parsed.m_flags = INTERNET_FLAG_SECURE;
+	}
+
+	return parsed;
+}