Skip to content

Commit

Permalink
Tighten checks in Objectify to only accept plain inputs
Browse files Browse the repository at this point in the history
Also clarify the documentation -- at least for "standard" GAP; it now
misses to mention that atomic lists/records can also be used as input in
HPC-GAP, but that seems acceptable, given the state of HPC-GAP and also
given that we don't mention specifics for it most of the rest of the
regular GAP reference manual.

Background: In the distant path we allowed Objectify to be invoked on
any kind of GAP list or record, even though it was only intended for
plain lists and records. Passing in another kind of list or string could
in principle be used to "hack" any object into something different. But
that seems like a *bad* idea in general, and there is no clear use case;
however, there are cases where this can be done by accident.

Hence I tightened this check in early 2020; however, it turned out that
there was actually a call to Objectify on a plain string in HAP, so we
added a workaround for that. This patch removes that workaround.
  • Loading branch information
fingolfin committed Jan 6, 2024
1 parent bb0786f commit 28fff9f
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 19 deletions.
4 changes: 2 additions & 2 deletions doc/ref/create.xml
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ and why it is useful to have a declaration part and an implementation part.

<Description>
New objects are created by <Ref Func="Objectify"/>.
<A>data</A> is a list or a record, and <A>type</A> is the type that the
desired object shall have.
<A>data</A> must be a plain list or a plain record, and <A>type</A>
is the type that the desired object shall have.
<Ref Func="Objectify"/> turns <A>data</A> into an object with type
<A>type</A>.
That is, <A>data</A> is changed, and afterwards it will not be a list or a
Expand Down
4 changes: 2 additions & 2 deletions lib/type1.g
Original file line number Diff line number Diff line change
Expand Up @@ -727,8 +727,8 @@ end );
## will take a lot of time for type changes.
## You can avoid this by setting the attributes immediately while the
## object is created, as follows.
## <Ref Func="ObjectifyWithAttributes"/>
## changes the type of object <A>obj</A> to type <A>type</A>
## <Ref Func="ObjectifyWithAttributes"/> takes a plain list or record
## <A>obj</A> and turns it an object just like <Ref Func="Objectify"/>
## and sets attribute <A>attr1</A> to <A>val1</A>,
## sets attribute <A>attr2</A> to <A>val2</A> and so forth.
## <P/>
Expand Down
24 changes: 9 additions & 15 deletions src/objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -228,20 +228,17 @@ void SET_TYPE_OBJ(Obj obj, Obj type)
break;

default:
if (IS_STRING_REP(obj)) {
// FIXME/TODO: Hap calls Objectify on a string...
if (!IS_PLIST(obj)) {
ErrorMayQuit("cannot change type of a %s", (Int)TNAM_OBJ(obj), 0);

Check warning on line 232 in src/objects.c

View check run for this annotation

Codecov / codecov/patch

src/objects.c#L232

Added line #L232 was not covered by tests
}
else if (IS_PLIST(obj)) {
// TODO: we should also reject immutable plists, but that risks
// breaking existing code
#ifdef HPCGAP
MEMBAR_WRITE();
MEMBAR_WRITE();
#endif
RetypeBag(obj, T_POSOBJ);
SET_TYPE_POSOBJ(obj, type);
CHANGED_BAG(obj);
}
else {
ErrorMayQuit("cannot change type of a %s", (Int)TNAM_OBJ(obj), 0);
}
RetypeBag(obj, T_POSOBJ);
SET_TYPE_POSOBJ(obj, type);
CHANGED_BAG(obj);
break;
}
}
Expand Down Expand Up @@ -1338,10 +1335,7 @@ static Obj FuncSET_TYPE_POSOBJ(Obj self, Obj obj, Obj type)
case T_POSOBJ:
break;
default:
if (IS_STRING_REP(obj)) {
// FIXME/TODO: Hap calls Objectify on a string...
}
else if (!IS_PLIST(obj)) {
if (!IS_PLIST(obj)) {
ErrorMayQuit("You can't make a positional object from a %s",
(Int)TNAM_OBJ(obj), 0);
}
Expand Down

0 comments on commit 28fff9f

Please sign in to comment.