From dc62389739bcc9ece889230cdd07cc6a2cb04a2d Mon Sep 17 00:00:00 2001 From: Juan Linietsky Date: Fri, 18 Aug 2017 10:59:31 -0300 Subject: [PATCH] -Properly check limits to objects sent (regarding to size), fixes #9034 -Changed the way objects are marshalled and sent to the debugger -Editing debugged objects happens in the remote inspector now --- core/io/marshalls.cpp | 208 +++++++++++++++++++----------- core/io/marshalls.h | 21 ++- core/io/packet_peer.cpp | 16 ++- core/io/packet_peer.h | 2 + core/object.h | 1 + core/register_core_types.cpp | 3 +- core/script_debugger_remote.cpp | 60 ++++----- core/script_debugger_remote.h | 2 + editor/property_editor.cpp | 29 ++++- editor/script_editor_debugger.cpp | 14 +- 10 files changed, 232 insertions(+), 124 deletions(-) diff --git a/core/io/marshalls.cpp b/core/io/marshalls.cpp index 8eb40b61d77..c5d59f786dd 100644 --- a/core/io/marshalls.cpp +++ b/core/io/marshalls.cpp @@ -33,8 +33,28 @@ #include "reference.h" #include +void EncodedObjectAsID::_bind_methods() { + ClassDB::bind_method(D_METHOD("set_object_id", "id"), &EncodedObjectAsID::set_object_id); + ClassDB::bind_method(D_METHOD("get_object_id"), &EncodedObjectAsID::get_object_id); +} + +void EncodedObjectAsID::set_object_id(ObjectID p_id) { + id = p_id; +} + +ObjectID EncodedObjectAsID::get_object_id() const { + + return id; +} + +EncodedObjectAsID::EncodedObjectAsID() { + + id = 0; +} + #define ENCODE_MASK 0xFF #define ENCODE_FLAG_64 1 << 16 +#define ENCODE_FLAG_OBJECT_AS_ID 1 << 16 static Error _decode_string(const uint8_t *&buf, int &len, int *r_len, String &r_string) { ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA); @@ -381,56 +401,74 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int } break; case Variant::OBJECT: { - ERR_FAIL_COND_V(!p_allow_objects, ERR_UNAUTHORIZED); + if (type & ENCODE_FLAG_OBJECT_AS_ID) { + //this _is_ allowed + ObjectID val = decode_uint64(buf); + if (r_len) + (*r_len) += 8; - String str; - Error err = _decode_string(buf, len, r_len, str); - if (err) - return err; + if (val == 0) { + r_variant = (Object *)NULL; + } else { + Ref obj_as_id; + obj_as_id.instance(); + obj_as_id->set_object_id(val); - if (str == String()) { - r_variant = (Object *)NULL; - } else { - - Object *obj = ClassDB::instance(str); - - ERR_FAIL_COND_V(!obj, ERR_UNAVAILABLE); - ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA); - - int32_t count = decode_uint32(buf); - buf += 4; - len -= 4; - if (r_len) { - (*r_len) += 4; + r_variant = obj_as_id; } - for (int i = 0; i < count; i++) { + } else { + ERR_FAIL_COND_V(!p_allow_objects, ERR_UNAUTHORIZED); - str = String(); - err = _decode_string(buf, len, r_len, str); - if (err) - return err; + String str; + Error err = _decode_string(buf, len, r_len, str); + if (err) + return err; - Variant value; - int used; - err = decode_variant(value, buf, len, &used, p_allow_objects); - if (err) - return err; + if (str == String()) { + r_variant = (Object *)NULL; + } else { - buf += used; - len -= used; + Object *obj = ClassDB::instance(str); + + ERR_FAIL_COND_V(!obj, ERR_UNAVAILABLE); + ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA); + + int32_t count = decode_uint32(buf); + buf += 4; + len -= 4; if (r_len) { - (*r_len) += used; + (*r_len) += 4; } - obj->set(str, value); - } + for (int i = 0; i < count; i++) { - if (obj->cast_to()) { - REF ref = REF(obj->cast_to()); - r_variant = ref; - } else { - r_variant = obj; + str = String(); + err = _decode_string(buf, len, r_len, str); + if (err) + return err; + + Variant value; + int used; + err = decode_variant(value, buf, len, &used, p_allow_objects); + if (err) + return err; + + buf += used; + len -= used; + if (r_len) { + (*r_len) += used; + } + + obj->set(str, value); + } + + if (obj->cast_to()) { + REF ref = REF(obj->cast_to()); + r_variant = ref; + } else { + r_variant = obj; + } } } @@ -776,7 +814,7 @@ static void _encode_string(const String &p_string, uint8_t *&buf, int &r_len) { r_len++; //pad } -Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len) { +Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bool p_object_as_id) { uint8_t *buf = r_buffer; @@ -800,6 +838,11 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len) { flags |= ENCODE_FLAG_64; //always encode real as double } } break; + case Variant::OBJECT: { + if (p_object_as_id) { + flags |= ENCODE_FLAG_OBJECT_AS_ID; + } + } break; } if (buf) { @@ -1071,49 +1114,66 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len) { } break; case Variant::OBJECT: { - Object *obj = p_variant; - if (!obj) { + if (p_object_as_id) { + if (buf) { - encode_uint32(0, buf); - buf += 4; - r_len += 4; + + Object *obj = p_variant; + ObjectID id = 0; + if (obj && ObjectDB::instance_validate(obj)) { + id = obj->get_instance_id(); + } + + encode_uint64(id, buf); } + + r_len += 8; + } else { - _encode_string(obj->get_class(), buf, r_len); + Object *obj = p_variant; + if (!obj) { + if (buf) { + encode_uint32(0, buf); + buf += 4; + r_len += 4; + } + } else { + _encode_string(obj->get_class(), buf, r_len); - List props; - obj->get_property_list(&props); + List props; + obj->get_property_list(&props); - int pc = 0; - for (List::Element *E = props.front(); E; E = E->next()) { + int pc = 0; + for (List::Element *E = props.front(); E; E = E->next()) { - if (!(E->get().usage & PROPERTY_USAGE_STORAGE)) - continue; - pc++; - } + if (!(E->get().usage & PROPERTY_USAGE_STORAGE)) + continue; + pc++; + } - if (buf) { - encode_uint32(pc, buf); - buf += 4; - } + if (buf) { + encode_uint32(pc, buf); + buf += 4; + } - r_len += 4; + r_len += 4; - for (List::Element *E = props.front(); E; E = E->next()) { + for (List::Element *E = props.front(); E; E = E->next()) { - if (!(E->get().usage & PROPERTY_USAGE_STORAGE)) - continue; + if (!(E->get().usage & PROPERTY_USAGE_STORAGE)) + continue; - _encode_string(E->get().name, buf, r_len); + _encode_string(E->get().name, buf, r_len); - int len; - Error err = encode_variant(obj->get(E->get().name), buf, len); - if (err) - return err; - ERR_FAIL_COND_V(len % 4, ERR_BUG); - r_len += len; - if (buf) - buf += len; + int len; + Error err = encode_variant(obj->get(E->get().name), buf, len, p_object_as_id); + if (err) + return err; + ERR_FAIL_COND_V(len % 4, ERR_BUG); + r_len += len; + if (buf) + buf += len; + } } } @@ -1147,12 +1207,12 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len) { r_len++; //pad */ int len; - encode_variant(E->get(), buf, len); + encode_variant(E->get(), buf, len, p_object_as_id); ERR_FAIL_COND_V(len % 4, ERR_BUG); r_len += len; if (buf) buf += len; - encode_variant(d[E->get()], buf, len); + encode_variant(d[E->get()], buf, len, p_object_as_id); ERR_FAIL_COND_V(len % 4, ERR_BUG); r_len += len; if (buf) @@ -1174,7 +1234,7 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len) { for (int i = 0; i < v.size(); i++) { int len; - encode_variant(v.get(i), buf, len); + encode_variant(v.get(i), buf, len, p_object_as_id); ERR_FAIL_COND_V(len % 4, ERR_BUG); r_len += len; if (buf) diff --git a/core/io/marshalls.h b/core/io/marshalls.h index a6cc72b6914..234ae3b183a 100644 --- a/core/io/marshalls.h +++ b/core/io/marshalls.h @@ -32,8 +32,8 @@ #include "typedefs.h" +#include "reference.h" #include "variant.h" - /** * Miscellaneous helpers for marshalling data types, and encoding * in an endian independent way @@ -183,7 +183,22 @@ static inline double decode_double(const uint8_t *p_arr) { return md.d; } -Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int *r_len = NULL, bool p_allow_objects=true); -Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len); +class EncodedObjectAsID : public Reference { + GDCLASS(EncodedObjectAsID, Reference); + + ObjectID id; + +protected: + static void _bind_methods(); + +public: + void set_object_id(ObjectID p_id); + ObjectID get_object_id() const; + + EncodedObjectAsID(); +}; + +Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int *r_len = NULL, bool p_allow_objects = true); +Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bool p_object_as_id = false); #endif diff --git a/core/io/packet_peer.cpp b/core/io/packet_peer.cpp index 66f8eea1713..ca00b8b4809 100644 --- a/core/io/packet_peer.cpp +++ b/core/io/packet_peer.cpp @@ -92,7 +92,7 @@ Error PacketPeer::get_var(Variant &r_variant) const { Error PacketPeer::put_var(const Variant &p_packet) { int len; - Error err = encode_variant(p_packet, NULL, len); // compute len first + Error err = encode_variant(p_packet, NULL, len, !allow_object_decoding); // compute len first if (err) return err; @@ -101,7 +101,7 @@ Error PacketPeer::put_var(const Variant &p_packet) { uint8_t *buf = (uint8_t *)alloca(len); ERR_FAIL_COND_V(!buf, ERR_OUT_OF_MEMORY); - err = encode_variant(p_packet, buf, len); + err = encode_variant(p_packet, buf, len, !allow_object_decoding); ERR_FAIL_COND_V(err, err); return put_packet(buf, len); @@ -155,6 +155,8 @@ void PacketPeerStream::_bind_methods() { ClassDB::bind_method(D_METHOD("set_stream_peer", "peer"), &PacketPeerStream::_set_stream_peer); ClassDB::bind_method(D_METHOD("set_input_buffer_max_size", "max_size_bytes"), &PacketPeerStream::set_input_buffer_max_size); ClassDB::bind_method(D_METHOD("set_output_buffer_max_size", "max_size_bytes"), &PacketPeerStream::set_output_buffer_max_size); + ClassDB::bind_method(D_METHOD("get_input_buffer_max_size"), &PacketPeerStream::get_input_buffer_max_size); + ClassDB::bind_method(D_METHOD("get_output_buffer_max_size"), &PacketPeerStream::get_output_buffer_max_size); } Error PacketPeerStream::_poll_buffer() const { @@ -268,11 +270,21 @@ void PacketPeerStream::set_input_buffer_max_size(int p_max_size) { input_buffer.resize(next_power_of_2(p_max_size + 4)); } +int PacketPeerStream::get_input_buffer_max_size() const { + + return input_buffer.size() - 4; +} + void PacketPeerStream::set_output_buffer_max_size(int p_max_size) { output_buffer.resize(next_power_of_2(p_max_size + 4)); } +int PacketPeerStream::get_output_buffer_max_size() const { + + return output_buffer.size() - 4; +} + PacketPeerStream::PacketPeerStream() { int rbsize = GLOBAL_GET("network/limits/packet_peer_stream/max_buffer_po2"); diff --git a/core/io/packet_peer.h b/core/io/packet_peer.h index 3bd6876aa7c..597119f7f48 100644 --- a/core/io/packet_peer.h +++ b/core/io/packet_peer.h @@ -98,7 +98,9 @@ public: void set_stream_peer(const Ref &p_peer); void set_input_buffer_max_size(int p_max_size); + int get_input_buffer_max_size() const; void set_output_buffer_max_size(int p_max_size); + int get_output_buffer_max_size() const; PacketPeerStream(); }; diff --git a/core/object.h b/core/object.h index ebcd4afeb94..b694876505c 100644 --- a/core/object.h +++ b/core/object.h @@ -83,6 +83,7 @@ enum PropertyHint { PROPERTY_HINT_PROPERTY_OF_BASE_TYPE, ///< a property of a base type PROPERTY_HINT_PROPERTY_OF_INSTANCE, ///< a property of an instance PROPERTY_HINT_PROPERTY_OF_SCRIPT, ///< a property of a script & base + PROPERTY_HINT_OBJECT_TOO_BIG, ///< object is too big to send PROPERTY_HINT_MAX, }; diff --git a/core/register_core_types.cpp b/core/register_core_types.cpp index 43f781af55e..fca9afbf5e5 100644 --- a/core/register_core_types.cpp +++ b/core/register_core_types.cpp @@ -39,6 +39,7 @@ #include "input_map.h" #include "io/config_file.h" #include "io/http_client.h" +#include "io/marshalls.h" #include "io/packet_peer.h" #include "io/packet_peer_udp.h" #include "io/pck_packer.h" @@ -56,7 +57,6 @@ #include "project_settings.h" #include "translation.h" #include "undo_redo.h" - static ResourceFormatSaverBinary *resource_saver_binary = NULL; static ResourceFormatLoaderBinary *resource_loader_binary = NULL; static ResourceFormatImporter *resource_format_importer = NULL; @@ -157,6 +157,7 @@ void register_core_types() { ClassDB::register_class(); ClassDB::register_virtual_class(); ClassDB::register_class(); + ClassDB::register_class(); ip = IP::create(); diff --git a/core/script_debugger_remote.cpp b/core/script_debugger_remote.cpp index 44e86bca6a6..25f0044cc61 100644 --- a/core/script_debugger_remote.cpp +++ b/core/script_debugger_remote.cpp @@ -30,10 +30,10 @@ #include "script_debugger_remote.h" #include "io/ip.h" +#include "io/marshalls.h" #include "os/input.h" #include "os/os.h" #include "project_settings.h" - void ScriptDebuggerRemote::_send_video_memory() { List usage; @@ -120,6 +120,18 @@ static ObjectID safe_get_instance_id(const Variant &p_v) { } } +void ScriptDebuggerRemote::_put_variable(const String &p_name, const Variant &p_variable) { + + packet_peer_stream->put_var(p_name); + int len = 0; + Error err = encode_variant(p_variable, NULL, len); + if (len > packet_peer_stream->get_output_buffer_max_size()) { //limit to max size + packet_peer_stream->put_var(Variant()); + } else { + packet_peer_stream->put_var(p_variable); + } +} + void ScriptDebuggerRemote::debug(ScriptLanguage *p_script, bool p_can_continue) { //this function is called when there is a debugger break (bug on script) @@ -210,14 +222,7 @@ void ScriptDebuggerRemote::debug(ScriptLanguage *p_script, bool p_can_continue) while (E) { - if (F->get().get_type() == Variant::OBJECT) { - packet_peer_stream->put_var("*" + E->get()); - String pretty_print = F->get().operator String(); - packet_peer_stream->put_var(pretty_print.ascii().get_data()); - } else { - packet_peer_stream->put_var(E->get()); - packet_peer_stream->put_var(F->get()); - } + _put_variable(E->get(), F->get()); E = E->next(); F = F->next(); @@ -231,15 +236,7 @@ void ScriptDebuggerRemote::debug(ScriptLanguage *p_script, bool p_can_continue) List::Element *F = local_vals.front(); while (E) { - - if (F->get().get_type() == Variant::OBJECT) { - packet_peer_stream->put_var("*" + E->get()); - String pretty_print = F->get().operator String(); - packet_peer_stream->put_var(pretty_print.ascii().get_data()); - } else { - packet_peer_stream->put_var(E->get()); - packet_peer_stream->put_var(F->get()); - } + _put_variable(E->get(), F->get()); E = E->next(); F = F->next(); @@ -566,30 +563,19 @@ void ScriptDebuggerRemote::_send_object_id(ObjectID p_id) { } Variant var = obj->get(E->get().name); + packet_peer_stream->put_var(E->get().type); + //only send information that can be sent.. - if (E->get().type == Variant::OBJECT || var.get_type() == Variant::OBJECT) { + int len = 0; //test how big is this to encode + encode_variant(var, NULL, len); - ObjectID id2; - Object *obj = var; - if (obj) { - id2 = obj->get_instance_id(); - } else { - id2 = 0; - } - - packet_peer_stream->put_var(Variant::INT); //hint string - packet_peer_stream->put_var(PROPERTY_HINT_OBJECT_ID); //hint - packet_peer_stream->put_var(E->get().hint_string); //hint string - packet_peer_stream->put_var(id2); //value + if (len > packet_peer_stream->get_output_buffer_max_size()) { //limit to max size + packet_peer_stream->put_var(PROPERTY_HINT_OBJECT_TOO_BIG); + packet_peer_stream->put_var(""); + packet_peer_stream->put_var(Variant()); } else { - packet_peer_stream->put_var(E->get().type); packet_peer_stream->put_var(E->get().hint); packet_peer_stream->put_var(E->get().hint_string); - //only send information that can be sent.. - - if (var.get_type() >= Variant::DICTIONARY) { - var = Array(); //send none for now, may be to big - } packet_peer_stream->put_var(var); } } diff --git a/core/script_debugger_remote.h b/core/script_debugger_remote.h index 924e3774a25..cf75c0eb4a1 100644 --- a/core/script_debugger_remote.h +++ b/core/script_debugger_remote.h @@ -126,6 +126,8 @@ class ScriptDebuggerRemote : public ScriptDebugger { Vector profile_frame_data; + void _put_variable(const String &p_name, const Variant &p_variable); + public: struct ResourceUsage { diff --git a/editor/property_editor.cpp b/editor/property_editor.cpp index 2f47d3e130b..0ec83d8a36c 100644 --- a/editor/property_editor.cpp +++ b/editor/property_editor.cpp @@ -38,6 +38,7 @@ #include "editor_node.h" #include "editor_settings.h" #include "io/image_loader.h" +#include "io/marshalls.h" #include "io/resource_loader.h" #include "multi_node_edit.h" #include "os/input.h" @@ -2320,7 +2321,15 @@ void PropertyEditor::set_item_text(TreeItem *p_item, int p_type, const String &p } break; case Variant::OBJECT: { - if (obj->get(p_name).get_type() == Variant::NIL || obj->get(p_name).operator RefPtr().is_null()) { + Ref encoded = obj->get(p_name); //for debugger and remote tools + + if (encoded.is_valid()) { + + p_item->set_text(1, "Object: " + itos(encoded->get_object_id())); + p_item->set_icon(1, Ref()); + p_item->set_custom_as_button(1, true); + + } else if (obj->get(p_name).get_type() == Variant::NIL || obj->get(p_name).operator RefPtr().is_null()) { p_item->set_text(1, ""); p_item->set_icon(1, Ref()); p_item->set_custom_as_button(1, false); @@ -3610,7 +3619,16 @@ void PropertyEditor::update_tree() { RES res = obj->get(p.name).operator RefPtr(); - if (obj->get(p.name).get_type() == Variant::NIL || res.is_null()) { + Ref encoded = obj->get(p.name); //for debugger and remote tools + + if (encoded.is_valid()) { + + item->set_text(1, "Object: " + itos(encoded->get_object_id())); + item->set_icon(1, Ref()); + item->set_custom_as_button(1, true); + item->set_editable(1, true); + + } else if (obj->get(p.name).get_type() == Variant::NIL || res.is_null()) { item->set_text(1, ""); item->set_icon(1, Ref()); item->set_custom_as_button(1, false); @@ -3939,6 +3957,13 @@ void PropertyEditor::_item_edited() { if (!item->is_custom_set_as_button(1)) break; + Ref encoded = obj->get(name); //for debugger and remote tools + + if (encoded.is_valid()) { + + emit_signal("object_id_selected", encoded->get_object_id()); + } + RES res = obj->get(name); if (res.is_valid()) { emit_signal("resource_selected", res.get_ref_ptr(), name); diff --git a/editor/script_editor_debugger.cpp b/editor/script_editor_debugger.cpp index 01cfdc1b57c..bcf24b98f6a 100644 --- a/editor/script_editor_debugger.cpp +++ b/editor/script_editor_debugger.cpp @@ -372,7 +372,13 @@ void ScriptEditorDebugger::_parse_message(const String &p_msg, const Array &p_da if (has_icon(p_data[i + 2], "EditorIcons")) it->set_icon(0, get_icon(p_data[i + 2], "EditorIcons")); it->set_metadata(0, id); + if (id == inspected_object_id) { + TreeItem *cti = it->get_parent(); //ensure selected is always uncollapsed + while (cti) { + cti->set_collapsed(false); + cti = cti->get_parent(); + } it->select(0); } @@ -385,6 +391,7 @@ void ScriptEditorDebugger::_parse_message(const String &p_msg, const Array &p_da it->set_collapsed(true); } } + lv[level] = it; } updating_scene_tree = false; @@ -439,11 +446,8 @@ void ScriptEditorDebugger::_parse_message(const String &p_msg, const Array &p_da inspected_object->last_edited_id = id; - if (tabs->get_current_tab() == 2) { - inspect_properties->edit(inspected_object); - } else { - editor->push_item(inspected_object); - } + tabs->set_current_tab(inspect_info->get_index()); + inspect_properties->edit(inspected_object); } else if (p_msg == "message:video_mem") {