pay-ui

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

  1. escape-signed-query
  2. Merge into first-branch
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 :

PASSED: Continuous integration, rev:135
http://jenkins.qa.ubuntu.com/job/pay-ui-ci/145/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-14.09-touch/21
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-14.09-armhf/43
    SUCCESS: http://jenkins.qa.ubuntu.com/job/pay-ui-14.09-armhf-ci/5
        deb: http://jenkins.qa.ubuntu.com/job/pay-ui-14.09-armhf-ci/5/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-mako/941
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-14.09-armhf/44
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24733

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/pay-ui-ci/145/rebuild

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 :

PASSED: Continuous integration, rev:136
http://jenkins.qa.ubuntu.com/job/pay-ui-ci/147/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-14.09-touch/25
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-14.09-armhf/51
    SUCCESS: http://jenkins.qa.ubuntu.com/job/pay-ui-14.09-armhf-ci/7
        deb: http://jenkins.qa.ubuntu.com/job/pay-ui-14.09-armhf-ci/7/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-touch/988
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-14.09-armhf/52
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25437

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/pay-ui-ci/147/rebuild

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
=== modified file 'backend/modules/payui/network.cpp'
--- backend/modules/payui/network.cpp 2015-10-01 13:54:35 +0000
+++ backend/modules/payui/network.cpp 2015-11-23 21:07:16 +0000
@@ -130,6 +130,13 @@
130 return url.toString().replace(regexp, "/");130 return url.toString().replace(regexp, "/");
131}131}
132132
133QString Network::encodeQuerySlashes(const QString& query)
134{
135 static QRegExp regexp("\\b\\/");
136 QString temp(query);
137 return temp.replace(regexp, "%2F");
138}
139
133void Network::onReply(QNetworkReply *reply)140void Network::onReply(QNetworkReply *reply)
134{141{
135 QVariant statusAttr = reply->attribute(142 QVariant statusAttr = reply->attribute(
@@ -201,7 +208,7 @@
201 qDebug() << "BUY STATE: in progress";208 qDebug() << "BUY STATE: in progress";
202 qDebug() << "BUY Redirect URL:" << url.toString();209 qDebug() << "BUY Redirect URL:" << url.toString();
203 QString sign = m_token.signUrl(url.toString(), "GET", true);210 QString sign = m_token.signUrl(url.toString(), "GET", true);
204 url.setQuery(sign);211 url.setQuery(encodeQuerySlashes(sign));
205 Q_EMIT buyInteractionRequired(url.toString());212 Q_EMIT buyInteractionRequired(url.toString());
206 } else {213 } else {
207 qDebug() << "BUY STATE: failed";214 qDebug() << "BUY STATE: failed";
@@ -451,8 +458,9 @@
451 query.addQueryItem("currency", currency);458 query.addQueryItem("currency", currency);
452 payUrl.setQuery(query);459 payUrl.setQuery(query);
453 qDebug() << "Get Add Payment URL:" << payUrl;460 qDebug() << "Get Add Payment URL:" << payUrl;
454 payUrl.setQuery(m_token.signUrl(payUrl.toString(),461 QString sign = m_token.signUrl(payUrl.toString(),
455 QStringLiteral("GET"), true));462 QStringLiteral("GET"), true);
463 payUrl.setQuery(encodeQuerySlashes(sign));
456 return payUrl.toString();464 return payUrl.toString();
457}465}
458466
459467
=== modified file 'backend/modules/payui/network.h'
--- backend/modules/payui/network.h 2015-09-18 21:08:28 +0000
+++ backend/modules/payui/network.h 2015-11-23 21:07:16 +0000
@@ -84,6 +84,7 @@
84 static QString getSymbolForCurrency(const QString& currency_code);84 static QString getSymbolForCurrency(const QString& currency_code);
85 static bool isSupportedCurrency(const QString& currency_code);85 static bool isSupportedCurrency(const QString& currency_code);
86 static QString sanitizeUrl(const QUrl& url);86 static QString sanitizeUrl(const QUrl& url);
87 static QString encodeQuerySlashes(const QString& query);
87 virtual QString getEnvironmentValue(const QString& key,88 virtual QString getEnvironmentValue(const QString& key,
88 const QString& defaultValue);89 const QString& defaultValue);
89 virtual QString getPayApiUrl(const QString& path);90 virtual QString getPayApiUrl(const QString& path);
9091
=== modified file 'backend/tests/test_network.cpp'
--- backend/tests/test_network.cpp 2015-09-18 21:08:28 +0000
+++ backend/tests/test_network.cpp 2015-11-23 21:07:16 +0000
@@ -62,6 +62,7 @@
62 void testCheckAlreadyPurchased();62 void testCheckAlreadyPurchased();
63 void testCheckAlreadyPurchasedFail();63 void testCheckAlreadyPurchasedFail();
64 void testSanitizeUrl();64 void testSanitizeUrl();
65 void testEncodeQuerySlashes();
65 void cleanupTestCase();66 void cleanupTestCase();
6667
67private:68private:
@@ -294,6 +295,13 @@
294 QTRY_COMPARE(result, expected.toString());295 QTRY_COMPARE(result, expected.toString());
295}296}
296297
298void TestNetwork::testEncodeQuerySlashes()
299{
300 QString query("abcdef/01235%3D");
301 QTRY_COMPARE(network.encodeQuerySlashes(query),
302 QStringLiteral("abcdef%2F01235%3D"));
303}
304
297void TestNetwork::cleanupTestCase()305void TestNetwork::cleanupTestCase()
298{306{
299 process->close();307 process->close();

Subscribers

People subscribed via source and target branches

to all changes: