MOO-cows Mailing List Archive
[Prev][Next][Index][Thread]

RE: Production release 1.8.0 of the LambdaMOO server

>> If you must do something like this I would MUCH prefer the following:
>>
>> -- If a built-in function FOO has been made wiz-only (by defining
>>    $server_options.protect_FOO with a true value), and a call is made
>>    to that function from ANY VERB NOT DEFINED ON #0,
>>                          === ----------------------
>>    the server first checks to see if the verb #0:bf_FOO exists.
>>    If so, it calls it (REGARDLESS OF THE [CALLER PERMISSIONS])
>>    If not, E_PERM is raised if the calling verb is nonwizardly
>>      (i.e., same behavior as before if #0:bf_FOO doesn't exist).

> Under your proposal, if you want a built-in to be wizard-restricted (or 
> have one currently that is this way), you have to write #0:bf_FOO:
> 
> if (caller_perms().wizard)
>   FOO(@args);
> endif

You've misunderstood the proposal.  If there is no #0:bf_FOO verb, the
server should indeed behave as before and you should not be having to
write extra verbs to get this behavior back.

My proposal *only* affects the case where one has actually gone to the
trouble to install a #0:bf_FOO verb.  The only difference I want is that
this verb should be invoked even when the caller of foo() is a wizard. 
I expect that the overwhelming majority of #0:bf_FOO verbs will have one
of the following forms:

(1)
  set_task_perms(caller_perms);
  ...<do_bookkeeping>...
  try
     return FOO(@args);
  except e (ANY)
     ...<fix_bookkeeping>...
     raise (@e[1..3]);
  endtry

(2)
  if (...<new_argument/permissions_check>...)
     return FOO(@args)
  else
     raise (E_PERM);
  endif

Recall that (1) is the stated purpose of Pavel's change, namely to be
able to wrap every builtin call while still being able to make use of
the server's builtin checker.  The only difference with my proposal is
that a wizardly foo() call (i.e., from a non-#0 verb) will also cause
the bookkeeping to be done.  This is in fact what you want in the vast
majority of cases; it's error-prone for the bookkeeping not to happen by
default.  Figure that most verbs will be made wizardly for reasons
having absolutely nothing to do with the bookkeeping (the vast majority
of LambdaCore wizard verbs are so because they need to bash a +c
property or to do a player:notify()).

I'll grant that once in a blue moon, you'll need to invoke foo() without
the bookkeeping, but this is the exceptional case, and as such should be
a different verb --- in particular it should LOOK DIFFERENT when I see
it in verb code.  Note that this need can be satisfied by a SINGLE verb

  @program #0:raw
  return caller_perms().wizard ? call_function(@args) | raise (E_PERM);
  .

Then, I am guaranteed that when I see

  add_property(...)

in random verb code, it's ALWAYS doing the bookkeeping, while

  $raw("add_property",...)

is ALWAYS specifically circumventing it.

As for (2), so long as <new_argument/permissions_check> allows wizards
in, there is no difference under my proposal; wizardly would now call
#0:bf_FOO but that then passes straight on through to the raw foo()
call.  Moreover, by having <new_argument/permissions_check> not always
allow wizards in, there is now the possibility of enforcing invariants
on calls from wizard code as well as ordinary player code.

>> The problem with the previous version is if I'm looking at some random
>> verb in the db and I see a call `create(...)', given that #0:bf_create
>> could do completely arbitrary things, I have NO IDEA what this call is
>> going to do unless I know the wizardliness of the verb.  

> You have no idea what any code is going to do unless you know what
> permissions it's going to run under, including whether they are wiz perms
> (or programmer perms, and the individual player.) You never have and you
> never will.  
> The permissions of a verb determine what everything will do:
> what verb calls do, whether property assignments work, what built-ins will
> allow you to do, etc. 

My point is that up through 1.7.9, I could look at a verb and know
exactly what property references/modifications it will directly attempt
and what verb calls it will directly make.  My goal is quite modest
here:  *GIVEN* knowledge of what the called verbs do, I want to be able
to tell what the verb itself does or attempts to do; the fewer the
situations in which I have to refer to the permissions, the better.  In
practice, most verbs are well behaved and conform to simple
specifications.  If a particular call happens to be to a pathological
verb, well,... verbs and programmers can be fixed.  1.8.0 as it stands
now throws this monkey wrench into the works: foo() might call
#0:bf_FOO, then again it might not.

I'll agree that with pathological verbs in the mix, all bets are off;
cf. above remark about fixing programmers.  Permissions should only
govern success/failure; that's their intended purpose.  If you're trying
to use the permission slot as a way to pass an extra argument, well,
keep reading...

> Furthermore, what you suggest doesn't improve things.  If the problem is 
> that you don't know what #0:bf_FOO does....

No.  I might understand perfectly well what #0:bf_FOO does; I can always
go read it if I don't.  What I won't know is *WHETHER* it will be
called.  I may need to make a verb wizardly in order to get around a +c
property or to do a notify(player,...) or something else totally
independent of the call to foo().  Chowning the verb silently changes
the behavior of the foo() call, whether that's what I wanted or not. 
I'll grant that this happened to some extent before; that was bad, too,
but at least then it was a simple success-vs-failure question; now it's
a potential #0:bf_FOO call with all that might imply.

The question here is what the DEFAULT behavior of a builtin should be. 
If it's #0:bf_FOO for non-wizards, it should be the same way for
wizards; if wizard code wants to do something different, that should be
explicit.  With a #0:raw verb, it's even easy to do so.

>> An underlying principle here should be:
>> 
>>   A verb should have the same semantics regardless of its permissions.
>> 
>> By semantics, I mean the sequence of actions it attempts to perform on
>> the db, property-writes and verb calls in particular.  The only
>> difference that should be made by changing the ownership of a verb to be
>> wizardly/nonwizardly is that in the nonwizard case there is the
>> possibility of it bombing out somewhere in this sequence of actions with
>> E_PERM.  In particular, a builtin function should not engage in an
>> entirely different sequence of actions if the verb is/isn't wizardly.  I
>> realize that
>
> This has some merit.  It does make things more complicated if functions do 
> different things (more than just permission errors) based on who calls 
> them.  The flexibility of it, however, allows enough useful things that 
> it seems worth it to me.  

I have yet to see a good example of a function that needs to do
significantly different things based on caller_perms() that wouldn't
work just as well if not better by depending on an explicit argument
instead.  Any verb of the form

  {arg1,arg2,...} = args;
  if (...(caller_perms()))
     <sell_girl_scout_cookies>
  else
     <bomb_world_trade_center>
  endif

can be easily re-written as

  {who,arg1,arg2,...} = args;
  if ($perm_utils:controls(caller_perms(),who))
     raise (E_PERM);
  endif
  if (...(who))
     <sell_girl_scout_cookies>
  else
     <bomb_world_trade_center>
  endif

The latter is better style because the specification for what the verb
does is now entirely in terms of its arguments.  Every call now
indicates explicitly who the verb is supposed to operate on, thus making
any calling code more readable.  Moreover the second verb is more
flexible in that `who' now doesn't have to match task_perms; if I as a
wizard want to call this verb on someone else's behalf, I can now
explicitly specify the first argument myself --- I don't have to do
set_task_perms() and then flush the rest of my own verb somewhere else
in order to get the wizard permissions back...

> This is especially true because of who it makes life more
> complicated for: wizards.  Ordinary programmers have to deal with
> one case: what it does for non-wizzes.  They have to know what the
> built-in does, and that it may be protected or redirected to a
> wrapped version.  Only wizards need to keep track of different
> cases: what it does under their own permissions, what it does under
> set_task_perms(whoever).  Wizards are going to need to know what's
> going on in any case (they either need to know when the built-in is
> going to do what, or when to call another verb.)

If a given verb/builtin behaves by default as it does for ordinary
players, that's one less thing they have to keep track of.

> Supporting example: $server_options.protect_create is set true because
> programmers are calling raw create() instead of requesting an object from
> the recycler, resulting in skyrocketing max_object() and gobs of recycled
> objects just sitting around.  #0:bf_foo is written to call the recycler's
> create; problem solved. 
> 
> However, there needs to be some way to call the built-in create, because:
> (1) Sometimes a fresh object is really desired, for example when creating 
> new players, and
> (2) The recycler's create is going to need to call it if there are no 
> recycled objects available.

But wizard code will, in general, ALSO want to be getting objects from
the recycler.  It's only in a few specific cases that you want to be
doing a raw create(), and in those cases you want to be indicating your
intention explicitly in the code.

> I'll quote out of order here.  You propose:
> 
>> Note that if you want to have a raw version of a given callable from
>> arbitrary wizard code on other objects, my proposal still allows you to
>> define such a version on #0.  The difference is that it will now be
>> apparent from examination of said code which version of the builtin was
>> intended.  E.g., `$create(...)' is always the raw version while
>> `create(...)' is always #0:bf_create.  
> 
> I don't think that the clarity advantage is so great.  If I'm a wizard I 
> ought to be able to keep track of what create() does for me.  

Agreed.

> If we do it your way, in order to wizard-protect a built-in,
> I've got to

> (1) set $server_options.protect_FOO to 1

You can stop here if all you want is wizardonly-ness.

> (2) write #0:foo with permisson check that calls built-in foo()

Actually you just write #0:raw as above or 

  @program #0:"create recycle ..."
  return caller_perms().wizard 
         ? call_function(verb,@args)
         | raise (E_PERM);

which is something you do exactly once, perhaps later adding aliases to
the verb as you bring more builtins under your control.

> (3) change all wizard-owned calls to FOO with calls to $FOO.

Not all of them; in fact, not anywhere near all of them.

Keep in mind that the vast majority of those calls will be just
ordinary-player-ish calls, e.g., create() calls that simply want to
obtain a new object, no matter whether it's fresh or comes out of the
recycler --- exactly the same manner that an ordinary player would use
it.  In the case of create(), you'd be changing the recycler internal
create(), the new player create() and that's it.  In general, you take
care of the 2 or 3 calls that matter and the rest will all do the right
thing.  What's important is that if you later inadvertantly use create()
in a fresh verb, that will also do the right thing by default --- it
will take deliberate action to introduce a $raw("create"...) that
violates whatever invariants you're trying to maintain and then you'll
be thinking about what other special stuff you need to do so that
everything stays consistent.

Note by the way, that as 1.8.0 currently stands, if you want to maintain
any kind of invariant, you now likewise have to go through and change
wizard-owned create() calls to call $bf_create() explicitly, only this
time you're having to change all of the ones that AREN'T recycler
internals instead of just those few.  AND you moreover have to remember
to be invoking $bf_create for each new wizard verb rather than create().

Just to keep things in perspective, out of 1600 some-odd verbs in
LambdaCore only 3 call create().  add_property() occurs about 11 times
as does set_property_info().  I haven't actually counted how many of
these are Wizard vs. Hacker.

> I'd guess that the existing method is ever so slightly more efficient as 
> well.

$raw() uses 3 or 4 ticks.  big deal.  
Or to put it another way, "tick wise, pound foolish..."
Home | Subject Index | Thread Index