From 28fff9fd44b7364e3a23dacaf3d2a0a00bd0a40f Mon Sep 17 00:00:00 2001
From: Max Horn <max@quendi.de>
Date: Wed, 3 Jan 2024 22:21:01 +0100
Subject: [PATCH] Tighten checks in Objectify to only accept plain inputs

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.
---
 doc/ref/create.xml |  4 ++--
 lib/type1.g        |  4 ++--
 src/objects.c      | 24 +++++++++---------------
 3 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/doc/ref/create.xml b/doc/ref/create.xml
index 33dd9d62e6..62e44a455f 100644
--- a/doc/ref/create.xml
+++ b/doc/ref/create.xml
@@ -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
diff --git a/lib/type1.g b/lib/type1.g
index 329e23c61c..9ea3efdb77 100644
--- a/lib/type1.g
+++ b/lib/type1.g
@@ -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/>
diff --git a/src/objects.c b/src/objects.c
index 70ecd1a329..911ac30685 100644
--- a/src/objects.c
+++ b/src/objects.c
@@ -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);
         }
-        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;
     }
 }
@@ -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);
         }