Re: pltcl crashes due to a syntax error - Mailing list pgsql-hackers

From Erik Wienhold
Subject Re: pltcl crashes due to a syntax error
Date
Msg-id 5af13487-7722-4bdd-bad3-939598c8dd4e@ewie.name
Whole thread Raw
In response to Re: pltcl crashes due to a syntax error  (Pierre Forstmann <pierre.forstmann@gmail.com>)
Responses Re: pltcl crashes due to a syntax error
List pgsql-hackers
On 2024-06-02 14:32 +0200, Pierre Forstmann wrote:
> I understand that Tcl_GetVar should not be used any more and should be
> replaced by Tcl_GetStringResult
> (but I know nothing about Tcl internals)
> 
> Following patch :
> diff postgres/src/pl/tcl/pltcl.c.orig postgres/src/pl/tcl/pltcl.c
> 1373c1373,1376
> <       econtext = utf_u2e(Tcl_GetVar(interp, "errorInfo",
> TCL_GLOBAL_ONLY));
> ---
> >       /*
> >        * econtext = utf_u2e(Tcl_GetVar(interp, "errorInfo",
> TCL_GLOBAL_ONLY));
> >        */
> >       econtext = utf_u2e(Tcl_GetStringResult(interp));
> 
> gives:
> 
> pierre=# CREATE OR REPLACE PROCEDURE test_proc(INOUT a text)
>  AS $$
>  set aa [concat $1 "+" $1]
>  return [list $aa $aa])
>  $$
>  LANGUAGE pltcl;
> CREATE PROCEDURE
> pierre=# CALL test_proc('abc');
> 2024-06-02 14:22:45.223 CEST [61649] ERROR:  list element in braces
> followed by ")" instead of space
> 2024-06-02 14:22:45.223 CEST [61649] CONTEXT:  list element in braces
> followed by ")" instead of space
> in PL/Tcl function "test_proc"
> 2024-06-02 14:22:45.223 CEST [61649] STATEMENT:  CALL test_proc('abc');
> ERROR:  list element in braces followed by ")" instead of space
> CONTEXT:  list element in braces followed by ")" instead of space
> in PL/Tcl function "test_proc"

Tcl_GetStringResult is already used for emsg.  Setting econtext to same
string is rather pointless.  The problem is that Tcl_ListObjGetElements
does not set errorInfo if conversion fails.  From the manpage:

"If listPtr is not already a list value, Tcl_ListObjGetElements will
 attempt to convert it to one; if the conversion fails, it returns
 TCL_ERROR and leaves an error message in the interpreter's result value
 if interp is not NULL."

Tcl_GetVar returns null if errorInfo does not exist.  Omitting econtext
from errcontext in that case looks like the proper fix to me.

Or just do away with throw_tcl_error and call ereport directly.  Compare
that to pltcl_trigger_handler where the same case is handled like this:

    /************************************************************
     * Otherwise, the return value should be a column name/value list
     * specifying the modified tuple to return.
     ************************************************************/
    if (Tcl_ListObjGetElements(interp, Tcl_GetObjResult(interp),
                               &result_Objc, &result_Objv) != TCL_OK)
        ereport(ERROR,
                (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
                 errmsg("could not split return value from trigger: %s",
                        utf_u2e(Tcl_GetStringResult(interp)))));

-- 
Erik



pgsql-hackers by date:

Previous
From: Alexander Lakhin
Date:
Subject: Re: The xversion-upgrade test fails to stop server
Next
From: Ranier Vilela
Date:
Subject: Re: Fix possible dereference null pointer (PQprint)