Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added checks for nullptr in script arguments #2736

Merged
merged 2 commits into from
Jan 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions src/tiled/editablemap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ void EditableMap::removeLayerAt(int index)
void EditableMap::removeLayer(EditableLayer *editableLayer)
{
if (!editableLayer) {
ScriptManager::instance().throwError(QCoreApplication::translate("Script Errors", "Invalid argument"));
ScriptManager::instance().throwNullArgError(0);
return;
}

Expand All @@ -204,7 +204,7 @@ void EditableMap::insertLayerAt(int index, EditableLayer *editableLayer)
}

if (!editableLayer) {
ScriptManager::instance().throwError(QCoreApplication::translate("Script Errors", "Invalid argument"));
ScriptManager::instance().throwNullArgError(0);
return;
}

Expand All @@ -228,6 +228,10 @@ void EditableMap::addLayer(EditableLayer *editableLayer)

bool EditableMap::addTileset(EditableTileset *editableTileset)
{
if (!editableTileset) {
ScriptManager::instance().throwNullArgError(0);
return false;
}
const auto &tileset = editableTileset->tileset()->sharedPointer();
if (map()->indexOfTileset(tileset) != -1)
return false; // can't add existing tileset
Expand All @@ -243,6 +247,14 @@ bool EditableMap::addTileset(EditableTileset *editableTileset)
bool EditableMap::replaceTileset(EditableTileset *oldEditableTileset,
EditableTileset *newEditableTileset)
{
if (!oldEditableTileset) {
ScriptManager::instance().throwNullArgError(0);
return false;
}
if (!newEditableTileset) {
ScriptManager::instance().throwNullArgError(1);
return false;
}
if (oldEditableTileset == newEditableTileset) {
ScriptManager::instance().throwError(QCoreApplication::translate("Script Errors", "Invalid argument"));
return false;
Expand All @@ -268,6 +280,10 @@ bool EditableMap::replaceTileset(EditableTileset *oldEditableTileset,

bool EditableMap::removeTileset(EditableTileset *editableTileset)
{
if (!editableTileset) {
ScriptManager::instance().throwNullArgError(0);
return false;
}
Tileset *tileset = editableTileset->tileset();
int index = map()->indexOfTileset(tileset->sharedPointer());
if (index == -1)
Expand Down Expand Up @@ -308,6 +324,10 @@ QList<QObject *> EditableMap::usedTilesets() const
*/
void EditableMap::merge(EditableMap *editableMap, bool canJoin)
{
if (!editableMap) {
ScriptManager::instance().throwNullArgError(0);
return;
}
if (!mapDocument()) { // todo: support this outside of the undo stack
ScriptManager::instance().throwError(QCoreApplication::translate("Script Errors", "Merge is currently not supported for detached maps"));
return;
Expand Down
8 changes: 8 additions & 0 deletions src/tiled/editableobjectgroup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ void EditableObjectGroup::removeObjectAt(int index)

void EditableObjectGroup::removeObject(EditableMapObject *editableMapObject)
{
if (!editableMapObject) {
ScriptManager::instance().throwNullArgError(0);
return;
}
int index = objectGroup()->objects().indexOf(editableMapObject->mapObject());
if (index == -1) {
ScriptManager::instance().throwError(QCoreApplication::translate("Script Errors", "Object not found"));
Expand All @@ -92,6 +96,10 @@ void EditableObjectGroup::removeObject(EditableMapObject *editableMapObject)

void EditableObjectGroup::insertObjectAt(int index, EditableMapObject *editableMapObject)
{
if (!editableMapObject) {
ScriptManager::instance().throwNullArgError(1);
return;
}
if (index < 0 || index > objectCount()) {
ScriptManager::instance().throwError(QCoreApplication::translate("Script Errors", "Index out of range"));
return;
Expand Down
2 changes: 1 addition & 1 deletion src/tiled/editabletile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ void EditableTile::setProbability(qreal probability)
void EditableTile::setObjectGroup(EditableObjectGroup *editableObjectGroup)
{
if (!editableObjectGroup) {
ScriptManager::instance().throwError(QCoreApplication::translate("Script Errors", "Invalid argument"));
ScriptManager::instance().throwNullArgError(0);
return;
}

Expand Down
5 changes: 5 additions & 0 deletions src/tiled/mapeditor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
#include "preferences.h"
#include "propertiesdock.h"
#include "reversingproxymodel.h"
#include "scriptmanager.h"
#include "selectsametiletool.h"
#include "shapefilltool.h"
#include "stampbrush.h"
Expand Down Expand Up @@ -1013,6 +1014,10 @@ EditableMap *MapEditor::currentBrush() const

void MapEditor::setCurrentBrush(EditableMap *editableMap)
{
if (!editableMap) {
ScriptManager::instance().throwNullArgError(0);
return;
}
// todo: filter any non-tilelayers out of the map?
setStamp(TileStamp(editableMap->map()->clone()));
}
Expand Down
4 changes: 4 additions & 0 deletions src/tiled/scriptedtool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ EditableMap *ScriptedTool::preview() const

void ScriptedTool::setPreview(EditableMap *editableMap)
{
if (!editableMap) {
ScriptManager::instance().throwNullArgError(0);
return;
}
// todo: filter any non-tilelayers out of the map?
auto map = editableMap->map()->clone();
brushItem()->setMap(SharedMap { map.release() });
Expand Down
8 changes: 8 additions & 0 deletions src/tiled/scriptfileformatwrappers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ EditableTileset *ScriptTilesetFormatWrapper::read(const QString &filename)

void ScriptTilesetFormatWrapper::write(EditableTileset *editable, const QString &filename)
{
if (!editable) {
ScriptManager::instance().throwNullArgError(0);
return;
}
if (!assertCanWrite())
return;

Expand Down Expand Up @@ -123,6 +127,10 @@ EditableMap *ScriptMapFormatWrapper::read(const QString &filename)

void ScriptMapFormatWrapper::write(EditableMap *editable, const QString &filename)
{
if (!editable) {
ScriptManager::instance().throwNullArgError(0);
return;
}
if (!assertCanWrite())
return;

Expand Down
7 changes: 7 additions & 0 deletions src/tiled/scriptmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#include <QStandardPaths>
#include <QTextCodec>
#include <QtDebug>
#include <QCoreApplication>

namespace Tiled {

Expand Down Expand Up @@ -260,6 +261,12 @@ void ScriptManager::throwError(const QString &message)
#endif
}

void ScriptManager::throwNullArgError(int argNumber)
{
throwError(QCoreApplication::translate("Script Errors",
"Argument %1 is undefined or the wrong type").arg(argNumber));
}

void ScriptManager::reset()
{
Tiled::INFO(tr("Resetting script engine"));
Expand Down
1 change: 1 addition & 0 deletions src/tiled/scriptmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class ScriptManager : public QObject

bool checkError(QJSValue value, const QString &program = QString());
void throwError(const QString &message);
void throwNullArgError(int argNumber);

void reset();

Expand Down
12 changes: 12 additions & 0 deletions src/tiled/scriptmodule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ EditableAsset *ScriptModule::activeAsset() const

bool ScriptModule::setActiveAsset(EditableAsset *asset) const
{
if (!asset) {
ScriptManager::instance().throwNullArgError(0);
return false;
}
auto documentManager = DocumentManager::instance();
for (const DocumentPtr &document : documentManager->documents())
if (document->editable() == asset)
Expand Down Expand Up @@ -201,6 +205,10 @@ EditableAsset *ScriptModule::open(const QString &fileName) const

bool ScriptModule::close(EditableAsset *asset) const
{
if (!asset) {
ScriptManager::instance().throwNullArgError(0);
return false;
}
auto documentManager = DocumentManager::instance();

int index = documentManager->findDocument(asset->document());
Expand All @@ -215,6 +223,10 @@ bool ScriptModule::close(EditableAsset *asset) const

EditableAsset *ScriptModule::reload(EditableAsset *asset) const
{
if (!asset) {
ScriptManager::instance().throwNullArgError(0);
return nullptr;
}
auto documentManager = DocumentManager::instance();

int index = documentManager->findDocument(asset->document());
Expand Down
5 changes: 5 additions & 0 deletions src/tiled/tilesetdock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "objectgroup.h"
#include "preferences.h"
#include "replacetileset.h"
#include "scriptmanager.h"
#include "swaptiles.h"
#include "terrain.h"
#include "tile.h"
Expand Down Expand Up @@ -880,6 +881,10 @@ SharedTileset TilesetDock::currentTileset() const

void TilesetDock::setCurrentEditableTileset(EditableTileset *tileset)
{
if (!tileset) {
ScriptManager::instance().throwNullArgError(0);
return;
}
setCurrentTileset(tileset->tileset()->sharedPointer());
}

Expand Down