From b86b8a45cc1b8f5c345867c18c40dc13cc449dce Mon Sep 17 00:00:00 2001
From: Jim Broadus <jbroadus@gmail.com>
Date: Thu, 13 Feb 2020 23:50:15 -0800
Subject: [PATCH] Fix dropbox json parsing.

Incorrect QJsonDocument::fromBinaryData was used several places in
DropboxService. Add a single ParseJsonReply method to the base class that
properly checks and parses network replies and reports errors.
---
 src/internet/core/internetservice.cpp   | 24 +++++++++++++++++++++++-
 src/internet/core/internetservice.h     |  7 +++++++
 src/internet/dropbox/dropboxservice.cpp | 21 +++++++++++++++++----
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/src/internet/core/internetservice.cpp b/src/internet/core/internetservice.cpp
index 1af4dd9be..ed9e07e61 100644
--- a/src/internet/core/internetservice.cpp
+++ b/src/internet/core/internetservice.cpp
@@ -25,15 +25,18 @@
 #include <QApplication>
 #include <QClipboard>
 #include <QIcon>
+#include <QJsonDocument>
 #include <QMenu>
 #include <QMessageBox>
+#include <QNetworkReply>
 #include <QPushButton>
 #include <QStandardItem>
 
-#include "internet/core/internetmodel.h"
+#include "core/application.h"
 #include "core/logging.h"
 #include "core/mergedproxymodel.h"
 #include "core/mimedata.h"
+#include "internet/core/internetmodel.h"
 #include "ui/iconloader.h"
 
 InternetService::InternetService(const QString& name, Application* app,
@@ -141,3 +144,22 @@ QStandardItem* InternetService::CreateSongItem(const Song& song) {
 
   return item;
 }
+
+QJsonDocument InternetService::ParseJsonReply(QNetworkReply* reply) {
+  if (reply->error() != QNetworkReply::NoError) {
+    app_->AddError(
+        tr("%1 request failed:\n%2").arg(name_).arg(reply->errorString()));
+    return QJsonDocument();
+  }
+
+  QJsonParseError error;
+  QJsonDocument document = QJsonDocument::fromJson(reply->readAll(), &error);
+  if (error.error != QJsonParseError::NoError) {
+    app_->AddError(tr("Failed to parse %1 response:\n%2")
+                       .arg(name_)
+                       .arg(error.errorString()));
+    return QJsonDocument();
+  }
+
+  return document;
+}
diff --git a/src/internet/core/internetservice.h b/src/internet/core/internetservice.h
index 63aeab47a..b66ecd2e6 100644
--- a/src/internet/core/internetservice.h
+++ b/src/internet/core/internetservice.h
@@ -38,6 +38,8 @@
 class Application;
 class InternetModel;
 class LibraryFilterWidget;
+class QJsonDocument;
+class QNetworkReply;
 class QStandardItem;
 
 class InternetService : public QObject {
@@ -111,6 +113,11 @@ signals:
   // Returns the 'open in new playlist' QAction.
   QAction* GetOpenInNewPlaylistAction();
 
+  // Check network reply for error, read, then parse. Will display errors if
+  // they occur. Returns a valid QJsonDocument on success, a null document on
+  // failure.
+  QJsonDocument ParseJsonReply(QNetworkReply* reply);
+
   // Describes how songs should be added to playlist.
   enum AddMode {
     // appends songs to the current playlist
diff --git a/src/internet/dropbox/dropboxservice.cpp b/src/internet/dropbox/dropboxservice.cpp
index 7cc738b6e..6d9c17f8d 100644
--- a/src/internet/dropbox/dropboxservice.cpp
+++ b/src/internet/dropbox/dropboxservice.cpp
@@ -137,7 +137,9 @@ void DropboxService::RequestFileList() {
 void DropboxService::RequestFileListFinished(QNetworkReply* reply) {
   reply->deleteLater();
 
-  QJsonDocument document = QJsonDocument::fromBinaryData(reply->readAll());
+  QJsonDocument document = ParseJsonReply(reply);
+  if (document.isNull()) return;
+
   QJsonObject json_response = document.object();
 
   if (json_response.contains("reset") && json_response["reset"].toBool()) {
@@ -213,7 +215,11 @@ void DropboxService::LongPollDelta() {
 
 void DropboxService::LongPollFinished(QNetworkReply* reply) {
   reply->deleteLater();
-  QJsonObject json_response = QJsonDocument::fromBinaryData(reply->readAll()).object();
+
+  QJsonDocument document = ParseJsonReply(reply);
+  if (document.isNull()) return;
+
+  QJsonObject json_response = document.object();
   if (json_response["changes"].toBool()) {
     // New changes, we should request deltas again.
     qLog(Debug) << "Detected new dropbox changes; fetching...";
@@ -241,7 +247,11 @@ QNetworkReply* DropboxService::FetchContentUrl(const QUrl& url) {
 void DropboxService::FetchContentUrlFinished(QNetworkReply* reply,
                                              const QVariantMap& data) {
   reply->deleteLater();
-  QJsonObject json_response = QJsonDocument::fromBinaryData(reply->readAll()).object();
+
+  QJsonDocument document = ParseJsonReply(reply);
+  if (document.isNull()) return;
+
+  QJsonObject json_response = document.object();
   QFileInfo info(data["path_lower"].toString());
 
   QUrl url;
@@ -267,6 +277,9 @@ QUrl DropboxService::GetStreamingUrlFromSongId(const QUrl& url) {
   QNetworkReply* reply = FetchContentUrl(url);
   WaitForSignal(reply, SIGNAL(finished()));
 
-  QJsonObject json_response = QJsonDocument::fromJson(reply->readAll()).object();
+  QJsonDocument document = ParseJsonReply(reply);
+  if (document.isNull()) return QUrl();
+
+  QJsonObject json_response = document.object();
   return QUrl::fromEncoded(json_response["link"].toVariant().toByteArray());
 }