From 456e56d5dcc7f5da1bd8f70a181d8df97c7567b9 Mon Sep 17 00:00:00 2001 From: Nick Bolton Date: Wed, 5 Feb 2014 16:28:29 +0000 Subject: [PATCH] - fixed: windows http get exceptions stop cleanup. - made premium auth errors more tidy. --- src/gui/src/PremiumAuth.cpp | 4 +- src/gui/src/PremiumAuth.h | 2 +- src/gui/src/SetupWizard.cpp | 30 +++- src/lib/arch/CArchInternetWindows.cpp | 199 ++++++++++++++++++++------ 4 files changed, 179 insertions(+), 56 deletions(-) diff --git a/src/gui/src/PremiumAuth.cpp b/src/gui/src/PremiumAuth.cpp index 0f4f3d83..c14db1ea 100644 --- a/src/gui/src/PremiumAuth.cpp +++ b/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(); } } diff --git a/src/gui/src/PremiumAuth.h b/src/gui/src/PremiumAuth.h index 55c56a6b..29362d4d 100644 --- a/src/gui/src/PremiumAuth.h +++ b/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); }; diff --git a/src/gui/src/SetupWizard.cpp b/src/gui/src/SetupWizard.cpp index 6753a21b..90deed5d 100644 --- a/src/gui/src/SetupWizard.cpp +++ b/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 - // parsing library involved. - QRegExp regex(".*\"result\":\\s*([^,}\\s]+).*"); - if (regex.exactMatch(responseJson)) { - QString boolString = regex.cap(1); + if (responseJson.trimmed() == "") { + message.setText(tr("Login failed, could not communicate with server.")); + message.exec(); + return false; + } + + 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; } diff --git a/src/lib/arch/CArchInternetWindows.cpp b/src/lib/arch/CArchInternetWindows.cpp index e469c827..f716fd77 100644 --- a/src/lib/arch/CArchInternetWindows.cpp +++ b/src/lib/arch/CArchInternetWindows.cpp @@ -19,61 +19,91 @@ #include "CArch.h" #include "Version.h" #include "XArchWindows.h" + #include #include +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; - userAgent << "Synergy "; - userAgent << kVersion; + CWinINetRequest request(url); + return request.send(); +} - HINTERNET session = InternetOpen( - userAgent.str().c_str(), - INTERNET_OPEN_TYPE_PRECONFIG, - NULL, NULL, NULL); +// +// CWinINetRequest +// - if (session == NULL) { - throw XArch(new XArchEvalWindows()); +static CWinINetUrl parseUrl(const CString& url); + +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 - // as robust, but it should do just fine for basic URLs. - 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; + if (m_connect != NULL) { + InternetCloseHandle(m_connect); } - HINTERNET connect = InternetConnect( - session, host.c_str(), port, NULL, NULL, - INTERNET_SERVICE_HTTP, NULL, NULL); + if (m_session != NULL) { + InternetCloseHandle(m_session); + } +} + +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)) { - int error = GetLastError(); + if (!HttpSendRequest(m_request, headers.c_str(), (DWORD)headers.length(), NULL, NULL)) { 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; +}