diff --git a/crates/db/src/db_worldstate.rs b/crates/db/src/db_worldstate.rs index 7cbceb25..fb7b5d1f 100644 --- a/crates/db/src/db_worldstate.rs +++ b/crates/db/src/db_worldstate.rs @@ -714,12 +714,18 @@ impl WorldState for DbTxWorldState { obj: &Obj, new_parent: &Obj, ) -> Result<(), WorldStateError> { - if obj == new_parent { - return Err(WorldStateError::RecursiveMove( - obj.clone(), - new_parent.clone(), - )); - } + { + let mut curr = new_parent.clone(); + while !curr.is_nothing() { + if &curr == obj { + return Err(WorldStateError::RecursiveMove( + obj.clone(), + new_parent.clone(), + )); + } + curr = self.parent_of(perms, &curr)?.clone(); + } + }; let (objflags, owner) = (self.flags_of(obj)?, self.owner_of(obj)?); diff --git a/crates/kernel/src/builtins/bf_objects.rs b/crates/kernel/src/builtins/bf_objects.rs index 7b3ab420..29789c55 100644 --- a/crates/kernel/src/builtins/bf_objects.rs +++ b/crates/kernel/src/builtins/bf_objects.rs @@ -62,7 +62,7 @@ fn bf_parent(bf_args: &mut BfCallState<'_>) -> Result { let Variant::Obj(obj) = bf_args.args[0].variant() else { return Err(BfErr::Code(E_TYPE)); }; - if !obj.is_positive() { + if !obj.is_positive() || !bf_args.world_state.valid(obj).map_err(world_state_bf_err)? { return Err(BfErr::Code(E_INVARG)); } let parent = bf_args @@ -83,6 +83,16 @@ fn bf_chparent(bf_args: &mut BfCallState<'_>) -> Result { let Variant::Obj(new_parent) = bf_args.args[1].variant() else { return Err(BfErr::Code(E_TYPE)); }; + // If object is not valid, or if new-parent is neither valid nor equal to #-1, then E_INVARG is raised. + if !bf_args.world_state.valid(obj).map_err(world_state_bf_err)? + || !(new_parent.is_nothing() + || bf_args + .world_state + .valid(new_parent) + .map_err(world_state_bf_err)?) + { + return Err(BfErr::Code(E_INVARG)); + } bf_args .world_state .change_parent(&bf_args.task_perms_who(), obj, new_parent) diff --git a/crates/kernel/testsuite/moot/objects/test_parent_chparent_errors.moot b/crates/kernel/testsuite/moot/objects/test_parent_chparent_errors.moot new file mode 100644 index 00000000..38d57288 --- /dev/null +++ b/crates/kernel/testsuite/moot/objects/test_parent_chparent_errors.moot @@ -0,0 +1,89 @@ +// Adapted from https://github.com/toddsundsted/stunt/blob/e83e946/test/test_objects.rb +// def test_parent_chparent_errors + +@wizard +; add_property($system, "a", create($nothing), {player, "wrc"}); +; add_property($system, "b", create($nothing), {player, "wrc"}); +; add_property($system, "c", create($nothing), {player, "wrc"}); +; $object = create($nothing); + +; chparent($a, $b); +; chparent($b, $c); + +; return parent(); +E_ARGS +; return parent(1, 2); +E_ARGS +; return parent(1); +E_TYPE +; return parent("1"); +E_TYPE +; return parent($nothing); +E_INVARG +; return parent($invalid_object); +E_INVARG + +; return chparent(); +E_ARGS +; return chparent(1); +E_ARGS +; return chparent(1, 2, 3); +E_ARGS +; return chparent(1, 2); +E_TYPE +; return chparent($object, 2); +E_TYPE +; return chparent($object, "2"); +E_TYPE +; return chparent($nothing, $object); +E_INVARG +; return chparent($object, $invalid_object); +E_INVARG +; return chparent($object, $object); +E_RECMOVE +; return chparent($c, $a); +E_RECMOVE + +// Test that if two objects define the same property by name, a +// new object cannot be created using both of them as parents. + +; add_property($system, "d", create($nothing), {player, "wrc"}); +; add_property($system, "e", create($nothing), {player, "wrc"}); + +; add_property($d, "foo", 123, {$d, ""}); +; add_property($e, "foo", "abc", {$e, ""}); + + +// A variety of tests that check permissions. + +@wizard +; $a = create($nothing); +; $b = create($a, $a); +; $b.f = 1; + +; return $a.owner; +$wizard_player +; return $b.owner; +$a + +@programmer +; $c = create($nothing); +; $d = create($b, $programmer_player); +; $d.f = 1; + +; return chparent($c, $a); +E_PERM +; return E_PERM == chparent($c, $b); +0 + +@wizard +; $e = create($nothing); + +; return E_PERM == chparent($e, $a); +0 +; return E_PERM == chparent($e, $b); +0 +; return E_PERM == chparent($e, $c); +0 +; return E_PERM == chparent($e, $d); +0 \ No newline at end of file diff --git a/crates/testing/moot/Test.db b/crates/testing/moot/Test.db index effd5ac8..dc2bc3a0 100644 --- a/crates/testing/moot/Test.db +++ b/crates/testing/moot/Test.db @@ -9,7 +9,7 @@ #0 System Object -16 +48 3 -1 -1 @@ -22,14 +22,20 @@ do_login_command 3 173 -1 -6 +12 nothing tmp tmp1 tmp2 system object -6 +invalid_object +ambiguous_match +failed_match +programmer_player +wizard_player +player +12 1 -1 3 @@ -54,6 +60,30 @@ object -1 3 7 +1 +2147483647 +3 +7 +1 +-2 +3 +7 +1 +-3 +3 +7 +1 +4 +3 +7 +1 +3 +3 +7 +1 +5 +3 +7 #1 Root Class @@ -130,21 +160,20 @@ nonprogrammer 0 #0:0 if (length(args) >= 2) - return eval("return " + args[2] + ";")[2]; +return eval(("return " + args[2]) + ";")[2]; else - return player; +return player; endif . #2:0 set_task_perms(player); try - notify(player, toliteral(eval(argstr + ";")[2])); -except err (ANY) - notify(player, toliteral(err[1])); +notify(player, toliteral(eval(argstr + ";")[2])); +except ERR (ANY) +notify(player, toliteral(ERR[1])); endtry . 0 clocks 0 queued tasks 0 suspended tasks -1 active connections with listeners -3 0 +0 active connections with listeners diff --git a/crates/testing/moot/Test.db.md b/crates/testing/moot/Test.db.md index 5cda9582..854477c6 100644 --- a/crates/testing/moot/Test.db.md +++ b/crates/testing/moot/Test.db.md @@ -7,11 +7,17 @@ Notable objects: - `#4`: programmer player - `#5`: non-programmer player -Globals: +Corified globals: - `$nothing`: `#-1` +- `$ambiguous_match`: `#-2` +- `$failed_match`: `#-3` - `$system`: `#0` +- `$wizard_player`: `#3` +- `$programmer_player`: `#4` +- `$player`: `#5` - `$object`, `$tmp`, `$tmp1`, `$tmp2`: for use as variables that persist across commands +- `$invalid_object`: `toobj(2142147483647)` Verbs: diff --git a/crates/testing/moot/tests/moot_lmoo.rs b/crates/testing/moot/tests/moot_lmoo.rs index 56990dac..4d4a0477 100644 --- a/crates/testing/moot/tests/moot_lmoo.rs +++ b/crates/testing/moot/tests/moot_lmoo.rs @@ -85,6 +85,6 @@ fn test_moo(path: &Path) { fn test_single() { test_moo( &PathBuf::from(env!("CARGO_MANIFEST_DIR")) - .join("../../kernel/testsuite/moot/objects/test_parent_chparent.moot"), + .join("../../kernel/testsuite/moot/objects/test_parent_chparent_errors.moot"), ); } diff --git a/tools/moot-translate.awk b/tools/moot-translate.awk index 750831a3..74cc3b84 100755 --- a/tools/moot-translate.awk +++ b/tools/moot-translate.awk @@ -6,6 +6,8 @@ # You should have received a copy of the GNU General Public License along with this program. If not, see . # +/ *end$/ { next } + { # Array syntax, comments, strings gsub(/\[/, "{"); @@ -15,10 +17,14 @@ # Standard corified references gsub("NOTHING", "$nothing"); + gsub(":nothing", "$nothing"); gsub("AMBIGUOUS_MATCH", "$ambiguous_match"); gsub("FAILED_MATCH", "$failed_match"); gsub("INVALID_OBJECT", "$invalid_object"); + # Other corified references + gsub(":object", "$object"); + # Assigment. Watch out: any variable names that are built-in MOO properties must be manually changed. s = gensub(/^(.*) = (.*)/, "; add_property($system, \"\\1\", \\2, {player, \"wrc\"});", "g", $0); @@ -40,6 +46,9 @@ # function calls without parens, because yay Ruby s = gensub(/^([a-z_]+) (.*)$/, "; \\1(\\2);", "g", s); + # run_test_as + s = gensub(/^run_test_as\(['"](.*)['"]\) do/, "@\\1", "g", s); + # TODO: somehow rewrite common variables like `a` into `$a`, but only when used as a variable? # this might be too hard to do without a full parser