Merge lp:~dobey/pay-ui/escape-signed-query into lp:pay-ui

Proposed by dobey
Status: Merged
Approved by: dobey
Approved revision: 136
Merged at revision: 138
Proposed branch: lp:~dobey/pay-ui/escape-signed-query
Merge into: lp:pay-ui
Diff against target: 76 lines (+20/-3)
3 files modified
backend/modules/payui/network.cpp (+11/-3)
backend/modules/payui/network.h (+1/-0)
backend/tests/test_network.cpp (+8/-0)
To merge this branch: bzr merge lp:~dobey/pay-ui/escape-signed-query
Reviewer Review Type Date Requested Status
Charles Kerr (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+276191@code.launchpad.net

Commit message

Encode slashes in URL query strings when signing as a query.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

I'm not sure that this does what we need. QUrl::setQuery appears to only check StrictMode in order to clear out the query if validation fails (http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/io/qurl.cpp#n2618) so instead of encoding the slash in auth_signature wouldn't this just clear the query altogether?

review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'backend/modules/payui/network.cpp'
2--- backend/modules/payui/network.cpp 2015-10-01 13:54:35 +0000
3+++ backend/modules/payui/network.cpp 2015-11-23 21:07:16 +0000
4@@ -130,6 +130,13 @@
5 return url.toString().replace(regexp, "/");
6 }
7
8+QString Network::encodeQuerySlashes(const QString& query)
9+{
10+ static QRegExp regexp("\\b\\/");
11+ QString temp(query);
12+ return temp.replace(regexp, "%2F");
13+}
14+
15 void Network::onReply(QNetworkReply *reply)
16 {
17 QVariant statusAttr = reply->attribute(
18@@ -201,7 +208,7 @@
19 qDebug() << "BUY STATE: in progress";
20 qDebug() << "BUY Redirect URL:" << url.toString();
21 QString sign = m_token.signUrl(url.toString(), "GET", true);
22- url.setQuery(sign);
23+ url.setQuery(encodeQuerySlashes(sign));
24 Q_EMIT buyInteractionRequired(url.toString());
25 } else {
26 qDebug() << "BUY STATE: failed";
27@@ -451,8 +458,9 @@
28 query.addQueryItem("currency", currency);
29 payUrl.setQuery(query);
30 qDebug() << "Get Add Payment URL:" << payUrl;
31- payUrl.setQuery(m_token.signUrl(payUrl.toString(),
32- QStringLiteral("GET"), true));
33+ QString sign = m_token.signUrl(payUrl.toString(),
34+ QStringLiteral("GET"), true);
35+ payUrl.setQuery(encodeQuerySlashes(sign));
36 return payUrl.toString();
37 }
38
39
40=== modified file 'backend/modules/payui/network.h'
41--- backend/modules/payui/network.h 2015-09-18 21:08:28 +0000
42+++ backend/modules/payui/network.h 2015-11-23 21:07:16 +0000
43@@ -84,6 +84,7 @@
44 static QString getSymbolForCurrency(const QString& currency_code);
45 static bool isSupportedCurrency(const QString& currency_code);
46 static QString sanitizeUrl(const QUrl& url);
47+ static QString encodeQuerySlashes(const QString& query);
48 virtual QString getEnvironmentValue(const QString& key,
49 const QString& defaultValue);
50 virtual QString getPayApiUrl(const QString& path);
51
52=== modified file 'backend/tests/test_network.cpp'
53--- backend/tests/test_network.cpp 2015-09-18 21:08:28 +0000
54+++ backend/tests/test_network.cpp 2015-11-23 21:07:16 +0000
55@@ -62,6 +62,7 @@
56 void testCheckAlreadyPurchased();
57 void testCheckAlreadyPurchasedFail();
58 void testSanitizeUrl();
59+ void testEncodeQuerySlashes();
60 void cleanupTestCase();
61
62 private:
63@@ -294,6 +295,13 @@
64 QTRY_COMPARE(result, expected.toString());
65 }
66
67+void TestNetwork::testEncodeQuerySlashes()
68+{
69+ QString query("abcdef/01235%3D");
70+ QTRY_COMPARE(network.encodeQuerySlashes(query),
71+ QStringLiteral("abcdef%2F01235%3D"));
72+}
73+
74 void TestNetwork::cleanupTestCase()
75 {
76 process->close();

Subscribers

People subscribed via source and target branches

to all changes: