Thread: function "cursor_to_xmlschema" causes a crash
Hello postgres hackers:
I recently notice that function "cursor_to_xmlschema" can lead to a crash if the
cursor parameter points to the query itself. Here is an example:
postgres=# SELECT cursor_to_xmlschema('' :: refcursor, TRUE , FALSE , 'xxx' ) into temp;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The reason could be that this function doesn't ensure the cursor is correctly
opened, as a "select into" statement can't be opened as a cursor. Although it may
be challenging to perform a perfect check in this scenario, it seems sufficient
just to check the tuple descriptor of the portal, since only the query that
returns tuples can be opened as a cursor.
Only in my opinion, self-pointing cursors like this do not make practical sense.
This bug is discovered through randomly generated SQL statements.
Best regards,
Boyu Yang
Attachment
"=?UTF-8?B?5p2o5Lyv5a6HKOmVv+Wggik=?=" <yangboyu.yby@alibaba-inc.com> writes: > I recently notice that function "cursor_to_xmlschema" can lead to a crash if the > cursor parameter points to the query itself. Here is an example: > postgres=# SELECT cursor_to_xmlschema('' :: refcursor, TRUE , FALSE , 'xxx' ) into temp; > server closed the connection unexpectedly Hmm, yeah, it's blindly assuming that whatever portal you point it at must have a result tupdesc, which in general PORTAL_MULTI_QUERY portals wouldn't. I always ask myself "where else did we make the same mistake?". Trawling through the callers of SPI_cursor_find and GetPortalByName, I couldn't find any other places that might get a hard failure. There are some where you might get weird error messages, eg regression=# select cursor_to_xml('',1,false,false,''); ERROR: portal "" cannot be run but maybe that's good enough. The thing that is disturbing is that SPI_cursor_find is a documented entry point, and the documentation doesn't warn that what you get back might not be a cursor-like object, so it seems like external callers might have this bug too. I thought about having SPI_cursor_find refuse to return things that aren't cursors, but I see that plpgsql's exec_stmt_open and exec_stmt_forc use SPI_cursor_find just to check for a duplicate cursor name. There, it seems like we don't want any additional filtering, because any portal will create a name conflict whether it's a cursor or not. While we could change those two callers to use GetPortalByName instead, any such change would make things strictly worse for outside callers that are doing the same thing. Also, PerformPortalFetch and PerformPortalClose have this very interesting kluge: /* * Disallow empty-string cursor name (conflicts with protocol-level * unnamed portal). */ if (!stmt->portalname || stmt->portalname[0] == '\0') ereport(ERROR, (errcode(ERRCODE_INVALID_CURSOR_NAME), errmsg("invalid cursor name: must not be empty"))); We could imagine making SPI_cursor_find do likewise, but again I'm not sure if that'd be good for all callers. In any case, the unnamed portal isn't the only source of this sort of problem. The argument for doing it would not be to protect careless callers like cursor_to_xmlschema, but to prevent SPI clients from having side-effects on the state of the unnamed portal. On balance I'm satisfied with just changing cursor_to_xmlschema as you suggest, though I'd probably phrase the error message as "portal %s is not a cursor". However, I feel like we'd better extend the documentation for SPI_cursor_find to point out that you might get something that isn't functionally like a cursor. regards, tom lane