CLOUD: Upload ListDirectory Requests

Lots of checks to avoid JSON-related segfaults added.

backends/cloud/box/boxlistdirectorybyidrequest.cpp

@@ -43,15 +43,13 @@ BoxListDirectoryByIdRequest::BoxListDirectoryByIdRequest(BoxStorage *storage, Co
 
 BoxListDirectoryByIdRequest::~BoxListDirectoryByIdRequest() {
 	_ignoreCallback = true;
-	if (_workingRequest)
+	if (_workingRequest) _workingRequest->finish();
-		_workingRequest->finish();
 	delete _listDirectoryCallback;
 }
 
 void BoxListDirectoryByIdRequest::start() {
 	_ignoreCallback = true;
-	if (_workingRequest)
+	if (_workingRequest) _workingRequest->finish();
-		_workingRequest->finish();
 	_files.clear();
 	_ignoreCallback = false;
 
@@ -76,8 +74,11 @@ void BoxListDirectoryByIdRequest::makeRequest(uint32 offset) {
 
 void BoxListDirectoryByIdRequest::responseCallback(Networking::JsonResponse response) {
 	_workingRequest = nullptr;
-	if (_ignoreCallback)
+	if (_ignoreCallback) {
+		delete response.value;
 		return;
+	}
+
 	if (response.request)
 		_date = response.request->date();
 
@@ -103,7 +104,7 @@ void BoxListDirectoryByIdRequest::responseCallback(Networking::JsonResponse resp
 	Common::JSONObject responseObject = json->asObject();
 	//debug(9, "%s", json->stringify(true).c_str());
 
-	//TODO: check that error is returned the right way
+	//TODO: handle error messages passed as JSON
 	/*
 	if (responseObject.contains("error") || responseObject.contains("error_summary")) {
 		warning("Box returned error: %s", responseObject.getVal("error_summary")->asString().c_str());
@@ -129,22 +130,15 @@ void BoxListDirectoryByIdRequest::responseCallback(Networking::JsonResponse resp
 
 		Common::JSONArray items = responseObject.getVal("entries")->asArray();
 		for (uint32 i = 0; i < items.size(); ++i) {
-			if (!Networking::CurlJsonRequest::jsonIsObject(items[i], "BoxListDirectoryByIdRequest"))
+			if (!Networking::CurlJsonRequest::jsonIsObject(items[i], "BoxListDirectoryByIdRequest")) continue;
-				continue;
 
 			Common::JSONObject item = items[i]->asObject();
 
-			if (!Networking::CurlJsonRequest::jsonContainsString(item, "id", "BoxListDirectoryByIdRequest"))
+			if (!Networking::CurlJsonRequest::jsonContainsString(item, "id", "BoxListDirectoryByIdRequest")) continue;
-				continue;
+			if (!Networking::CurlJsonRequest::jsonContainsString(item, "name", "BoxListDirectoryByIdRequest")) continue;
-			if (!Networking::CurlJsonRequest::jsonContainsString(item, "name", "BoxListDirectoryByIdRequest"))
+			if (!Networking::CurlJsonRequest::jsonContainsString(item, "type", "BoxListDirectoryByIdRequest")) continue;
-				continue;
+			if (!Networking::CurlJsonRequest::jsonContainsString(item, "modified_at", "BoxListDirectoryByIdRequest")) continue;
-			if (!Networking::CurlJsonRequest::jsonContainsString(item, "type", "BoxListDirectoryByIdRequest"))
+			if (!Networking::CurlJsonRequest::jsonContainsStringOrIntegerNumber(item, "size", "BoxListDirectoryByIdRequest")) continue;
-				continue;
-			if (!Networking::CurlJsonRequest::jsonContainsString(item, "modified_at", "BoxListDirectoryByIdRequest"))
-				continue;
-			if (!Networking::CurlJsonRequest::jsonContainsString(item, "size", "BoxListDirectoryByIdRequest") &&
-					!Networking::CurlJsonRequest::jsonContainsIntegerNumber(item, "size", "BoxListDirectoryByIdRequest"))
-				continue;
 
 			Common::String id = item.getVal("id")->asString();
 			Common::String name = item.getVal("name")->asString();
@@ -172,20 +166,16 @@ void BoxListDirectoryByIdRequest::responseCallback(Networking::JsonResponse resp
 		received += responseObject.getVal("limit")->asIntegerNumber();
 	bool hasMore = (received < totalCount);
 
-	if (hasMore)
+	if (hasMore) makeRequest(received);
-		makeRequest(received);
+	else finishListing(_files);
-	else
-		finishListing(_files);
 
 	delete json;
 }
 
 void BoxListDirectoryByIdRequest::errorCallback(Networking::ErrorResponse error) {
 	_workingRequest = nullptr;
-	if (_ignoreCallback)
+	if (_ignoreCallback) return;
-		return;
+	if (error.request) _date = error.request->date();
-	if (error.request)
-		_date = error.request->date();
 	finishError(error);
 }
 
@@ -197,8 +187,7 @@ Common::String BoxListDirectoryByIdRequest::date() const { return _date; }
 
 void BoxListDirectoryByIdRequest::finishListing(Common::Array<StorageFile> &files) {
 	Request::finishSuccess();
-	if (_listDirectoryCallback)
+	if (_listDirectoryCallback) (*_listDirectoryCallback)(Storage::ListDirectoryResponse(this, files));
-		(*_listDirectoryCallback)(Storage::ListDirectoryResponse(this, files));
 }
 
 } // End of namespace Box

backends/cloud/dropbox/dropboxlistdirectoryrequest.cpp

@@ -71,8 +71,10 @@ void DropboxListDirectoryRequest::start() {
 
 void DropboxListDirectoryRequest::responseCallback(Networking::JsonResponse response) {
 	_workingRequest = nullptr;
-	if (_ignoreCallback)
+	if (_ignoreCallback) {
+		delete response.value;
 		return;
+	}
 
 	if (response.request)
 		_date = response.request->date();

backends/cloud/googledrive/googledrivelistdirectorybyidrequest.cpp

@@ -72,8 +72,10 @@ void GoogleDriveListDirectoryByIdRequest::makeRequest(Common::String pageToken)
 
 void GoogleDriveListDirectoryByIdRequest::responseCallback(Networking::JsonResponse response) {
 	_workingRequest = nullptr;
-	if (_ignoreCallback)
+	if (_ignoreCallback) {
+		delete response.value;
 		return;
+	}
 	if (response.request)
 		_date = response.request->date();
 

backends/cloud/onedrive/onedrivelistdirectoryrequest.cpp

@@ -104,20 +104,40 @@ void OneDriveListDirectoryRequest::listedDirectoryCallback(Networking::JsonRespo
 	if (rq && rq->getNetworkReadStream())
 		error.httpResponseCode = rq->getNetworkReadStream()->httpResponseCode();
 
-	if (!json) {
+	if (json == nullptr) {
-		error.failed = true;
+		error.response = "Failed to parse JSON, null passed!";
 		finishError(error);
 		return;
 	}
 
+	if (!json->isObject()) {
+		error.response = "Passed JSON is not an object!";
+		finishError(error);
+		delete json;
+		return;
+	}
+
 	Common::JSONObject object = json->asObject();
 
-	//TODO: check that ALL keys exist AND HAVE RIGHT TYPE to avoid segfaults
+	//check that ALL keys exist AND HAVE RIGHT TYPE to avoid segfaults
+	if (!Networking::CurlJsonRequest::jsonContainsArray(object, "value", "OneDriveListDirectoryRequest")) {
+		error.response = "\"value\" not found or that's not an array!";
+		finishError(error);
+		delete json;
+		return;
+	}
 
 	Common::JSONArray items = object.getVal("value")->asArray();
 	for (uint32 i = 0; i < items.size(); ++i) {
+		if (!Networking::CurlJsonRequest::jsonIsObject(items[i], "OneDriveListDirectoryRequest")) continue;
+
 		Common::JSONObject item = items[i]->asObject();
 
+		if (!Networking::CurlJsonRequest::jsonContainsAttribute(item, "folder", "OneDriveListDirectoryRequest", true)) continue;
+		if (!Networking::CurlJsonRequest::jsonContainsString(item, "name", "OneDriveListDirectoryRequest")) continue;
+		if (!Networking::CurlJsonRequest::jsonContainsIntegerNumber(item, "size", "OneDriveListDirectoryRequest")) continue;
+		if (!Networking::CurlJsonRequest::jsonContainsString(item, "lastModifiedDateTime", "OneDriveListDirectoryRequest")) continue;
+
 		Common::String path = _currentDirectory + item.getVal("name")->asString();
 		bool isDirectory = item.contains("folder");
 		uint32 size = item.getVal("size")->asIntegerNumber();
@@ -132,6 +152,13 @@ void OneDriveListDirectoryRequest::listedDirectoryCallback(Networking::JsonRespo
 
 	bool hasMore = object.contains("@odata.nextLink");
 	if (hasMore) {
+		if (!Networking::CurlJsonRequest::jsonContainsString(object, "@odata.nextLink", "OneDriveListDirectoryRequest")) {
+			error.response = "\"@odata.nextLink\" is not a string!";
+			finishError(error);
+			delete json;
+			return;
+		}
+
 		makeRequest(object.getVal("@odata.nextLink")->asString());
 	} else {
 		listNextDirectory();

backends/networking/curl/curljsonrequest.cpp

@@ -42,7 +42,7 @@ CurlJsonRequest::~CurlJsonRequest() {
 
 char *CurlJsonRequest::getPreparedContents() {
 	//write one more byte in the end
-	byte zero[1] = { 0 };
+	byte zero[1] = {0};
 	_contentsStream.write(zero, 1);
 
 	//replace all "bad" bytes with '.' character
@@ -114,8 +114,12 @@ bool CurlJsonRequest::jsonIsObject(Common::JSONValue *item, const char *warningP
 	return false;
 }
 
-bool CurlJsonRequest::jsonContainsString(Common::JSONObject &item, const char *key, const char *warningPrefix) {
+bool CurlJsonRequest::jsonContainsString(Common::JSONObject &item, const char *key, const char *warningPrefix, bool isOptional) {
 	if (!item.contains(key)) {
+		if (isOptional) {
+			return true;
+		}
+
 		warning("%s: passed item misses the \"%s\" attribute!", warningPrefix, key);
 		return false;
 	}
@@ -127,8 +131,12 @@ bool CurlJsonRequest::jsonContainsString(Common::JSONObject &item, const char *k
 	return false;
 }
 
-bool CurlJsonRequest::jsonContainsIntegerNumber(Common::JSONObject &item, const char *key, const char *warningPrefix) {
+bool CurlJsonRequest::jsonContainsIntegerNumber(Common::JSONObject &item, const char *key, const char *warningPrefix, bool isOptional) {
 	if (!item.contains(key)) {
+		if (isOptional) {
+			return true;
+		}
+
 		warning("%s: passed item misses the \"%s\" attribute!", warningPrefix, key);
 		return false;
 	}
@@ -140,4 +148,51 @@ bool CurlJsonRequest::jsonContainsIntegerNumber(Common::JSONObject &item, const
 	return false;
 }
 
+bool CurlJsonRequest::jsonContainsArray(Common::JSONObject &item, const char *key, const char *warningPrefix, bool isOptional) {
+	if (!item.contains(key)) {
+		if (isOptional) {
+			return true;
+		}
+
+		warning("%s: passed item misses the \"%s\" attribute!", warningPrefix, key);
+		return false;
+	}
+
+	if (item.getVal(key)->isArray()) return true;
+
+	warning("%s: passed item's \"%s\" attribute is not an array!", warningPrefix, key);
+	debug(9, "%s", item.getVal(key)->stringify(true).c_str());
+	return false;
+}
+
+bool CurlJsonRequest::jsonContainsStringOrIntegerNumber(Common::JSONObject &item, const char *key, const char *warningPrefix, bool isOptional) {
+	if (!item.contains(key)) {
+		if (isOptional) {
+			return true;
+		}
+
+		warning("%s: passed item misses the \"%s\" attribute!", warningPrefix, key);
+		return false;
+	}
+
+	if (item.getVal(key)->isString() || item.getVal(key)->isIntegerNumber()) return true;
+
+	warning("%s: passed item's \"%s\" attribute is neither a string or an integer!", warningPrefix, key);
+	debug(9, "%s", item.getVal(key)->stringify(true).c_str());
+	return false;
+}
+
+bool CurlJsonRequest::jsonContainsAttribute(Common::JSONObject &item, const char *key, const char *warningPrefix, bool isOptional) {
+	if (!item.contains(key)) {
+		if (isOptional) {
+			return true;
+		}
+
+		warning("%s: passed item misses the \"%s\" attribute!", warningPrefix, key);
+		return false;
+	}
+
+	return true;
+}
+
 } // End of namespace Networking

backends/networking/curl/curljsonrequest.h

@@ -54,8 +54,11 @@ public:
 	virtual void restart();
 
 	static bool jsonIsObject(Common::JSONValue *item, const char *warningPrefix);
-	static bool jsonContainsString(Common::JSONObject &item, const char *key, const char *warningPrefix);
+	static bool jsonContainsString(Common::JSONObject &item, const char *key, const char *warningPrefix, bool isOptional = false);
-	static bool jsonContainsIntegerNumber(Common::JSONObject &item, const char *key, const char *warningPrefix);
+	static bool jsonContainsIntegerNumber(Common::JSONObject &item, const char *key, const char *warningPrefix, bool isOptional = false);
+	static bool jsonContainsArray(Common::JSONObject &item, const char *key, const char *warningPrefix, bool isOptional = false);
+	static bool jsonContainsStringOrIntegerNumber(Common::JSONObject &item, const char *key, const char *warningPrefix, bool isOptional = false);
+	static bool jsonContainsAttribute(Common::JSONObject &item, const char *key, const char *warningPrefix, bool isOptional = false);
 };
 
 } // End of namespace Networking