On Mon, Mar 30, 2026 at 2:41 PM Jacob Champion <[email protected]> wrote: > v2, attached, rebases this over 993368113. The big change is the > removal of `custom-ca`; there were a couple of other tweaks to get > both commits compiling independently.
Now for review. 0001: I like how the UNSAFE: split works in practice. Implementation-wise, I think I'd prefer that the debug flags be implemented as bits in a uint32, and then we can cut down on the boilerplate. fe-auth-oauth-debug.c is IMO too much code for what the feature is providing. We should ideally be able to include this in a header from both locations it's used. I think `fast-retry` needs to be moved under UNSAFE and renamed to something that doesn't sound "good". `dos-interval` maybe? nitpick: `poll-counts` and `print-plugin-errors` choose different naming conventions, and we're not referring to the poll() API for the former. `call-count`? `dlopen`? We need to test that ignored unsafe options are in fact ignored (the parsed flag is currently being accumulated anyway, in contradiction to the warning message). I have a sample patch locally for these suggestions, if you'd like. -- I'm not a fan of 0002; I don't think it provides enough useful functionality to offset the cost. You said > validators authors should be able to verify that mismatched > configurations are rejected properly by the validator. but "mismatched configuration" is kind of meaningless here: the validator interacts with a token, not a client. Real-world validator implementations already need to test against all manner of broken tokens -- creating one that's signed by the wrong party shouldn't really be harder to create than one that's signed by the right party -- and the code to send a custom token from libpq is minimal (<40 lines). --Jacob
