From 9ab7f7d140d779ef23323d95f3a01569d997a889 Mon Sep 17 00:00:00 2001 From: melex750 Date: Sat, 17 Sep 2016 08:00:34 -0400 Subject: [PATCH] Fix access to protected and private variables --- src/CBot/CBotClass.cpp | 1 + src/CBot/CBotInstr/CBotExprRetVar.cpp | 4 +- src/CBot/CBotInstr/CBotExprVar.cpp | 7 +- src/CBot/CBotInstr/CBotFieldExpr.cpp | 53 +++++++++ src/CBot/CBotInstr/CBotFieldExpr.h | 11 ++ src/CBot/CBotInstr/CBotLeftExpr.cpp | 7 +- src/CBot/CBotProgram.cpp | 2 - src/CBot/CBotProgram.h | 7 -- test/unit/CBot/CBot_test.cpp | 149 ++++++++++++++++++++++++++ 9 files changed, 222 insertions(+), 19 deletions(-) diff --git a/src/CBot/CBotClass.cpp b/src/CBot/CBotClass.cpp index b342ab42..460ab9fa 100644 --- a/src/CBot/CBotClass.cpp +++ b/src/CBot/CBotClass.cpp @@ -682,6 +682,7 @@ bool CBotClass::CompileDefItem(CBotToken* &p, CBotCStack* pStack, bool bSecond) initType = CBotVar::InitType::DEF; pcopy->SetInit(initType); pcopy->SetUniqNum(pv->GetUniqNum()); + pcopy->SetPrivate(pv->GetPrivate()); pile->AddVar(pcopy); pv = pv->GetNext(); } diff --git a/src/CBot/CBotInstr/CBotExprRetVar.cpp b/src/CBot/CBotInstr/CBotExprRetVar.cpp index 96ff03c0..c0e0e18f 100644 --- a/src/CBot/CBotInstr/CBotExprRetVar.cpp +++ b/src/CBot/CBotInstr/CBotExprRetVar.cpp @@ -105,12 +105,12 @@ CBotInstr* CBotExprRetVar::Compile(CBotToken*& p, CBotCStack* pStack, bool bMeth CBotFieldExpr* i = new CBotFieldExpr(); i->SetToken(pp); inst->AddNext3(i); + CBotVar* preVar = var; var = var->GetItem(p->GetString()); if (var != nullptr) { i->SetUniqNum(var->GetUniqNum()); - if ( var->IsPrivate() && - !pStk->GetProgram()->m_bCompileClass) + if (CBotFieldExpr::ProtectionError(pStk, preVar, var)) { pStk->SetError(CBotErrPrivate, pp); goto err; diff --git a/src/CBot/CBotInstr/CBotExprVar.cpp b/src/CBot/CBotInstr/CBotExprVar.cpp index 3b774caf..1963d30a 100644 --- a/src/CBot/CBotInstr/CBotExprVar.cpp +++ b/src/CBot/CBotInstr/CBotExprVar.cpp @@ -67,8 +67,7 @@ CBotInstr* CBotExprVar::Compile(CBotToken*& p, CBotCStack* pStack, CBotVar::Prot if (ident > 0 && ident < 9000) { - if ( var->IsPrivate(privat) && - !pStk->GetProgram()->m_bCompileClass) + if (CBotFieldExpr::ProtectionError(pStk, nullptr, var, privat)) { pStk->SetError(CBotErrPrivate, p); goto err; @@ -133,12 +132,12 @@ CBotInstr* CBotExprVar::Compile(CBotToken*& p, CBotCStack* pStack, CBotVar::Prot CBotFieldExpr* i = new CBotFieldExpr(); // new element i->SetToken(pp); // keeps the name of the token inst->AddNext3(i); // add after + CBotVar* preVar = var; var = var->GetItem(p->GetString()); // get item correspondent if (var != nullptr) { i->SetUniqNum(var->GetUniqNum()); - if ( var->IsPrivate() && - !pStk->GetProgram()->m_bCompileClass) + if (CBotFieldExpr::ProtectionError(pStk, preVar, var, privat)) { pStk->SetError(CBotErrPrivate, pp); goto err; diff --git a/src/CBot/CBotInstr/CBotFieldExpr.cpp b/src/CBot/CBotInstr/CBotFieldExpr.cpp index 56ba987c..1d12a657 100644 --- a/src/CBot/CBotInstr/CBotFieldExpr.cpp +++ b/src/CBot/CBotInstr/CBotFieldExpr.cpp @@ -134,4 +134,57 @@ std::string CBotFieldExpr::GetDebugData() return ss.str(); } +//////////////////////////////////////////////////////////////////////////////// +bool CBotFieldExpr::ProtectionError(CBotCStack* pStack, CBotVar* pPrev, CBotVar* pVar, + CBotVar::ProtectionLevel privat) +{ + CBotVar::ProtectionLevel varPriv = pVar->GetPrivate(); + + if (privat == CBotVar::ProtectionLevel::ReadOnly && varPriv == privat) + return true; + + if (varPriv == CBotVar::ProtectionLevel::Public) return false; + + std::string prevName = (pPrev == nullptr) ? "" : pPrev->GetName(); + + // implicit 'this.'var, this.var, or super.var + if (pPrev == nullptr || prevName == "this" || prevName == "super") // member of the current class + { + if (varPriv == CBotVar::ProtectionLevel::Private) // var is private ? + { + CBotToken token("this"); + CBotVar* pThis = pStack->FindVar(token); + CBotClass* pClass = pThis->GetClass(); // the current class + + CBotVar* pVarList = pClass->GetVar(); + + int ident = pVar->GetUniqNum(); + // check if var is inherited from a parent class + if (pVarList == nullptr || ident < pVarList->GetUniqNum()) + return true; + } + } + else // any other context + { + if (pVar->IsPrivate()) // var is protected or private ? + { + CBotToken token("this"); + CBotVar* pThis = pStack->FindVar(token); + + if (pThis == nullptr) return true; // inside a function ? + if (pThis->GetType() != CBotTypPointer) return true; + + CBotClass* pClass = pThis->GetClass(); // the current class + + if (!pClass->IsChildOf(pPrev->GetClass())) // var is member of some other class ? + return true; + + if (varPriv == CBotVar::ProtectionLevel::Private && // private member of a parent class + pClass != pPrev->GetClass()) return true; + } + } + + return false; +} + } // namespace CBot diff --git a/src/CBot/CBotInstr/CBotFieldExpr.h b/src/CBot/CBotInstr/CBotFieldExpr.h index db48839b..2ee2851a 100644 --- a/src/CBot/CBotInstr/CBotFieldExpr.h +++ b/src/CBot/CBotInstr/CBotFieldExpr.h @@ -65,6 +65,17 @@ public: */ void RestoreStateVar(CBotStack* &pj, bool bMain) override; + /*! + * \brief ProtectionError Test if access to a variable is not allowed. + * \param pStack + * \param pPrev + * \param pVar + * \param privat + * \return True if pVar is protected in the current context. + */ + static bool ProtectionError(CBotCStack* pStack, CBotVar* pPrev, CBotVar* pVar, + CBotVar::ProtectionLevel privat = CBotVar::ProtectionLevel::Protected); + protected: virtual const std::string GetDebugName() override { return "CBotFieldExpr"; } virtual std::string GetDebugData() override; diff --git a/src/CBot/CBotInstr/CBotLeftExpr.cpp b/src/CBot/CBotInstr/CBotLeftExpr.cpp index 3a9fe155..8201137e 100644 --- a/src/CBot/CBotInstr/CBotLeftExpr.cpp +++ b/src/CBot/CBotInstr/CBotLeftExpr.cpp @@ -64,8 +64,7 @@ CBotLeftExpr* CBotLeftExpr::Compile(CBotToken* &p, CBotCStack* pStack) inst->m_nIdent = var->GetUniqNum(); if (inst->m_nIdent > 0 && inst->m_nIdent < 9000) { - if ( var->IsPrivate(CBotVar::ProtectionLevel::ReadOnly) && - !pStk->GetProgram()->m_bCompileClass) + if (CBotFieldExpr::ProtectionError(pStk, nullptr, var, CBotVar::ProtectionLevel::ReadOnly)) { pStk->SetError(CBotErrPrivate, p); goto err; @@ -125,11 +124,11 @@ CBotLeftExpr* CBotLeftExpr::Compile(CBotToken* &p, CBotCStack* pStack) if (p->GetType() == TokenTypVar) // must be a name { + CBotVar* preVar = var; var = var->GetItem(p->GetString()); // get item correspondent if (var != nullptr) { - if ( var->IsPrivate(CBotVar::ProtectionLevel::ReadOnly) && - !pStk->GetProgram()->m_bCompileClass) + if (CBotFieldExpr::ProtectionError(pStk, preVar, var, CBotVar::ProtectionLevel::ReadOnly)) { pStk->SetError(CBotErrPrivate, pp); goto err; diff --git a/src/CBot/CBotProgram.cpp b/src/CBot/CBotProgram.cpp index c406a01a..81008297 100644 --- a/src/CBot/CBotProgram.cpp +++ b/src/CBot/CBotProgram.cpp @@ -124,12 +124,10 @@ bool CBotProgram::Compile(const std::string& program, std::vector& if ( p->GetType() == ID_CLASS || ( p->GetType() == ID_PUBLIC && p->GetNext()->GetType() == ID_CLASS )) { - m_bCompileClass = true; CBotClass::Compile(p, pStack.get()); // completes the definition of the class } else { - m_bCompileClass = false; CBotFunction::Compile(p, pStack.get(), next); if (next->IsExtern()) functions.push_back(next->GetName()/* + next->GetParams()*/); if (next->IsPublic()) CBotFunction::AddPublic(next); diff --git a/src/CBot/CBotProgram.h b/src/CBot/CBotProgram.h index 9f5cdc00..165defc1 100644 --- a/src/CBot/CBotProgram.h +++ b/src/CBot/CBotProgram.h @@ -332,13 +332,6 @@ public: */ CBotFunction* GetFunctions(); - /** - * \brief true while compiling class - * - * TODO: refactor this - */ - bool m_bCompileClass; - /** * \brief Returns static list of all registered external calls */ diff --git a/test/unit/CBot/CBot_test.cpp b/test/unit/CBot/CBot_test.cpp index fb63ed91..797a674e 100644 --- a/test/unit/CBot/CBot_test.cpp +++ b/test/unit/CBot/CBot_test.cpp @@ -1904,3 +1904,152 @@ TEST_F(CBotUT, ClassMethodWithPublicKeyword) CBotErrUndefCall ); } + +TEST_F(CBotUT, ClassTestProtectedMember) +{ + auto publicProgram = ExecuteTest( + "public class BaseClass {\n" + " protected int a_protected = 1;\n" + " bool test() {\n" + " a_protected = 1;\n" + " int a = a_protected;\n" + " return true;\n" + " }\n" + "}\n" + "extern void Test() {\n" + " BaseClass b();\n" + " ASSERT(true == b.test());\n" + "}\n" + ); + + ExecuteTest( + "public class SubClass extends BaseClass {\n" + " bool testProtected() {\n" + " a_protected = 1;\n" + " int a = a_protected;\n" + " return true;\n" + " }\n" + "}\n" + "extern void TestSubClassAccessProtected() {\n" + " SubClass s();\n" + " ASSERT(true == s.test());\n" + " ASSERT(true == s.testProtected());\n" + "}\n" + ); + + ExecuteTest( + "extern void TestErrorProtected() {\n" + " BaseClass b();\n" + " int i = b.a_protected;\n" + "}\n", + CBotErrPrivate + ); + + ExecuteTest( + "extern void ErrorProtectedAssignment() {\n" + " BaseClass b();\n" + " b.a_protected = 1;\n" + "}\n", + CBotErrPrivate + ); + + ExecuteTest( + "public class SomeOtherClass {\n" + " void testErrorProtected() {\n" + " BaseClass b();\n" + " int i = b.a_protected;\n" + " }\n" + "}\n", + CBotErrPrivate + ); + + ExecuteTest( + "public class SomeOtherClass {\n" + " void testErrorProtectedAssignment() {\n" + " BaseClass b();\n" + " b.a_protected = 1;\n" + " }\n" + "}\n", + CBotErrPrivate + ); +} + +TEST_F(CBotUT, ClassTestPrivateMember) +{ + auto publicProgram = ExecuteTest( + "public class BaseClass {\n" + " private int a_private = 2;\n" + "\n" + " bool test() {\n" + " a_private = 2;\n" + " int a = a_private;\n" + " return true;\n" + " }\n" + " bool NoErrorPrivateSameClass() {\n" + " BaseClass b = new BaseClass();\n" + " int a = b.a_private;\n" + " b.a_private = 2;\n" + " return true;\n" + " }\n" + "}\n" + "extern void Test() {\n" + " BaseClass b();\n" + " ASSERT(true == b.test());\n" + " ASSERT(true == b.NoErrorPrivateSameClass());\n" + "}\n" + ); + + ExecuteTest( + "public class SubClass extends BaseClass {\n" + " void testErrorPrivate() {\n" + " int a = a_private;\n" + " }\n" + "}\n", + CBotErrPrivate + ); + + ExecuteTest( + "public class SubClass extends BaseClass {\n" + " void testErrorPrivateAssignment() {\n" + " a_private = 2;\n" + " }\n" + "}\n", + CBotErrPrivate + ); + + ExecuteTest( + "extern void TestErrorPrivate() {\n" + " BaseClass b();\n" + " int i = b.a_private;\n" + "}\n", + CBotErrPrivate + ); + + ExecuteTest( + "extern void ErrorPrivateAssignment() {\n" + " BaseClass b();\n" + " b.a_private = 2;\n" + "}\n", + CBotErrPrivate + ); + + ExecuteTest( + "public class SomeOtherClass {\n" + " void testErrorPrivate() {\n" + " BaseClass b();\n" + " int i = b.a_private;\n" + " }\n" + "}\n", + CBotErrPrivate + ); + + ExecuteTest( + "public class SomeOtherClass {\n" + " void testErrorPrivateAssignment() {\n" + " BaseClass b();\n" + " b.a_private = 1;\n" + " }\n" + "}\n", + CBotErrPrivate + ); +}