On Tue, Jun 08, 2021 at 04:37:46PM +0000, Jacob Champion wrote: > 1. Prep > > 0001 decouples the SASL code from the SCRAM implementation. > 0002 makes it possible to use common/jsonapi from the frontend. > 0003 lets the json_errdetail() result be freed, to avoid leaks. > > The first three patches are, hopefully, generally useful outside of > this implementation, and I'll plan to register them in the next > commitfest. The middle two patches are the "interesting" pieces, and > I've split them into client and server for ease of understanding, > though neither is particularly useful without the other.
Beginning with the beginning, could you spawn two threads for the jsonapi rework and the SASL/SCRAM business? I agree that these look independently useful. Glad to see someone improving the code with SASL and SCRAM which are too inter-dependent now. I saw in the RFCs dedicated to OAUTH the need for the JSON part as well. +# define check_stack_depth() +# ifdef JSONAPI_NO_LOG +# define json_log_and_abort(...) \ + do { fprintf(stderr, __VA_ARGS__); exit(1); } while(0) +# else In patch 0002, this is the wrong approach. libpq will not be able to feed on such reports, and you cannot use any of the APIs from the palloc() family either as these just fail on OOM. libpq should be able to know about the error, and would fill in the error back to the application. This abstraction is not necessary on HEAD as pg_verifybackup is fine with this level of reporting. My rough guess is that we will need to split the existing jsonapi.c into two files, one that can be used in shared libraries and a second that handles the errors. + /* TODO: SASL_EXCHANGE_FAILURE with output is forbidden in SASL */ if (result == SASL_EXCHANGE_SUCCESS) sendAuthRequest(port, AUTH_REQ_SASL_FIN, output, outputlen); Perhaps that's an issue we need to worry on its own? I didn't recall this part.. -- Michael
signature.asc
Description: PGP signature