Fix signature-generation for already-percent-encoded queries

As mentioned in RFC 5849 in section 3.4.1.3.1 queries
should be treated as application/x-www-form-urlencoded
and be decoded before being used in the base string.

The queries located in the URL now gets fully decoded and any
already-percent-encoded key-value pairs should be added to the URL. The
parameters list is used for non-encoded input.

[ChangeLog][OAuth1] QOAuth1 will now decode the query-segment of the URL
supplied in get/post/head/deleteResource during the signature-generation
phase. Any key-value-pair which needs to be passed in as percent-encoded
must be passed in the query of the URL.

Task-number: QTBUG-61125
Change-Id: I2ecb145f19f6c9a0031e9f00c6cb22d0fa5c96ea
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>

4 files changed, 28 insertions, 6 deletions

diff --git a/examples/oauth/twittertimeline/twittertimelinemodel.cpp b/examples/oauth/twittertimeline/twittertimelinemodel.cpp
index 87e387d..6ebe2da 100644
--- a/examples/oauth/twittertimeline/twittertimelinemodel.cpp
+++ b/examples/oauth/twittertimeline/twittertimelinemodel.cpp
@@ -133,11 +133,11 @@ void TwitterTimelineModel::updateTimeline()
         QMessageBox::warning(nullptr, qApp->applicationName(), "Not authenticated");
 
     QUrl url("https://api.twitter.com/1.1/statuses/home_timeline.json");
-    QUrlQuery query;
+    QVariantMap parameters;
     if (tweets.size()) {
         // Tweets are time-ordered, newest first.  Pass the most recent
         // ID we have to request everything more recent than it:
-        query.addQueryItem("since_id", QString::number(tweets.first().id));
+        parameters.insert("since_id", QString::number(tweets.first().id));
         // From https://dev.twitter.com/rest/reference/get/search/tweets:
         // Returns results with an ID greater than (that is, more recent than)
         // the specified ID. There are limits to the number of Tweets which can
@@ -145,8 +145,7 @@ void TwitterTimelineModel::updateTimeline()
         // since the since_id, the since_id will be forced to the oldest ID
         // available.
     }
-    url.setQuery(query);
-    QNetworkReply *reply = twitter.get(url);
+    QNetworkReply *reply = twitter.get(url, parameters);
     connect(reply, &QNetworkReply::finished, this, &TwitterTimelineModel::parseJson);
 }
 
diff --git a/src/oauth/qoauth1.cpp b/src/oauth/qoauth1.cpp
index e5d23d6..e23c21e 100644
--- a/src/oauth/qoauth1.cpp
+++ b/src/oauth/qoauth1.cpp
@@ -505,7 +505,7 @@ void QOAuth1::setup(QNetworkRequest *request,
     if (operation == QNetworkAccessManager::GetOperation) {
         if (signingParameters.size()) {
             QUrl url = request->url();
-            QUrlQuery query;
+            QUrlQuery query = QUrlQuery(url.query());
             for (auto it = signingParameters.begin(), end = signingParameters.end(); it != end;
                  ++it)
                 query.addQueryItem(it.key(), it.value().toString());
diff --git a/src/oauth/qoauth1signature.cpp b/src/oauth/qoauth1signature.cpp
index fe92ab0..d8bf191 100644
--- a/src/oauth/qoauth1signature.cpp
+++ b/src/oauth/qoauth1signature.cpp
@@ -96,7 +96,9 @@ QByteArray QOAuth1SignaturePrivate::signatureBaseString() const
 
     QVariantMap p = parameters;
     {
-        const auto queryItems = QUrlQuery(url.query()).queryItems();
+        // replace '+' with spaces now before decoding so that '%2B' gets left as '+'
+        const QString query = url.query().replace(QLatin1Char('+'), QLatin1Char(' '));
+        const auto queryItems = QUrlQuery(query).queryItems(QUrl::FullyDecoded);
         for (auto it = queryItems.begin(), end = queryItems.end(); it != end; ++it)
             p.insert(it->first, it->second);
     }
diff --git a/tests/auto/oauth1/tst_oauth1.cpp b/tests/auto/oauth1/tst_oauth1.cpp
index e80eb6c..e6d458c 100644
--- a/tests/auto/oauth1/tst_oauth1.cpp
+++ b/tests/auto/oauth1/tst_oauth1.cpp
@@ -564,6 +564,14 @@ void tst_OAuth1::authenticatedCalls_data()
                                     << QUrl("http://term.ie/oauth/example/echo_api.php")
                                     << parameters
                                     << QNetworkAccessManager::PostOperation;
+        QTest::newRow("term.ie_percent_encoded_query")
+            << "key"
+            << "secret"
+            << "accesskey"
+            << "accesssecret"
+            << QUrl("http://term.ie/oauth/example/echo_api.php?key=%40value+1%2B2=3")
+            << parameters
+            << QNetworkAccessManager::GetOperation;
     }
     if (hostReachable(QLatin1String("oauthbin.com"))) {
         QTest::newRow("oauthbin.com_get") << "key"
@@ -580,6 +588,14 @@ void tst_OAuth1::authenticatedCalls_data()
                                         << QUrl("http://oauthbin.com/v1/echo")
                                         << parameters
                                         << QNetworkAccessManager::PostOperation;
+        QTest::newRow("oauthbin.com_percent_encoded_query")
+            << "key"
+            << "secret"
+            << "accesskey"
+            << "accesssecret"
+            << QUrl("http://oauthbin.com/v1/echo?key=%40value+1%2B2=3")
+            << parameters
+            << QNetworkAccessManager::GetOperation;
     }
 }
 
@@ -598,6 +614,11 @@ void tst_OAuth1::authenticatedCalls()
     QString receivedData;
     QString parametersString;
     {
+        if (url.hasQuery()) {
+            parametersString = url.query(QUrl::FullyDecoded);
+            if (!parameters.empty())
+                parametersString.append(QLatin1Char('&'));
+        }
         bool first = true;
         for (auto it = parameters.begin(), end = parameters.end(); it != end; ++it) {
             if (first)