Skip to content

Commit

Permalink
having no idea how to add access protection without interrupting othe…
Browse files Browse the repository at this point in the history 
…r modules...
  • Loading branch information
@Lazy-Rabbit-2001
Lazy-Rabbit-2001 committed Oct 22, 2024
1 parent 62a6627 commit e03fdc9
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 32 deletions.
1 change: 1 addition & 0 deletions core/object/object.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ void Object::set(const StringName &p_name, const Variant &p_value, bool *r_valid
#ifdef TOOLS_ENABLED

_edited = true;

#endif

if (script_instance) {
Expand Down
2 changes: 2 additions & 0 deletions core/object/object.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,8 @@ class Object {
bool _property_get_revert(const StringName &p_name, Variant &r_property) const { return false; };
void _notification(int p_notification) {}

bool execute_access_protection_runtime(const StringName &p_member_name, const StringName &p_derived_class, const Variant::AccessRestriction p_access_restriction, const StringName &p_member_owner_class, const Vector<StringName> &p_super_classes) const;

_FORCE_INLINE_ static void (*_get_bind_methods())() {
return &Object::_bind_methods;
}
Expand Down
1 change: 1 addition & 0 deletions core/variant/variant.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ class Variant {
_FORCE_INLINE_ Type get_type() const {
return type;
}

static String get_type_name(Variant::Type p_type);
static bool can_convert(Type p_type_from, Type p_type_to);
static bool can_convert_strict(Type p_type_from, Type p_type_to);
Expand Down
54 changes: 37 additions & 17 deletions modules/gdscript/gdscript.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1192,6 +1192,23 @@ bool GDScript::inherits_script(const Ref<Script> &p_script) const {
return false;
}

bool GDScript::inherits_class(const StringName &p_super_class) const {
if (p_super_class.is_empty()) {
return false;
}

const GDScript *s = this;

while (s) {
if (s->local_name == p_super_class || p_super_class == s->local_name) {
return true;
}
s = s->_base;
}

return false;
}

GDScript *GDScript::find_class(const String &p_qualified_name) {
String first = p_qualified_name.get_slice("::", 0);

Expand Down Expand Up @@ -1669,7 +1686,7 @@ bool GDScriptInstance::set(const StringName &p_name, const Variant &p_value) {
const GDScript::MemberInfo *member = &E->value;
Variant value = p_value;
if (member->data_type.has_type && !member->data_type.is_type(value)) {
if (!execute_access_restriction(p_name, script, script->member_access_restrictions[p_name])) {
if (!execute_access_restriction_runtime(p_name, script, script->member_access_restrictions[p_name])) {
return false;
}
const Variant *args = &p_value;
Expand All @@ -1680,14 +1697,14 @@ bool GDScriptInstance::set(const StringName &p_name, const Variant &p_value) {
}
}
if (likely(script->valid) && member->setter) {
if (!execute_access_restriction(member->setter, script, script->member_access_restrictions[p_name])) {
if (!execute_access_restriction_runtime(member->setter, script, script->member_access_restrictions[p_name])) {
return false;
}
const Variant *args = &value;
Callable::CallError err;
callp(member->setter, &args, 1, err);
return err.error == Callable::CallError::CALL_OK;
} else if (!execute_access_restriction(member->setter, script, script->member_access_restrictions[p_name])) {
} else if (!execute_access_restriction_runtime(member->setter, script, script->member_access_restrictions[p_name])) {
return false;
} else {
members.write[member->index] = value;
Expand All @@ -1704,7 +1721,7 @@ bool GDScriptInstance::set(const StringName &p_name, const Variant &p_value) {
const GDScript::MemberInfo *member = &E->value;
Variant value = p_value;
if (member->data_type.has_type && !member->data_type.is_type(value)) {
if (!execute_access_restriction(p_name, script, sptr->member_access_restrictions[p_name])) {
if (!execute_access_restriction_runtime(p_name, script, sptr->member_access_restrictions[p_name])) {
return false;
}
const Variant *args = &p_value;
Expand All @@ -1715,18 +1732,18 @@ bool GDScriptInstance::set(const StringName &p_name, const Variant &p_value) {
}
}
if (likely(sptr->valid) && member->setter) {
if (!execute_access_restriction(member->setter, script, sptr->member_access_restrictions[p_name])) {
if (!execute_access_restriction_runtime(member->setter, script, sptr->member_access_restrictions[p_name])) {
return false;
}
const Variant *args = &value;
Callable::CallError err;
callp(member->setter, &args, 1, err);
return err.error == Callable::CallError::CALL_OK;
} else if (!execute_access_restriction(member->setter, script, script->member_access_restrictions[p_name])) {
} else if (!execute_access_restriction_runtime(member->setter, script, script->member_access_restrictions[p_name])) {
return false;
} else {
sptr->static_variables.write[member->index] = value;
return execute_access_restriction(member->setter, script, script->member_access_restrictions[p_name]);
return execute_access_restriction_runtime(member->setter, script, script->member_access_restrictions[p_name]);
}
}
}
Expand Down Expand Up @@ -1759,10 +1776,10 @@ bool GDScriptInstance::get(const StringName &p_name, Variant &r_ret) const {
Callable::CallError err;
const Variant ret = const_cast<GDScriptInstance *>(this)->callp(E->value.getter, nullptr, 0, err);
r_ret = (err.error == Callable::CallError::CALL_OK) ? ret : Variant();
return execute_access_restriction(E->value.getter, script, script->member_access_restrictions[p_name]);
return execute_access_restriction_runtime(E->value.getter, script, script->member_access_restrictions[p_name]);
}
r_ret = members[E->value.index];
return execute_access_restriction(p_name, script, script->member_access_restrictions[p_name]);
return execute_access_restriction_runtime(p_name, script, script->member_access_restrictions[p_name]);
}
}

Expand All @@ -1772,7 +1789,7 @@ bool GDScriptInstance::get(const StringName &p_name, Variant &r_ret) const {
HashMap<StringName, Variant>::ConstIterator E = sptr->constants.find(p_name);
if (E) {
r_ret = E->value;
return execute_access_restriction(p_name, script, sptr->script_static_access_restrictions[p_name]);
return execute_access_restriction_runtime(p_name, script, sptr->script_static_access_restrictions[p_name]);
}
}

Expand All @@ -1783,18 +1800,18 @@ bool GDScriptInstance::get(const StringName &p_name, Variant &r_ret) const {
Callable::CallError ce;
const Variant ret = const_cast<GDScript *>(sptr)->callp(E->value.getter, nullptr, 0, ce);
r_ret = (ce.error == Callable::CallError::CALL_OK) ? ret : Variant();
return execute_access_restriction(E->value.getter, script, sptr->script_static_access_restrictions[p_name]);
return execute_access_restriction_runtime(E->value.getter, script, sptr->script_static_access_restrictions[p_name]);
}
r_ret = sptr->static_variables[E->value.index];
return execute_access_restriction(p_name, script, sptr->script_static_access_restrictions[p_name]);
return execute_access_restriction_runtime(p_name, script, sptr->script_static_access_restrictions[p_name]);
}
}

{
HashMap<StringName, MethodInfo>::ConstIterator E = sptr->_signals.find(p_name);
if (E) {
r_ret = Signal(owner, E->key);
return execute_access_restriction(p_name, script, sptr->script_static_access_restrictions[p_name]);
return execute_access_restriction_runtime(p_name, script, sptr->script_static_access_restrictions[p_name]);
}
}

Expand All @@ -1806,7 +1823,7 @@ bool GDScriptInstance::get(const StringName &p_name, Variant &r_ret) const {
} else {
r_ret = Callable(owner, E->key);
}
return execute_access_restriction(p_name, script, sptr->script_static_access_restrictions[p_name]);
return execute_access_restriction_runtime(p_name, script, sptr->script_static_access_restrictions[p_name]);
}
}

Expand Down Expand Up @@ -2051,20 +2068,23 @@ void GDScriptInstance::_call_implicit_ready_recursively(GDScript *p_script) {
}
}

bool GDScriptInstance::execute_access_restriction(const StringName &p_member_name, const Ref<GDScript> &p_current_script, const GDScript::MemberAccessRestriction &p_member_access_restriction) const {
bool GDScriptInstance::execute_access_restriction_runtime(const StringName &p_member_name, const Ref<GDScript> &p_current_script, const GDScript::MemberAccessRestriction &p_member_access_restriction) {
if (p_member_access_restriction.access_restriction == GDScriptParser::Node::ACCESS_RESTRICTION_PUBLIC) {
return true;
}

ERR_FAIL_NULL_V_EDMSG(p_current_script, false, R"(Trying to execute access protection on a null script...)");

// Don't use ClassDB for checking if a class inherits a super class
// As this method doesn not work in this case.
// Use script->inherits_class() instead because this can access the target class successfully
if (p_current_script->local_name == p_member_access_restriction.access_member_owner || p_member_access_restriction.access_member_owner == p_current_script->local_name) {
return true;
} else if (p_member_access_restriction.access_restriction == GDScriptParser::Node::ACCESS_RESTRICTION_PRIVATE) {
String err = vformat("Invalid access to %s (access level: private, owner: %s)", p_member_name, p_member_access_restriction.access_member_owner);
print_error(err);
ERR_FAIL_V_MSG(false, err);
} else if (p_member_access_restriction.access_restriction == GDScriptParser::Node::ACCESS_RESTRICTION_PROTECTED && !ClassDB::is_parent_class(p_current_script->local_name, p_member_access_restriction.access_member_owner)) {
} else if (p_member_access_restriction.access_restriction == GDScriptParser::Node::ACCESS_RESTRICTION_PROTECTED && !p_current_script->inherits_class(p_member_access_restriction.access_member_owner)) {
String err = vformat("Invalid access to %s (access level: protected, owner: %s)", p_member_name, p_member_access_restriction.access_member_owner);
print_error(err);
ERR_FAIL_V_MSG(false, err);
Expand All @@ -2083,7 +2103,7 @@ Variant GDScriptInstance::callp(const StringName &p_method, const Variant **p_ar
if (likely(sptr->valid)) {
HashMap<StringName, GDScriptFunction *>::Iterator E = sptr->member_functions.find(p_method);
if (E) {
if (!execute_access_restriction(p_method, script, sptr->script_static_access_restrictions[p_method])) {
if (!execute_access_restriction_runtime(p_method, script, sptr->script_static_access_restrictions[p_method])) {
return Variant();
}
return E->value->call(this, p_args, p_argcount, r_error);
Expand Down
6 changes: 3 additions & 3 deletions modules/gdscript/gdscript.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ class GDScript : public Script {
virtual bool is_abstract() const override { return false; } // GDScript does not support abstract classes.

bool inherits_script(const Ref<Script> &p_script) const override;
bool inherits_class(const StringName &p_super_class) const;

GDScript *find_class(const String &p_qualified_name);
bool has_class(const GDScript *p_script);
Expand Down Expand Up @@ -393,9 +394,6 @@ class GDScriptInstance : public ScriptInstance {

void _call_implicit_ready_recursively(GDScript *p_script);

private:
bool execute_access_restriction(const StringName &p_member_name, const Ref<GDScript> &p_current_script, const GDScript::MemberAccessRestriction &p_member_access_restriction) const;

public:
virtual Object *get_owner() { return owner; }

Expand Down Expand Up @@ -430,6 +428,8 @@ class GDScriptInstance : public ScriptInstance {

virtual const Variant get_rpc_config() const;

static bool execute_access_restriction_runtime(const StringName &p_member_name, const Ref<GDScript> &p_current_script, const GDScript::MemberAccessRestriction &p_member_access_restriction);

GDScriptInstance();
~GDScriptInstance();
};
Expand Down
21 changes: 11 additions & 10 deletions modules/gdscript/gdscript_analyzer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ Error GDScriptAnalyzer::check_class_member_name_conflict(const GDScriptParser::C
return OK;
}

bool GDScriptAnalyzer::execute_access_protection(const GDScriptParser::ClassNode *p_derived_class, const GDScriptParser::Node *p_protected_member, const GDScriptParser::Node *p_node, const bool p_is_call) {
bool GDScriptAnalyzer::execute_access_protection(const GDScriptParser::ClassNode *p_derived_class, const GDScriptParser::Node *p_protected_member, const Vector<StringName> &p_super_classes, const GDScriptParser::Node *p_node, const bool p_is_call) {
if (p_protected_member->access_restriction == GDScriptParser::Node::ACCESS_RESTRICTION_PUBLIC) {
return true;
}
Expand All @@ -307,12 +307,12 @@ bool GDScriptAnalyzer::execute_access_protection(const GDScriptParser::ClassNode
const String member_type = p_protected_member->type == GDScriptParser::Node::FUNCTION ? "method" : (p_protected_member->type == GDScriptParser::Node::Type::SIGNAL ? "signal" : "property");
const String action = p_is_call ? "call" : "access";

const bool is_from_non_derived = !ClassDB::is_parent_class(p_derived_class->identifier->name, p_protected_member->access_member_owner);
const bool is_from_non_derived = !p_super_classes.has(p_protected_member->access_member_owner);

const GDScriptParser::AssignableNode *member_assignable = static_cast<const GDScriptParser::AssignableNode *>(p_protected_member);
const StringName member_name = member_assignable ? member_assignable->identifier->name : "";

switch (execute_access_protection_global(p_derived_class->identifier->name, p_protected_member, p_node)) {
switch (execute_access_protection_global(p_derived_class->identifier->name, p_protected_member->access_member_owner, p_protected_member->access_restriction, p_super_classes, p_node)) {
case GDScriptAnalyzer::AccessRestrictionError::ACCESS_PRIVATE:
push_error(vformat(R"*(Could not %s %s "%s%s" in %s class, because it is private.)*", action, member_type, member_name, p_is_call ? "()" : "", is_from_non_derived ? "external" : "super"), p_node);
return false;
Expand All @@ -326,19 +326,21 @@ bool GDScriptAnalyzer::execute_access_protection(const GDScriptParser::ClassNode
return true;
}

GDScriptAnalyzer::AccessRestrictionError GDScriptAnalyzer::execute_access_protection_global(const StringName &p_derived_class, const GDScriptParser::Node *p_protected_member, const GDScriptParser::Node *p_node) {
if (p_protected_member->access_restriction == GDScriptParser::Node::ACCESS_RESTRICTION_PUBLIC) {
GDScriptAnalyzer::AccessRestrictionError GDScriptAnalyzer::execute_access_protection_global(const StringName &p_derived_class, const StringName &p_protected_member_owner, const GDScriptParser::Node::AccessRestriction p_access_restriction, const Vector<StringName> &p_super_classes, const GDScriptParser::Node *p_node) {
if (p_access_restriction == GDScriptParser::Node::ACCESS_RESTRICTION_PUBLIC) {
return GDScriptAnalyzer::AccessRestrictionError::ACCESS_OK;
}

ERR_FAIL_COND_V_MSG(!p_derived_class, GDScriptAnalyzer::AccessRestrictionError::ACCESS_OTHER_ERR, R"(Could not resolve the null derived class node...)");
ERR_FAIL_COND_V_MSG(!p_protected_member, GDScriptAnalyzer::AccessRestrictionError::ACCESS_OTHER_ERR, R"(Could not resolve the null member node...)");

if (p_derived_class == p_protected_member->access_member_owner || p_protected_member->access_member_owner == p_derived_class) {
// Don't use ClassDB for checking if a class inherits a super class
// As this method doesn not work in this case.
// Use script->inherits_class() instead because this can access the target class successfully
if (p_derived_class == p_protected_member_owner || p_protected_member_owner == p_derived_class) {
return GDScriptAnalyzer::AccessRestrictionError::ACCESS_OK;
} else if (p_protected_member->access_restriction == GDScriptParser::Node::ACCESS_RESTRICTION_PRIVATE) {
} else if (p_access_restriction == GDScriptParser::Node::ACCESS_RESTRICTION_PRIVATE) {
return GDScriptAnalyzer::AccessRestrictionError::ACCESS_PRIVATE;
} else if (p_protected_member->access_restriction == GDScriptParser::Node::ACCESS_RESTRICTION_PROTECTED && !ClassDB::is_parent_class(p_derived_class, p_protected_member->access_member_owner)) {
} else if (p_access_restriction == GDScriptParser::Node::ACCESS_RESTRICTION_PROTECTED && !p_super_classes.has(p_protected_member_owner)) {
return GDScriptAnalyzer::AccessRestrictionError::ACCESS_PROTECTED;
}

Expand Down Expand Up @@ -3594,7 +3596,6 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
}
current_super_class = current_super_class->base_type.class_type;
}
print_line(vformat(R"(Method: %s, owner: %s)", method->identifier->name, method->access_member_owner));
}
if (method) {
execute_access_protection(parser->current_class, method, parser->current_class->get_access_member_owner_extends_names(), p_call, true);
Expand Down
4 changes: 2 additions & 2 deletions modules/gdscript/gdscript_analyzer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class GDScriptAnalyzer {
Error check_native_member_name_conflict(const StringName &p_member_name, const GDScriptParser::Node *p_member_node, const StringName &p_native_type_string);
Error check_class_member_name_conflict(const GDScriptParser::ClassNode *p_class_node, const StringName &p_member_name, const GDScriptParser::Node *p_member_node);

bool execute_access_protection(const GDScriptParser::ClassNode *p_derived_class, const GDScriptParser::Node *p_protected_member, const GDScriptParser::Node *p_node, const bool p_is_call = false);
bool execute_access_protection(const GDScriptParser::ClassNode *p_derived_class, const GDScriptParser::Node *p_protected_member, const Vector<StringName> &p_super_classes, const GDScriptParser::Node *p_node, const bool p_is_call = false);

void get_class_node_current_scope_classes(GDScriptParser::ClassNode *p_node, List<GDScriptParser::ClassNode *> *p_list, GDScriptParser::Node *p_source);

Expand Down Expand Up @@ -176,7 +176,7 @@ class GDScriptAnalyzer {
Variant make_variable_default_value(GDScriptParser::VariableNode *p_variable);
static bool check_type_compatibility(const GDScriptParser::DataType &p_target, const GDScriptParser::DataType &p_source, bool p_allow_implicit_conversion = false, const GDScriptParser::Node *p_source_node = nullptr);

static AccessRestrictionError execute_access_protection_global(const StringName &p_derived_class, const GDScriptParser::Node *p_protected_member, const GDScriptParser::Node *p_node);
static AccessRestrictionError execute_access_protection_global(const StringName &p_derived_class, const StringName &p_protected_member_owner, const GDScriptParser::Node::AccessRestriction p_access_restriction, const Vector<StringName> &p_super_classes, const GDScriptParser::Node *p_node);

GDScriptAnalyzer(GDScriptParser *p_parser);
};
Expand Down

0 comments on commit e03fdc9

Please sign in to comment.