On Wed, Aug 25, 2021 at 11:42 AM Jacob Champion <pchamp...@vmware.com> wrote:
> On Tue, 2021-06-22 at 23:22 +0000, Jacob Champion wrote: > > On Fri, 2021-06-18 at 11:31 +0300, Heikki Linnakangas wrote: > > > > > > A few small things caught my eye in the backend oauth_exchange > function: > > > > > > > + /* Handle the client's initial message. */ > > > > + p = strdup(input); > > > > > > this strdup() should be pstrdup(). > > > > Thanks, I'll fix that in the next re-roll. > > > > > In the same function, there are a bunch of reports like this: > > > > > > > ereport(ERROR, > > > > + (errcode(ERRCODE_PROTOCOL_VIOLATION), > > > > + errmsg("malformed OAUTHBEARER message"), > > > > + errdetail("Comma expected, but found > character \"%s\".", > > > > + sanitize_char(*p)))); > > > > > > I don't think the double quotes are needed here, because sanitize_char > > > will return quotes if it's a single character. So it would end up > > > looking like this: ... found character "'x'". > > > > I'll fix this too. Thanks! > > v2, attached, incorporates Heikki's suggested fixes and also rebases on > top of latest HEAD, which had the SASL refactoring changes committed > last month. > > The biggest change from the last patchset is 0001, an attempt at > enabling jsonapi in the frontend without the use of palloc(), based on > suggestions by Michael and Tom from last commitfest. I've also made > some improvements to the pytest suite. No major changes to the OAuth > implementation yet. > > --Jacob > Hi, For v2-0001-common-jsonapi-support-FRONTEND-clients.patch : + /* Clean up. */ + termJsonLexContext(&lex); At the end of termJsonLexContext(), empty is copied to lex. For stack based JsonLexContext, the copy seems unnecessary. Maybe introduce a boolean parameter for termJsonLexContext() to signal that the copy can be omitted ? +#ifdef FRONTEND + /* make sure initialization succeeded */ + if (lex->strval == NULL) + return JSON_OUT_OF_MEMORY; Should PQExpBufferBroken(lex->strval) be used for the check ? Thanks