From f6446c6806e17dfdc76e423b8757143393cf97f2 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Fri, 13 Jan 2017 13:18:24 -0800 Subject: [PATCH 01/25] minimum entity edit filter --- .../src/entities/EntityServer.cpp | 6 +++ assignment-client/src/octree/OctreeServer.cpp | 3 ++ .../resources/describe-settings.json | 8 ++++ libraries/entities/src/EntityTree.cpp | 42 +++++++++++++++++++ libraries/entities/src/EntityTree.h | 12 ++++++ libraries/octree/src/Octree.h | 1 + 6 files changed, 72 insertions(+) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index 23eec6197c..e5c22c3d34 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -285,6 +285,12 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio } else { tree->setEntityScriptSourceWhitelist(""); } + + QString entityEditFilter; + if (readOptionString("entityEditFilter", settingsSectionObject, entityEditFilter)) { + tree->setEntityEditFilter(entityEditFilter); + } + tree->initEntityEditFilterEngine(); // whether supplied or not. } void EntityServer::nodeAdded(SharedNodePointer node) { diff --git a/assignment-client/src/octree/OctreeServer.cpp b/assignment-client/src/octree/OctreeServer.cpp index 3e36250a82..ad40b1c671 100644 --- a/assignment-client/src/octree/OctreeServer.cpp +++ b/assignment-client/src/octree/OctreeServer.cpp @@ -660,6 +660,7 @@ bool OctreeServer::handleHTTPRequest(HTTPConnection* connection, const QUrl& url quint64 averageUpdateTime = _tree->getAverageUpdateTime(); quint64 averageCreateTime = _tree->getAverageCreateTime(); quint64 averageLoggingTime = _tree->getAverageLoggingTime(); + quint64 averageFilterTime = _tree->getAverageFilterTime(); int FLOAT_PRECISION = 3; @@ -699,6 +700,8 @@ bool OctreeServer::handleHTTPRequest(HTTPConnection* connection, const QUrl& url .arg(locale.toString((uint)averageCreateTime).rightJustified(COLUMN_WIDTH, ' ')); statsString += QString(" Average Logging Time: %1 usecs\r\n") .arg(locale.toString((uint)averageLoggingTime).rightJustified(COLUMN_WIDTH, ' ')); + statsString += QString(" Average Filter Time: %1 usecs\r\n") + .arg(locale.toString((uint)averageFilterTime).rightJustified(COLUMN_WIDTH, ' ')); int senderNumber = 0; diff --git a/domain-server/resources/describe-settings.json b/domain-server/resources/describe-settings.json index dd0e4ad4a1..f701f35433 100644 --- a/domain-server/resources/describe-settings.json +++ b/domain-server/resources/describe-settings.json @@ -1290,6 +1290,14 @@ "default": "", "advanced": true }, + { + "name": "entityEditFilter", + "label": "Filter Entity Edits", + "help": "Check all entity edits against this filter function.", + "placeholder": "function filter(properties) { return properties; }", + "default": "", + "advanced": true + }, { "name": "persistFilePath", "label": "Entities File Path", diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 4796dda671..c88ff15032 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -918,6 +918,31 @@ void EntityTree::fixupTerseEditLogging(EntityItemProperties& properties, QList Date: Fri, 13 Jan 2017 16:59:51 -0800 Subject: [PATCH 02/25] getting there, but not yet complete --- libraries/entities/src/EntityItemProperties.cpp | 4 ++-- libraries/entities/src/EntityItemProperties.h | 2 +- libraries/entities/src/EntityTree.cpp | 11 ++++++----- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 927708fc4b..788217b847 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -346,11 +346,11 @@ EntityPropertyFlags EntityItemProperties::getChangedProperties() const { return changedProperties; } -QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults) const { +QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime) const { QScriptValue properties = engine->newObject(); EntityItemProperties defaultEntityProperties; - if (_created == UNKNOWN_CREATED_TIME) { + if (_created == UNKNOWN_CREATED_TIME && !allowUknownCreateTime) { // No entity properties can have been set so return without setting any default, zero property values. return properties; } diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index fb08182a2e..0fc8db185b 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -73,7 +73,7 @@ public: EntityTypes::EntityType getType() const { return _type; } void setType(EntityTypes::EntityType type) { _type = type; } - virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults) const; + virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime = false) const; virtual void copyFromScriptValue(const QScriptValue& object, bool honorReadOnly); static QScriptValue entityPropertyFlagsToScriptValue(QScriptEngine* engine, const EntityPropertyFlags& flags); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index c88ff15032..47b59a1c19 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -931,7 +931,7 @@ bool EntityTree::filterProperties(const EntityItemProperties& propertiesIn, Enti wasChanged = false; // not changed return true; // allowed } - QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, false); + QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, true, true); QScriptValueList args; args << inputValues; @@ -940,6 +940,7 @@ bool EntityTree::filterProperties(const EntityItemProperties& propertiesIn, Enti propertiesOut.copyFromScriptValue(result, false); wasChanged = result.equals(inputValues); // not changed + qDebug() << "result" << propertiesOut << result.toVariant() << wasChanged; return result.isObject(); // filters should return null or false to completely reject edit or add } @@ -1028,17 +1029,17 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c if (validEditPacket) { startFilter = usecTimestampNow(); - QScriptValue originalValues = properties.copyToScriptValue(&_entityEditFilterEngine, true); bool wasChanged = false; bool allowed = filterProperties(properties, properties, wasChanged); - QScriptValue filteredResult = properties.copyToScriptValue(&_entityEditFilterEngine, true); - if (wasChanged) { + if (!allowed) { + properties = EntityItemProperties(); // Maybe other behavior + } else if (wasChanged) { if (properties.getLastEdited() == UNKNOWN_CREATED_TIME) { properties.setLastEdited(usecTimestampNow()); } properties.setLastEdited(properties.getLastEdited() + LAST_EDITED_SERVERSIDE_BUMP); } - qDebug() << "FIXME changed/allowed" << wasChanged << allowed; + qDebug() << "filtered:" << properties; endFilter = usecTimestampNow(); // search for the entity by EntityItemID From 09a57efaf30bc292701e10c3a57cc2e51a60fff3 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Mon, 16 Jan 2017 12:47:26 -0800 Subject: [PATCH 03/25] checkpoint --- libraries/entities/src/EntityTree.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 47b59a1c19..8da21ea582 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -932,6 +932,7 @@ bool EntityTree::filterProperties(const EntityItemProperties& propertiesIn, Enti return true; // allowed } QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, true, true); + qDebug() << "input" << propertiesIn << inputValues.toVariant(); QScriptValueList args; args << inputValues; @@ -1032,8 +1033,9 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c bool wasChanged = false; bool allowed = filterProperties(properties, properties, wasChanged); if (!allowed) { - properties = EntityItemProperties(); // Maybe other behavior - } else if (wasChanged) { + properties = EntityItemProperties(); + } + if (!allowed || wasChanged) { if (properties.getLastEdited() == UNKNOWN_CREATED_TIME) { properties.setLastEdited(usecTimestampNow()); } From 2e9b26057ea3e61a18bd3cb0094f6fd1c0d1f691 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 18 Jan 2017 10:27:32 -0800 Subject: [PATCH 04/25] refactor bumpTimestamp, and fix implementation of wasChanged --- libraries/entities/src/EntityTree.cpp | 50 +++++++++++++++++---------- libraries/entities/src/EntityTree.h | 1 + 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 8da21ea582..bb6bf204fb 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -932,17 +932,36 @@ bool EntityTree::filterProperties(const EntityItemProperties& propertiesIn, Enti return true; // allowed } QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, true, true); - qDebug() << "input" << propertiesIn << inputValues.toVariant(); + auto in = QJsonValue::fromVariant(inputValues.toVariant()); // grab json copy now, because the inputValues might be side effected by the filter. + qDebug() << "fixme filter input" << propertiesIn << inputValues.toVariant() << in; QScriptValueList args; args << inputValues; - QScriptValue result = _entityEditFilterEngine.newObject(); - result = _entityEditFilterFunction.call(_nullObjectForFilter, args); + QScriptValue result = _entityEditFilterFunction.call(_nullObjectForFilter, args); propertiesOut.copyFromScriptValue(result, false); - wasChanged = result.equals(inputValues); // not changed - qDebug() << "result" << propertiesOut << result.toVariant() << wasChanged; - return result.isObject(); // filters should return null or false to completely reject edit or add + bool accepted = result.isObject(); // filters should return null or false to completely reject edit or add + if (accepted) { + // Javascript objects are == only if they are the same object. To compare arbitrary values, we need to use JSON. + + auto out = QJsonValue::fromVariant(result.toVariant()); + bool eq = in == out; + qDebug() << "in:" << in << "out:" << out << "eq:" << eq; + wasChanged = !eq; + } + + qDebug() << "fixme filter result" << propertiesOut << result.toVariant() << "accepted:" << accepted << "changed:" << (accepted && wasChanged); + return accepted; +} + +void EntityTree::bumpTimestamp(EntityItemProperties& properties) { //fixme put class/header + const quint64 LAST_EDITED_SERVERSIDE_BUMP = 1; // usec + // also bump up the lastEdited time of the properties so that the interface that created this edit + // will accept our adjustment to lifetime back into its own entity-tree. + if (properties.getLastEdited() == UNKNOWN_CREATED_TIME) { + properties.setLastEdited(usecTimestampNow()); + } + properties.setLastEdited(properties.getLastEdited() + LAST_EDITED_SERVERSIDE_BUMP); } int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned char* editData, int maxLength, @@ -971,7 +990,6 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c quint64 startFilter = 0, endFilter = 0; quint64 startLogging = 0, endLogging = 0; - const quint64 LAST_EDITED_SERVERSIDE_BUMP = 1; // usec bool suppressDisallowedScript = false; _totalEditMessages++; @@ -1016,12 +1034,7 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c if (properties.getLifetime() == ENTITY_ITEM_IMMORTAL_LIFETIME || properties.getLifetime() > _maxTmpEntityLifetime) { properties.setLifetime(_maxTmpEntityLifetime); - // also bump up the lastEdited time of the properties so that the interface that created this edit - // will accept our adjustment to lifetime back into its own entity-tree. - if (properties.getLastEdited() == UNKNOWN_CREATED_TIME) { - properties.setLastEdited(usecTimestampNow()); - } - properties.setLastEdited(properties.getLastEdited() + LAST_EDITED_SERVERSIDE_BUMP); + bumpTimestamp(properties); } } @@ -1036,12 +1049,13 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c properties = EntityItemProperties(); } if (!allowed || wasChanged) { - if (properties.getLastEdited() == UNKNOWN_CREATED_TIME) { - properties.setLastEdited(usecTimestampNow()); + bumpTimestamp(properties); + if (properties.getSimulationOwner().getID() == senderNode->getUUID()) { + qDebug() << "fixme Suppressing ownership from" << senderNode->getUUID(); + properties.setSimulationOwner(QUuid(), 0); } - properties.setLastEdited(properties.getLastEdited() + LAST_EDITED_SERVERSIDE_BUMP); } - qDebug() << "filtered:" << properties; + qDebug() << "fixme final:" << properties; endFilter = usecTimestampNow(); // search for the entity by EntityItemID @@ -1051,7 +1065,7 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c if (existingEntity && message.getType() == PacketType::EntityEdit) { if (suppressDisallowedScript) { - properties.setLastEdited(properties.getLastEdited() + LAST_EDITED_SERVERSIDE_BUMP); + bumpTimestamp(properties); properties.setScript(existingEntity->getScript()); } diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index d471694023..7cdf1b0c77 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -289,6 +289,7 @@ protected: static bool findInBoxOperation(OctreeElementPointer element, void* extraData); static bool findInFrustumOperation(OctreeElementPointer element, void* extraData); static bool sendEntitiesOperation(OctreeElementPointer element, void* extraData); + static void bumpTimestamp(EntityItemProperties& properties); void notifyNewlyCreatedEntity(const EntityItem& newEntity, const SharedNodePointer& senderNode); From 60eb6dca9d0b283161f603e08ca6dd08692c789d Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 18 Jan 2017 10:29:11 -0800 Subject: [PATCH 05/25] remove debug dump that was printing to screeen during << operator --- libraries/entities/src/EntityItemProperties.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index 0fc8db185b..20491bdaf5 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -460,10 +460,6 @@ inline QDebug operator<<(QDebug debug, const EntityItemProperties& properties) { DEBUG_PROPERTY_IF_CHANGED(debug, properties, LastEditedBy, lastEditedBy, ""); - properties.getAnimation().debugDump(); - properties.getSkybox().debugDump(); - properties.getStage().debugDump(); - debug << " last edited:" << properties.getLastEdited() << "\n"; debug << " edited ago:" << properties.getEditedAgo() << "\n"; debug << "]"; From 00e53c8ea6b07a0c290d277802238303d84a789e Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 18 Jan 2017 13:21:28 -0800 Subject: [PATCH 06/25] more tractable copy semantics --- libraries/entities/src/EntityItemProperties.cpp | 16 ++++++++++------ libraries/entities/src/EntityItemProperties.h | 4 +++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 788217b847..a16a4fbaba 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -346,7 +346,11 @@ EntityPropertyFlags EntityItemProperties::getChangedProperties() const { return changedProperties; } -QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime) const { +QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime, bool strictSemantics) const { + // If strictSemantics is true and skipDefaults is false, then all and only those properties are copied in the property flag + // is included in _desiredProperties, or is one of the specially enumerated ALWAYS properties below. + // (There may be exceptions, but if so, they are bugs.) + // In all other cases, you are welcome to inspect the code and try to figure out what was intended. I wish you luck. -HRS 1/18/17 QScriptValue properties = engine->newObject(); EntityItemProperties defaultEntityProperties; @@ -364,7 +368,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool created.setTimeSpec(Qt::OffsetFromUTC); COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_ALWAYS(created, created.toString(Qt::ISODate)); - if (!skipDefaults || _lifetime != defaultEntityProperties._lifetime) { + if ((!skipDefaults || _lifetime != defaultEntityProperties._lifetime) && !strictSemantics) { COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_NO_SKIP(age, getAge()); // gettable, but not settable COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_NO_SKIP(ageAsText, formatSecondsElapsed(getAge())); // gettable, but not settable } @@ -539,7 +543,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool } // Sitting properties support - if (!skipDefaults) { + if (!skipDefaults && !strictSemantics) { QScriptValue sittingPoints = engine->newObject(); for (int i = 0; i < _sittingPoints.size(); ++i) { QScriptValue sittingPoint = engine->newObject(); @@ -552,7 +556,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_ALWAYS(sittingPoints, sittingPoints); // gettable, but not settable } - if (!skipDefaults) { + if (!skipDefaults && !strictSemantics) { AABox aaBox = getAABox(); QScriptValue boundingBox = engine->newObject(); QScriptValue bottomRightNear = vec3toScriptValue(engine, aaBox.getCorner()); @@ -567,7 +571,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool } QString textureNamesStr = QJsonDocument::fromVariant(_textureNames).toJson(); - if (!skipDefaults) { + if (!skipDefaults && !strictSemantics) { COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_NO_SKIP(originalTextures, textureNamesStr); // gettable, but not settable } @@ -584,7 +588,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_OWNING_AVATAR_ID, owningAvatarID); // Rendering info - if (!skipDefaults) { + if (!skipDefaults && !strictSemantics) { QScriptValue renderInfo = engine->newObject(); // currently only supported by models diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index 20491bdaf5..dd81792283 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -73,7 +73,7 @@ public: EntityTypes::EntityType getType() const { return _type; } void setType(EntityTypes::EntityType type) { _type = type; } - virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime = false) const; + virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime = false, bool strictSemantics = false) const; virtual void copyFromScriptValue(const QScriptValue& object, bool honorReadOnly); static QScriptValue entityPropertyFlagsToScriptValue(QScriptEngine* engine, const EntityPropertyFlags& flags); @@ -93,6 +93,8 @@ public: void debugDump() const; void setLastEdited(quint64 usecTime); + EntityPropertyFlags getDesiredProperties() { return _desiredProperties; } + void setDesiredProperties(EntityPropertyFlags properties) { _desiredProperties = properties; } // Note: DEFINE_PROPERTY(PROP_FOO, Foo, foo, type, value) creates the following methods and variables: // type getFoo() const; From 957b1d9ae434d514acca36da50c0261204133613 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 18 Jan 2017 13:22:54 -0800 Subject: [PATCH 07/25] pass just what is given to filter, more or less --- libraries/entities/src/EntityTree.cpp | 11 ++++++++--- libraries/entities/src/EntityTree.h | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index bb6bf204fb..8f5df26c17 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -925,13 +925,18 @@ void EntityTree::initEntityEditFilterEngine() { _hasEntityEditFilter = _entityEditFilterFunction.isFunction(); } -bool EntityTree::filterProperties(const EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged) { +bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged) { if (!_hasEntityEditFilter) { propertiesOut = propertiesIn; wasChanged = false; // not changed return true; // allowed } - QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, true, true); + auto oldProperties = propertiesIn.getDesiredProperties(); + auto specifiedProperties = propertiesIn.getChangedProperties(); + propertiesIn.setDesiredProperties(specifiedProperties); + QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, false, true, true); + propertiesIn.setDesiredProperties(oldProperties); + auto in = QJsonValue::fromVariant(inputValues.toVariant()); // grab json copy now, because the inputValues might be side effected by the filter. qDebug() << "fixme filter input" << propertiesIn << inputValues.toVariant() << in; QScriptValueList args; @@ -950,7 +955,7 @@ bool EntityTree::filterProperties(const EntityItemProperties& propertiesIn, Enti wasChanged = !eq; } - qDebug() << "fixme filter result" << propertiesOut << result.toVariant() << "accepted:" << accepted << "changed:" << (accepted && wasChanged); + qDebug() << "fixme filter accepted:" << accepted << " changed : " << (accepted && wasChanged) << " result:" << propertiesOut << result.toVariant(); return accepted; } diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 7cdf1b0c77..29c51560fa 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -352,7 +352,7 @@ protected: float _maxTmpEntityLifetime { DEFAULT_MAX_TMP_ENTITY_LIFETIME }; - bool filterProperties(const EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged); + bool filterProperties(EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged); QString _entityEditFilter; bool _hasEntityEditFilter{ false }; QScriptEngine _entityEditFilterEngine; From 90c9c5b137dd1d3495534c26c68ff66252bad8c4 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 18 Jan 2017 14:32:05 -0800 Subject: [PATCH 08/25] logging change --- libraries/entities/src/EntityTree.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 8f5df26c17..e6dc0de185 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -938,7 +938,6 @@ bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItem propertiesIn.setDesiredProperties(oldProperties); auto in = QJsonValue::fromVariant(inputValues.toVariant()); // grab json copy now, because the inputValues might be side effected by the filter. - qDebug() << "fixme filter input" << propertiesIn << inputValues.toVariant() << in; QScriptValueList args; args << inputValues; @@ -950,12 +949,17 @@ bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItem // Javascript objects are == only if they are the same object. To compare arbitrary values, we need to use JSON. auto out = QJsonValue::fromVariant(result.toVariant()); - bool eq = in == out; - qDebug() << "in:" << in << "out:" << out << "eq:" << eq; - wasChanged = !eq; + wasChanged = in != out; + if (wasChanged) { + // Logging will be removed eventually, but for now, the behavior is so fragile that it's worth logging. + qCDebug(entities) << "filter accepted. changed:" << wasChanged; + qCDebug(entities) << " in:" << in; + qCDebug(entities) << " out:" << out; + } + } else { + qCDebug(entities) << "filter rejected. in:" << in; } - qDebug() << "fixme filter accepted:" << accepted << " changed : " << (accepted && wasChanged) << " result:" << propertiesOut << result.toVariant(); return accepted; } @@ -1056,11 +1060,9 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c if (!allowed || wasChanged) { bumpTimestamp(properties); if (properties.getSimulationOwner().getID() == senderNode->getUUID()) { - qDebug() << "fixme Suppressing ownership from" << senderNode->getUUID(); properties.setSimulationOwner(QUuid(), 0); } } - qDebug() << "fixme final:" << properties; endFilter = usecTimestampNow(); // search for the entity by EntityItemID From 7b986dcac64f2b5aafb5a8cb1ea270ce48db64ee Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Fri, 20 Jan 2017 17:03:52 -0800 Subject: [PATCH 09/25] works, but removes an optimization we'd like to keep --- libraries/entities/src/EntityItem.cpp | 4 +++- libraries/entities/src/EntityTree.cpp | 7 +++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 00c9ab2dde..a295319857 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -349,7 +349,6 @@ int EntityItem::expectedBytes() { return MINIMUM_HEADER_BYTES; } - // clients use this method to unpack FULL updates from entity-server int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLeftToRead, ReadBitstreamToTreeParams& args) { if (args.bitstreamVersion < VERSION_ENTITIES_SUPPORT_SPLIT_MTU) { @@ -667,6 +666,9 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef // entity-server is trying to clear our ownership (probably at our own request) // but we actually want to own it, therefore we ignore this clear event // and pretend that we own it (we assume we'll recover it soon) + + // However, for now, when the server uses a newer time than what we sent, listen to what we're told. + if (overwriteLocalData) weOwnSimulation = false; } else if (_simulationOwner.set(newSimOwner)) { _dirtyFlags |= Simulation::DIRTY_SIMULATOR_ID; somethingChanged = true; diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index e6dc0de185..9d5d14402d 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -952,7 +952,7 @@ bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItem wasChanged = in != out; if (wasChanged) { // Logging will be removed eventually, but for now, the behavior is so fragile that it's worth logging. - qCDebug(entities) << "filter accepted. changed:" << wasChanged; + qCDebug(entities) << "filter accepted. changed: true"; qCDebug(entities) << " in:" << in; qCDebug(entities) << " out:" << out; } @@ -1059,9 +1059,8 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c } if (!allowed || wasChanged) { bumpTimestamp(properties); - if (properties.getSimulationOwner().getID() == senderNode->getUUID()) { - properties.setSimulationOwner(QUuid(), 0); - } + // For now, free ownership on any modification. + properties.clearSimulationOwner(); } endFilter = usecTimestampNow(); From f34f90841d01d5bfd2d7664992f4e9bffe323578 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Mon, 23 Jan 2017 13:08:33 -0800 Subject: [PATCH 10/25] example --- script-archive/entity-server-filter-example.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 script-archive/entity-server-filter-example.js diff --git a/script-archive/entity-server-filter-example.js b/script-archive/entity-server-filter-example.js new file mode 100644 index 0000000000..2ccb550acd --- /dev/null +++ b/script-archive/entity-server-filter-example.js @@ -0,0 +1,15 @@ +function filter(p) { + /* block comments are ok, but not double-slash end-of-line-comments */ + + /* Simple example: if someone specifies name, add an 'x' to it. Note that print is ok to use. */ + if (p.name) {p.name += 'x'; print('fixme name', p. name);} + + /* This example clamps y. A better filter would probably zero y component of velocity and acceleration. */ + if (p.position) {p.position.y = Math.min(1, p.position.y); print('fixme p.y', p.position.y);} + + /* Can also reject altogether */ + if (p.userData) { return false; } + + /* If we make it this far, return the (possibly modified) properties. */ + return p; +} From 0a4634446d217e1ab12ed17f20274e8eb0230078 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Mon, 23 Jan 2017 13:09:28 -0800 Subject: [PATCH 11/25] typo --- libraries/entities/src/EntityItemProperties.cpp | 4 ++-- libraries/entities/src/EntityItemProperties.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index a16a4fbaba..925eb5d351 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -346,7 +346,7 @@ EntityPropertyFlags EntityItemProperties::getChangedProperties() const { return changedProperties; } -QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime, bool strictSemantics) const { +QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnkownCreateTime, bool strictSemantics) const { // If strictSemantics is true and skipDefaults is false, then all and only those properties are copied in the property flag // is included in _desiredProperties, or is one of the specially enumerated ALWAYS properties below. // (There may be exceptions, but if so, they are bugs.) @@ -354,7 +354,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool QScriptValue properties = engine->newObject(); EntityItemProperties defaultEntityProperties; - if (_created == UNKNOWN_CREATED_TIME && !allowUknownCreateTime) { + if (_created == UNKNOWN_CREATED_TIME && !allowUnkownCreateTime) { // No entity properties can have been set so return without setting any default, zero property values. return properties; } diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index dd81792283..bc7263bd8e 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -73,7 +73,7 @@ public: EntityTypes::EntityType getType() const { return _type; } void setType(EntityTypes::EntityType type) { _type = type; } - virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime = false, bool strictSemantics = false) const; + virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnkownCreateTime = false, bool strictSemantics = false) const; virtual void copyFromScriptValue(const QScriptValue& object, bool honorReadOnly); static QScriptValue entityPropertyFlagsToScriptValue(QScriptEngine* engine, const EntityPropertyFlags& flags); From ada685a376b61c119d969b06d7ee09c20ae618bd Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Mon, 23 Jan 2017 15:31:55 -0800 Subject: [PATCH 12/25] cr changes --- libraries/entities/src/EntityItemProperties.cpp | 6 +++--- libraries/entities/src/EntityItemProperties.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 925eb5d351..77a0209143 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -346,15 +346,15 @@ EntityPropertyFlags EntityItemProperties::getChangedProperties() const { return changedProperties; } -QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnkownCreateTime, bool strictSemantics) const { - // If strictSemantics is true and skipDefaults is false, then all and only those properties are copied in the property flag +QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnknownCreateTime, bool strictSemantics) const { + // If strictSemantics is true and skipDefaults is false, then all and only those properties are copied for which the property flag // is included in _desiredProperties, or is one of the specially enumerated ALWAYS properties below. // (There may be exceptions, but if so, they are bugs.) // In all other cases, you are welcome to inspect the code and try to figure out what was intended. I wish you luck. -HRS 1/18/17 QScriptValue properties = engine->newObject(); EntityItemProperties defaultEntityProperties; - if (_created == UNKNOWN_CREATED_TIME && !allowUnkownCreateTime) { + if (_created == UNKNOWN_CREATED_TIME && !allowUnknownCreateTime) { // No entity properties can have been set so return without setting any default, zero property values. return properties; } diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index bc7263bd8e..d8b148bdb6 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -73,7 +73,7 @@ public: EntityTypes::EntityType getType() const { return _type; } void setType(EntityTypes::EntityType type) { _type = type; } - virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnkownCreateTime = false, bool strictSemantics = false) const; + virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnknownCreateTime = false, bool strictSemantics = false) const; virtual void copyFromScriptValue(const QScriptValue& object, bool honorReadOnly); static QScriptValue entityPropertyFlagsToScriptValue(QScriptEngine* engine, const EntityPropertyFlags& flags); From 3a17a64c991108a04d1dfc1d817d3db028cbabc1 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Tue, 24 Jan 2017 11:56:50 -0800 Subject: [PATCH 13/25] V1 of these filters --- .../entity-server-filter-example.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/script-archive/entity-server-filter-example.js b/script-archive/entity-server-filter-example.js index 2ccb550acd..ce35964a79 100644 --- a/script-archive/entity-server-filter-example.js +++ b/script-archive/entity-server-filter-example.js @@ -9,6 +9,25 @@ function filter(p) { /* Can also reject altogether */ if (p.userData) { return false; } + + /* Reject if modifications made to Model properties */ + if (p.modelURL || p.compoundShapeURL || p.shape || p.shapeType || p.url || p.fps || p.currentFrame || p.running || p.loop || p.firstFrame || p.lastFrame || p.hold || p.textures || p.xTextureURL || p.yTextureURL || p.zTextureURL) { return false; } + + /* Clamp velocity to 10 units/second. Zeroing each component of acceleration keeps us from slamming.*/ + if (p.velocity) { + if (p.velocity.x > 10) { + p.velocity.x = 10; + p.acceleration.x = 0; + } + if (p.velocity.y > 10) { + p.velocity.y = 10; + p.acceleration.y = 0; + } + if (p.velocity.z > 10) { + p.velocity.z = 10; + p.acceleration.z = 0; + } + } /* If we make it this far, return the (possibly modified) properties. */ return p; From da914f0a4fbf624113e49219c8ceb94b903755bb Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Tue, 24 Jan 2017 11:59:31 -0800 Subject: [PATCH 14/25] bypass entity-server edit filter if you have lock/unlock rights --- libraries/entities/src/EntityTree.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 9d5d14402d..dbf5cbe72a 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1053,7 +1053,8 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c startFilter = usecTimestampNow(); bool wasChanged = false; - bool allowed = filterProperties(properties, properties, wasChanged); + // Having (un)lock rights bypasses the filter. + bool allowed = senderNode->isAllowedEditor() || filterProperties(properties, properties, wasChanged); if (!allowed) { properties = EntityItemProperties(); } From 0e16686fb8ab0b5dab36e02c8b0749dacaa0236f Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Tue, 24 Jan 2017 12:11:33 -0800 Subject: [PATCH 15/25] Change the velocity filter to 5 --- script-archive/entity-server-filter-example.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/script-archive/entity-server-filter-example.js b/script-archive/entity-server-filter-example.js index ce35964a79..e3a0426087 100644 --- a/script-archive/entity-server-filter-example.js +++ b/script-archive/entity-server-filter-example.js @@ -13,18 +13,18 @@ function filter(p) { /* Reject if modifications made to Model properties */ if (p.modelURL || p.compoundShapeURL || p.shape || p.shapeType || p.url || p.fps || p.currentFrame || p.running || p.loop || p.firstFrame || p.lastFrame || p.hold || p.textures || p.xTextureURL || p.yTextureURL || p.zTextureURL) { return false; } - /* Clamp velocity to 10 units/second. Zeroing each component of acceleration keeps us from slamming.*/ + /* Clamp velocity to 5 units/second. Zeroing each component of acceleration keeps us from slamming.*/ if (p.velocity) { - if (p.velocity.x > 10) { - p.velocity.x = 10; + if (p.velocity.x > 5) { + p.velocity.x = 5; p.acceleration.x = 0; } - if (p.velocity.y > 10) { - p.velocity.y = 10; + if (p.velocity.y > 5) { + p.velocity.y = 5; p.acceleration.y = 0; } - if (p.velocity.z > 10) { - p.velocity.z = 10; + if (p.velocity.z > 5) { + p.velocity.z = 5; p.acceleration.z = 0; } } From 53173d8f037a71cf33898ede0b25869263b3045b Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Tue, 24 Jan 2017 12:50:18 -0800 Subject: [PATCH 16/25] use var for maxvelocity --- script-archive/entity-server-filter-example.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/script-archive/entity-server-filter-example.js b/script-archive/entity-server-filter-example.js index e3a0426087..4b3a8a7212 100644 --- a/script-archive/entity-server-filter-example.js +++ b/script-archive/entity-server-filter-example.js @@ -13,18 +13,19 @@ function filter(p) { /* Reject if modifications made to Model properties */ if (p.modelURL || p.compoundShapeURL || p.shape || p.shapeType || p.url || p.fps || p.currentFrame || p.running || p.loop || p.firstFrame || p.lastFrame || p.hold || p.textures || p.xTextureURL || p.yTextureURL || p.zTextureURL) { return false; } - /* Clamp velocity to 5 units/second. Zeroing each component of acceleration keeps us from slamming.*/ + /* Clamp velocity to maxVelocity units/second. Zeroing each component of acceleration keeps us from slamming.*/ + var maxVelocity = 5; if (p.velocity) { - if (p.velocity.x > 5) { - p.velocity.x = 5; + if (p.velocity.x > maxVelocity) { + p.velocity.x = maxVelocity; p.acceleration.x = 0; } - if (p.velocity.y > 5) { - p.velocity.y = 5; + if (p.velocity.y > maxVelocity) { + p.velocity.y = maxVelocity; p.acceleration.y = 0; } - if (p.velocity.z > 5) { - p.velocity.z = 5; + if (p.velocity.z > maxVelocity) { + p.velocity.z = maxVelocity; p.acceleration.z = 0; } } From 9796a476eacabd4775de99796acd327335811ef0 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Tue, 24 Jan 2017 15:08:17 -0800 Subject: [PATCH 17/25] Clamp negative velocity too --- script-archive/entity-server-filter-example.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/script-archive/entity-server-filter-example.js b/script-archive/entity-server-filter-example.js index 4b3a8a7212..ab4d813927 100644 --- a/script-archive/entity-server-filter-example.js +++ b/script-archive/entity-server-filter-example.js @@ -16,16 +16,16 @@ function filter(p) { /* Clamp velocity to maxVelocity units/second. Zeroing each component of acceleration keeps us from slamming.*/ var maxVelocity = 5; if (p.velocity) { - if (p.velocity.x > maxVelocity) { - p.velocity.x = maxVelocity; + if (Math.abs(p.velocity.x) > maxVelocity) { + p.velocity.x = Math.sign(p.velocity.x) * maxVelocity; p.acceleration.x = 0; } - if (p.velocity.y > maxVelocity) { - p.velocity.y = maxVelocity; + if (Math.abs(p.velocity.y) > maxVelocity) { + p.velocity.y = Math.sign(p.velocity.y) * maxVelocity; p.acceleration.y = 0; } - if (p.velocity.z > maxVelocity) { - p.velocity.z = maxVelocity; + if (Math.abs(p.velocity.z) > maxVelocity) { + p.velocity.z = Math.sign(p.velocity.z) * maxVelocity; p.acceleration.z = 0; } } From 8eff3b1e83b11b31f0ca25817bbbd9dd07229717 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Tue, 24 Jan 2017 16:14:41 -0800 Subject: [PATCH 18/25] restructure initialization so that server can do server-like things to get the filter program from file --- assignment-client/src/entities/EntityServer.cpp | 8 ++++---- assignment-client/src/entities/EntityServer.h | 3 +++ libraries/entities/src/EntityTree.cpp | 10 +++++----- libraries/entities/src/EntityTree.h | 10 ++++------ 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index e5c22c3d34..84d9b198ea 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -286,11 +286,11 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio tree->setEntityScriptSourceWhitelist(""); } - QString entityEditFilter; - if (readOptionString("entityEditFilter", settingsSectionObject, entityEditFilter)) { - tree->setEntityEditFilter(entityEditFilter); + if (readOptionString("entityEditFilter", settingsSectionObject, _entityEditFilter)) { + // FIXME: Fetch script from file synchronously. We don't want the server processing edits while a restarting entity server is fetching from a DOS'd source. + _entityEditFilterEngine.evaluate(_entityEditFilter); + tree->initEntityEditFilterEngine(&_entityEditFilterEngine); } - tree->initEntityEditFilterEngine(); // whether supplied or not. } void EntityServer::nodeAdded(SharedNodePointer node) { diff --git a/assignment-client/src/entities/EntityServer.h b/assignment-client/src/entities/EntityServer.h index 0486a97ede..469ac21dd9 100644 --- a/assignment-client/src/entities/EntityServer.h +++ b/assignment-client/src/entities/EntityServer.h @@ -76,6 +76,9 @@ private: QReadWriteLock _viewerSendingStatsLock; QMap> _viewerSendingStats; + + QString _entityEditFilter; + QScriptEngine _entityEditFilterEngine; }; #endif // hifi_EntityServer_h diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 9d5d14402d..b4d209033f 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -918,15 +918,15 @@ void EntityTree::fixupTerseEditLogging(EntityItemProperties& properties, QListglobalObject(); _entityEditFilterFunction = global.property("filter"); _hasEntityEditFilter = _entityEditFilterFunction.isFunction(); } bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged) { - if (!_hasEntityEditFilter) { + if (!_hasEntityEditFilter || !_entityEditFilterEngine) { propertiesOut = propertiesIn; wasChanged = false; // not changed return true; // allowed @@ -934,7 +934,7 @@ bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItem auto oldProperties = propertiesIn.getDesiredProperties(); auto specifiedProperties = propertiesIn.getChangedProperties(); propertiesIn.setDesiredProperties(specifiedProperties); - QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, false, true, true); + QScriptValue inputValues = propertiesIn.copyToScriptValue(_entityEditFilterEngine, false, true, true); propertiesIn.setDesiredProperties(oldProperties); auto in = QJsonValue::fromVariant(inputValues.toVariant()); // grab json copy now, because the inputValues might be side effected by the filter. diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 29c51560fa..a643c481cc 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -65,7 +65,6 @@ public: void setEntityMaxTmpLifetime(float maxTmpEntityLifetime) { _maxTmpEntityLifetime = maxTmpEntityLifetime; } void setEntityScriptSourceWhitelist(const QString& entityScriptSourceWhitelist); - void setEntityEditFilter(const QString& entityEditFilter) { _entityEditFilter = entityEditFilter; } /// Implements our type specific root element factory virtual OctreeElementPointer createNewElement(unsigned char* octalCode = NULL) override; @@ -263,7 +262,7 @@ public: void notifyNewCollisionSoundURL(const QString& newCollisionSoundURL, const EntityItemID& entityID); - void initEntityEditFilterEngine(); + void initEntityEditFilterEngine(QScriptEngine* engine); static const float DEFAULT_MAX_TMP_ENTITY_LIFETIME; @@ -353,11 +352,10 @@ protected: float _maxTmpEntityLifetime { DEFAULT_MAX_TMP_ENTITY_LIFETIME }; bool filterProperties(EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged); - QString _entityEditFilter; bool _hasEntityEditFilter{ false }; - QScriptEngine _entityEditFilterEngine; - QScriptValue _entityEditFilterFunction; - QScriptValue _nullObjectForFilter; + QScriptEngine* _entityEditFilterEngine{}; + QScriptValue _entityEditFilterFunction{}; + QScriptValue _nullObjectForFilter{}; QStringList _entityScriptSourceWhitelist; }; From 9ce3dc134af628578f03704cc394a77bf8262200 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Tue, 24 Jan 2017 16:53:50 -0800 Subject: [PATCH 19/25] V1 of the filter --- .../entity-server-filter-example.js | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/script-archive/entity-server-filter-example.js b/script-archive/entity-server-filter-example.js index ab4d813927..4d4f7273f1 100644 --- a/script-archive/entity-server-filter-example.js +++ b/script-archive/entity-server-filter-example.js @@ -29,6 +29,28 @@ function filter(p) { p.acceleration.z = 0; } } + + /* Define an axis-aligned zone in which entities are not allowed to enter. */ + /* This example zone corresponds to an area to the right of the spawnpoint + in your Sandbox. It's an area near the big rock to the right. If an entity + enters the zone, it'll move behind the rock.*/ + var boxMin = {x: 25.5, y: -0.48, z: -9.9}; + var boxMax = {x: 31.1, y: 4, z: -3.79}; + var zero = {x: 0.0, y: 0.0, z: 0.0}; + + if (p.position) { + var x = p.position.x; + var y = p.position.y; + var z = p.position.z; + if ((x > boxMin.x && x < boxMax.x) && + (y > boxMin.y && y < boxMax.y) && + (z > boxMin.z && z < boxMax.z)) { + /* Move it to the origin of the zone */ + p.position = boxMin; + p.velocity = zero; + p.acceleration = zero; + } + } /* If we make it this far, return the (possibly modified) properties. */ return p; From df91ef5f5fcc4b8ac242b40c628c5f2c8d14114e Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 25 Jan 2017 14:22:42 -0800 Subject: [PATCH 20/25] blocking fetch of http scripts --- .../src/entities/EntityServer.cpp | 57 +++++++++++++++++-- assignment-client/src/entities/EntityServer.h | 6 +- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index 84d9b198ea..0e71d82c3a 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -9,9 +9,12 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // +#include #include #include #include +#include +#include #include "EntityServer.h" #include "EntityServerConsts.h" @@ -26,6 +29,10 @@ EntityServer::EntityServer(ReceivedMessage& message) : OctreeServer(message), _entitySimulation(NULL) { + ResourceManager::init(); + DependencyManager::set(); + DependencyManager::set(); + auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); packetReceiver.registerListenerForTypes({ PacketType::EntityAdd, PacketType::EntityEdit, PacketType::EntityErase }, this, "handleEntityPacket"); @@ -286,13 +293,55 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio tree->setEntityScriptSourceWhitelist(""); } - if (readOptionString("entityEditFilter", settingsSectionObject, _entityEditFilter)) { - // FIXME: Fetch script from file synchronously. We don't want the server processing edits while a restarting entity server is fetching from a DOS'd source. - _entityEditFilterEngine.evaluate(_entityEditFilter); - tree->initEntityEditFilterEngine(&_entityEditFilterEngine); + if (readOptionString("entityEditFilter", settingsSectionObject, _entityEditFilter) && !_entityEditFilter.isEmpty()) { + // Fetch script from file synchronously. We don't want the server processing edits while a restarting entity server is fetching from a DOS'd source. + QUrl scriptURL(_entityEditFilter); + + // The following should be abstracted out for use in Agent.cpp (and maybe later AvatarMixer.cpp) + if (scriptURL.scheme().isEmpty() || (scriptURL.scheme() == URL_SCHEME_FILE)) { + qWarning() << "Cannot load script from local filesystem, because assignment may be on a different computer."; + scriptRequestFinished(); + return; + } + auto scriptRequest = ResourceManager::createResourceRequest(this, scriptURL); + if (!scriptRequest) { + qWarning() << "Could not create ResourceRequest for Agent script at" << scriptURL.toString(); + scriptRequestFinished(); + return; + } + // Agent.cpp sets up a timeout here, but that is unnecessary, as ResourceRequest has its own. + connect(scriptRequest, &ResourceRequest::finished, this, &EntityServer::scriptRequestFinished); + // FIXME: handle atp rquests setup here. See Agent::requestScript() + qInfo() << "Requesting script at URL" << qPrintable(scriptRequest->getUrl().toString()); + scriptRequest->send(); + _scriptRequestLoop.exec(); // Block here, but allow the request to be processed and its signals to be handled. } } +void EntityServer::scriptRequestFinished() { + auto scriptRequest = qobject_cast(sender()); + if (scriptRequest && scriptRequest->getResult() == ResourceRequest::Success) { + auto scriptContents = scriptRequest->getData(); + qInfo() << "Downloaded script:" << scriptContents; + _entityEditFilterEngine.evaluate(scriptContents); + std::static_pointer_cast(_tree)->initEntityEditFilterEngine(&_entityEditFilterEngine); + scriptRequest->deleteLater(); + if (_scriptRequestLoop.isRunning()) { + _scriptRequestLoop.quit(); + } + return; + } else if (scriptRequest) { + qCritical() << "Failed to download script at" << scriptRequest->getUrl().toString(); + // See HTTPResourceRequest::onRequestFinished for interpretation of codes. For example, a 404 is code 6 and 403 is 3. A timeout is 2. Go figure. + qCritical() << "ResourceRequest error was" << scriptRequest->getResult(); + } else { + qCritical() << "Failed to create script request."; + } + // Hard stop of the assignment client on failure. We don't want anyone to think they have a filter in place when they don't. + // Alas, only indications will be the above logging with assignment client restarting repeatedly, and clients will not see any entities. + stop(); +} + void EntityServer::nodeAdded(SharedNodePointer node) { EntityTreePointer tree = std::static_pointer_cast(_tree); tree->knowAvatarID(node->getUUID()); diff --git a/assignment-client/src/entities/EntityServer.h b/assignment-client/src/entities/EntityServer.h index 469ac21dd9..25270c9dd5 100644 --- a/assignment-client/src/entities/EntityServer.h +++ b/assignment-client/src/entities/EntityServer.h @@ -69,6 +69,7 @@ protected: private slots: void handleEntityPacket(QSharedPointer message, SharedNodePointer senderNode); + void scriptRequestFinished(); private: SimpleEntitySimulationPointer _entitySimulation; @@ -77,8 +78,9 @@ private: QReadWriteLock _viewerSendingStatsLock; QMap> _viewerSendingStats; - QString _entityEditFilter; - QScriptEngine _entityEditFilterEngine; + QString _entityEditFilter{}; + QScriptEngine _entityEditFilterEngine{}; + QEventLoop _scriptRequestLoop{}; }; #endif // hifi_EntityServer_h From 34e9c5debc914861a1a14e3c2acaae2c840389a9 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 25 Jan 2017 14:56:06 -0800 Subject: [PATCH 21/25] initial script error checking --- .../src/entities/EntityServer.cpp | 50 ++++++++++++++++--- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index 0e71d82c3a..ae227ea425 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -318,20 +318,58 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio } } +// Copied from ScriptEngine.cpp. We should make this a class method for reuse. +// Note: I've deliberately stopped short of using ScriptEngine instead of QScriptEngine, as that is out of project scope at this point. +static bool hasCorrectSyntax(const QScriptProgram& program) { + const auto syntaxCheck = QScriptEngine::checkSyntax(program.sourceCode()); + if (syntaxCheck.state() != QScriptSyntaxCheckResult::Valid) { + const auto error = syntaxCheck.errorMessage(); + const auto line = QString::number(syntaxCheck.errorLineNumber()); + const auto column = QString::number(syntaxCheck.errorColumnNumber()); + const auto message = QString("[SyntaxError] %1 in %2:%3(%4)").arg(error, program.fileName(), line, column); + qCritical() << qPrintable(message); + return false; + } + return true; +} +static bool hadUncaughtExceptions(QScriptEngine& engine, const QString& fileName) { + if (engine.hasUncaughtException()) { + const auto backtrace = engine.uncaughtExceptionBacktrace(); + const auto exception = engine.uncaughtException().toString(); + const auto line = QString::number(engine.uncaughtExceptionLineNumber()); + engine.clearExceptions(); + + static const QString SCRIPT_EXCEPTION_FORMAT = "[UncaughtException] %1 in %2:%3"; + auto message = QString(SCRIPT_EXCEPTION_FORMAT).arg(exception, fileName, line); + if (!backtrace.empty()) { + static const auto lineSeparator = "\n "; + message += QString("\n[Backtrace]%1%2").arg(lineSeparator, backtrace.join(lineSeparator)); + } + qCritical() << qPrintable(message); + return true; + } + return false; +} void EntityServer::scriptRequestFinished() { auto scriptRequest = qobject_cast(sender()); + const QString urlString = scriptRequest->getUrl().toString(); if (scriptRequest && scriptRequest->getResult() == ResourceRequest::Success) { auto scriptContents = scriptRequest->getData(); qInfo() << "Downloaded script:" << scriptContents; - _entityEditFilterEngine.evaluate(scriptContents); - std::static_pointer_cast(_tree)->initEntityEditFilterEngine(&_entityEditFilterEngine); - scriptRequest->deleteLater(); - if (_scriptRequestLoop.isRunning()) { - _scriptRequestLoop.quit(); + QScriptProgram program(scriptContents, urlString); + if (hasCorrectSyntax(program)) { + _entityEditFilterEngine.evaluate(scriptContents); + if (!hadUncaughtExceptions(_entityEditFilterEngine, urlString)) { + std::static_pointer_cast(_tree)->initEntityEditFilterEngine(&_entityEditFilterEngine); + scriptRequest->deleteLater(); + if (_scriptRequestLoop.isRunning()) { + _scriptRequestLoop.quit(); + } + } } return; } else if (scriptRequest) { - qCritical() << "Failed to download script at" << scriptRequest->getUrl().toString(); + qCritical() << "Failed to download script at" << urlString; // See HTTPResourceRequest::onRequestFinished for interpretation of codes. For example, a 404 is code 6 and 403 is 3. A timeout is 2. Go figure. qCritical() << "ResourceRequest error was" << scriptRequest->getResult(); } else { From b62f3e550a8dc3798f26e58c66a102eda795a486 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 25 Jan 2017 16:23:07 -0800 Subject: [PATCH 22/25] error check/log on execution, too --- assignment-client/src/entities/EntityServer.cpp | 4 +++- libraries/entities/src/EntityTree.cpp | 9 ++++++--- libraries/entities/src/EntityTree.h | 3 ++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index ae227ea425..ff93dbae09 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -360,7 +360,9 @@ void EntityServer::scriptRequestFinished() { if (hasCorrectSyntax(program)) { _entityEditFilterEngine.evaluate(scriptContents); if (!hadUncaughtExceptions(_entityEditFilterEngine, urlString)) { - std::static_pointer_cast(_tree)->initEntityEditFilterEngine(&_entityEditFilterEngine); + std::static_pointer_cast(_tree)->initEntityEditFilterEngine(&_entityEditFilterEngine, [this]() { + return hadUncaughtExceptions(_entityEditFilterEngine, _entityEditFilter); + }); scriptRequest->deleteLater(); if (_scriptRequestLoop.isRunning()) { _scriptRequestLoop.quit(); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index b4d209033f..9a0b0a3d1c 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -918,8 +918,9 @@ void EntityTree::fixupTerseEditLogging(EntityItemProperties& properties, QList entityEditFilterHadUncaughtExceptions) { _entityEditFilterEngine = engine; + _entityEditFilterHadUncaughtExceptions = entityEditFilterHadUncaughtExceptions; auto global = _entityEditFilterEngine->globalObject(); _entityEditFilterFunction = global.property("filter"); _hasEntityEditFilter = _entityEditFilterFunction.isFunction(); @@ -942,12 +943,14 @@ bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItem args << inputValues; QScriptValue result = _entityEditFilterFunction.call(_nullObjectForFilter, args); + if (_entityEditFilterHadUncaughtExceptions()) { + result = QScriptValue(); + } - propertiesOut.copyFromScriptValue(result, false); bool accepted = result.isObject(); // filters should return null or false to completely reject edit or add if (accepted) { + propertiesOut.copyFromScriptValue(result, false); // Javascript objects are == only if they are the same object. To compare arbitrary values, we need to use JSON. - auto out = QJsonValue::fromVariant(result.toVariant()); wasChanged = in != out; if (wasChanged) { diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index a643c481cc..16fb0f68e8 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -262,7 +262,7 @@ public: void notifyNewCollisionSoundURL(const QString& newCollisionSoundURL, const EntityItemID& entityID); - void initEntityEditFilterEngine(QScriptEngine* engine); + void initEntityEditFilterEngine(QScriptEngine* engine, std::function entityEditFilterHadUncaughtExceptions); static const float DEFAULT_MAX_TMP_ENTITY_LIFETIME; @@ -356,6 +356,7 @@ protected: QScriptEngine* _entityEditFilterEngine{}; QScriptValue _entityEditFilterFunction{}; QScriptValue _nullObjectForFilter{}; + std::function _entityEditFilterHadUncaughtExceptions; QStringList _entityScriptSourceWhitelist; }; From 1c30da42af33556be736564fe45f50b51ed682af Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 25 Jan 2017 17:01:07 -0800 Subject: [PATCH 23/25] change placeholder text in settings --- domain-server/resources/describe-settings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/domain-server/resources/describe-settings.json b/domain-server/resources/describe-settings.json index 3d5b17dffe..3ceb425ccb 100644 --- a/domain-server/resources/describe-settings.json +++ b/domain-server/resources/describe-settings.json @@ -1294,7 +1294,7 @@ "name": "entityEditFilter", "label": "Filter Entity Edits", "help": "Check all entity edits against this filter function.", - "placeholder": "function filter(properties) { return properties; }", + "placeholder": "url whose content is like: function filter(properties) { return properties; }", "default": "", "advanced": true }, From 4437063a3ae95d212efeb06e54ca1f92e9a0fdeb Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 25 Jan 2017 17:04:09 -0800 Subject: [PATCH 24/25] remove logging, now that we have error logging --- libraries/entities/src/EntityTree.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 90c33ea572..6795eb7ef9 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -957,14 +957,6 @@ bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItem // Javascript objects are == only if they are the same object. To compare arbitrary values, we need to use JSON. auto out = QJsonValue::fromVariant(result.toVariant()); wasChanged = in != out; - if (wasChanged) { - // Logging will be removed eventually, but for now, the behavior is so fragile that it's worth logging. - qCDebug(entities) << "filter accepted. changed: true"; - qCDebug(entities) << " in:" << in; - qCDebug(entities) << " out:" << out; - } - } else { - qCDebug(entities) << "filter rejected. in:" << in; } return accepted; From f9f0a4ea2d282023c3d25b25712be1d945b918fa Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Thu, 26 Jan 2017 14:05:16 -0800 Subject: [PATCH 25/25] fix startup-failure case --- assignment-client/src/entities/EntityServer.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index ff93dbae09..724571c111 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -367,9 +367,9 @@ void EntityServer::scriptRequestFinished() { if (_scriptRequestLoop.isRunning()) { _scriptRequestLoop.quit(); } + return; } } - return; } else if (scriptRequest) { qCritical() << "Failed to download script at" << urlString; // See HTTPResourceRequest::onRequestFinished for interpretation of codes. For example, a 404 is code 6 and 403 is 3. A timeout is 2. Go figure. @@ -380,6 +380,9 @@ void EntityServer::scriptRequestFinished() { // Hard stop of the assignment client on failure. We don't want anyone to think they have a filter in place when they don't. // Alas, only indications will be the above logging with assignment client restarting repeatedly, and clients will not see any entities. stop(); + if (_scriptRequestLoop.isRunning()) { + _scriptRequestLoop.quit(); + } } void EntityServer::nodeAdded(SharedNodePointer node) {