pgjdbc/pgjdbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feed[pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
42+ messages / 4 participants
[nested] [flat]
* [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 18:04 "davecramer (@davecramer)" <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2025-12-16 18:04 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
I still need to figure out how to run the tests.
And add documentation.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 18:04 ` "gitguardian[bot] (bot) (@gitguardian[bot])" <noreply+gitguardian[bot]@github.com>
40 siblings, 0 replies; 42+ messages in thread
From: gitguardian[bot] (bot) (@gitguardian[bot]) @ 2025-12-16 18:04 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
#### ⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.
Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.
Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.
<details>
<summary>🔎 Detected hardcoded secret in your pull request</summary>
<br>
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
| -------------- | ------------------ | ------------------------------ | ---------------- | --------------- | -------------------- |
| [-](https://dashboard.gitguardian.com/workspace/59849/incidents/secrets) | - | Generic Password | 23d62b377ef2c1da898d3d9f27ae537e7088ea9b | pgjdbc/src/main/java/org/postgresql/PGProperty.java | [View secret](https://github.com/pgjdbc/pgjdbc/commit/23d62b377ef2c1da898d3d9f27ae537e7088ea9b#diff-f1f17a9527eed8...) |
</details>
<details>
<summary>🛠 Guidelines to remediate hardcoded secrets</summary>
<br>
1. Understand the implications of revoking this secret by investigating where it is used in your code.
2. Replace and store your secret safely. [Learn here](https://blog.gitguardian.com/secrets-api-management?utm_source=product&utm_medium=GitHub_che...) the best practices.
3. Revoke and [rotate this secret](https://docs.gitguardian.com/secrets-detection/secrets-detection-engine/detectors/generics/generic_p...).
4. If possible, [rewrite git history](https://blog.gitguardian.com/rewriting-git-history-cheatsheet?utm_source=product&utm_medium=...). Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these [best practices](https://blog.gitguardian.com/secrets-api-management/?utm_source=product&utm_medium=GitHub_ch...) for managing and storing secrets including API keys and other credentials
- install [secret detection on pre-commit](https://docs.gitguardian.com/ggshield-docs/integrations/git-hooks/pre-commit?utm_source=product&...) to catch secret before it leaves your machine and ease remediation.
</details>
---
<sup>🦉 [GitGuardian](https://dashboard.gitguardian.com/auth/login/?utm_medium=checkruns&utm_source=github&...) detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.<br/></sup>
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 18:21 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2025-12-16 18:21 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
WDYT of `enum` and `EnumSet`?
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 18:26 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2025-12-16 18:26 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
libpq says "Negated and non-negated forms may not be combined in the same setting".
WDYT of replicating the same behavior?
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 19:04 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2025-12-16 19:04 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
sure, that makes more sense
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 19:04 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2025-12-16 19:04 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
seems appropriate, will change
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 19:32 ` "sehrope (@sehrope)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: sehrope (@sehrope) @ 2025-12-16 19:32 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
Why are we doing it with bit masks rather than throwing the allowed / disallowed in a `Set<string>(...)` and calling `.has(...)`?
We have to do the mapping from the FEBE code to a name once and then can use that for the check. It's not like this happening in some tight loop. It'd be a single check for the whole connection handshake right?
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 19:35 ` "sehrope (@sehrope)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: sehrope (@sehrope) @ 2025-12-16 19:35 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
Missed this before I wrote the other comment. Yes this whole thing would be easier to implement and understand with enums and sets. The parsing step becomes much easier too (just check if we've seen the value already for the dupe error).
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 20:17 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2025-12-16 20:17 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
changed in latest
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 20:17 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2025-12-16 20:17 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
changed in latest
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 20:18 ` "sehrope (@sehrope)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: sehrope (@sehrope) @ 2025-12-16 20:18 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
We're silently skipping unknown methods here. Is that intentional?
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 20:19 ` "sehrope (@sehrope)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: sehrope (@sehrope) @ 2025-12-16 20:19 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
(Because the `fromString(...)` is going to return null)
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-16 20:46 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2025-12-16 20:46 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
I changed fromString to throw an exception for unrecognized options
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-17 05:47 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2025-12-17 05:47 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
There's no need to track two sets just one set with "seen flags" is enough
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-17 05:47 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2025-12-17 05:47 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
It does not need to have its own loop
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-17 13:00 ` "sehrope (@sehrope)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: sehrope (@sehrope) @ 2025-12-17 13:00 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
Can put all of this logic in the enum itself and expose a single method to check the auth mode vs the connection's actual value. So it'd just be `public static void check(String requireAuth, String authMethod)`.
That'd make the logic itself easier to unit test vs all the possible server configs. Would still be good to have some server related coverage for it as well, but we could do something pretty exhaustive in the unit test.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-24 14:20 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2025-12-24 14:20 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
```suggestion
static void checkAuth(Set<AuthMethod> allowedMethods, AuthMethod authMethod) throws PSQLException {
```
`isAllowed(...)` results in repeated parsing, so why don't we parse the set once and pass it over the methods?
I'm not fond of `is...` methods that return void.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-24 14:23 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2025-12-24 14:23 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/core/RequireAuthTest.java)
```suggestion
```
Just rethrow the exception
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-24 14:24 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2025-12-24 14:24 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/core/RequireAuthTest.java)
```suggestion
```
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-24 14:26 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2025-12-24 14:26 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/core/RequireAuthTest.java)
```suggestion
TestUtil.setTestUrlProperty(props, PGProperty.PG_DBNAME, "authtest");
```
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-24 15:17 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2025-12-24 15:17 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
agreed on changing the name.
As for parsing once, we only ever call checkAuth once so it is only parsed once, and requireAuth could be null
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-24 15:42 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2025-12-24 15:42 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
`AUTH_REQ_OK` brings the second parse, doesn't it?
Frankly, I find it much eaiser to reason about if the inputs are parsed once, and the code deals with parsed values rather than parse at multiple locations.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2025-12-24 15:55 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2025-12-24 15:55 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
AFAIUI `AUTH_REQ_OK` will only require a parse if it is simple password. Otherwise `pgStream.isFinishedAuthenticationRequests()` will be true and we don't call checkAuth. That said if you find it easier to reason, that's a good enough reason for me to change it
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-23 12:49 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2026-02-23 12:49 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
@vlsi I believe this is ready to go? Everything has been resolved.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-23 13:02 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2026-02-23 13:02 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java:985)
GSS encryption != GSS authentication.
I think AuthMethod.GSS should be in `case AUTH_REQ_GSS`
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-23 13:11 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2026-02-23 13:11 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/AuthMethod.java)
I would rather have `@Nullable EnumSet<AuthMethod>` to distinguish "not set" vs "property was set to an empty value" cases.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-23 13:16 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2026-02-23 13:16 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java)
Whitespace looks odd
```suggestion
if (!pgStream.isFinishedAuthenticationRequests()) {
```
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-23 13:17 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2026-02-23 13:17 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/test/java/org/postgresql/test/core/RequireAuthTest.java)
Is there a reason for explicit `= true`?
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-23 13:21 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2026-02-23 13:21 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java:840)
Does it really mean `finished authentication requests`?
Can it be the case the backend issues SCRAM request after MD5?
If the only reason for this call is to track "have at least one auth request", we should consider renaming the property to something like "has auth requests", and we could set it like `if (authReq != AUTH_REQ_OK) { pgStream.hasAuthRequests(); }`
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-23 16:23 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2026-02-23 16:23 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java:985)
GSS encryption is strange the way it is implemented. IIRC it's a bit like TLS I don't believe there is ever a GSS auth request, just an encryption request.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-23 19:15 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2026-02-23 19:15 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java:985)
We have `case AUTH_REQ_GSS`. Does it mean something different from "gss authentication requested"?
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-23 19:28 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2026-02-23 19:28 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java:985)
Yeah, it's strange the way they implemented encrypted GSS . Starts at https://github.com/pgjdbc/pgjdbc/blob/5aa76f88df5c95f1a8a9f5235ba45acffd5bb50e/pgjdbc/src/main/java/... and the function is https://github.com/pgjdbc/pgjdbc/blob/5aa76f88df5c95f1a8a9f5235ba45acffd5bb50e/pgjdbc/src/main/java/... I think we have to attempt the encrypted GSS and if it succeeds then fail with an error.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-23 19:46 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2026-02-23 19:46 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
any idea why https://github.com/pgjdbc/pgjdbc/actions/runs/22321662457/job/64581275265?pr=3895 is suddenly failing with ```org.postgresql.util.PSQLException: ERROR: out of shared memory
Hinweis: You might need to increase max_locks_per_transaction.```
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-24 16:16 ` "sehrope (@sehrope)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: sehrope (@sehrope) @ 2026-02-24 16:16 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
I'm able to reproduce this error locally and tracked it down with git bisect:
```
3fab588d93ae4cfeb13f75ba08894b804b256d51 is the first bad commit
commit 3fab588d93ae4cfeb13f75ba08894b804b256d51
Author: Vladimir Sitnikov <[email protected]>
Date: Mon Jan 19 12:16:57 2026 +0300
test: add autosave=always|never|conservative and cleanupSavepoints=true|false to the randomized CI jobs
:040000 040000 f663e7608969c1d7de2dbcc4df3bf8003a51c112 a2ace7b120c0c1e16f7640077410ce511aec429d M .github
:040000 040000 93916651a74db1749db76dba81068287f9c87e0a c44dce9d078a5385473e9eccf9143057f0f38937 M build-logic
:040000 040000 d1201b2777e25f2e89f51c6729e97511fb352e02 fa625fdb5b472f40550997b24dd625da8d265b09 M pgjdbc
bisect run success
```
Here's the steps if you want to try this locally:
Save this script as `build/database-encoding-test` and `chmod +x`:
```
#!/usr/bin/env bash
main () {
./gradlew -Duser.country=de -D.user.language=DE -DreWriteBatchedInserts=true -Dautosave=always cleanTest postgresql:test --tests DatabaseEncodingTest
}
main "$@"
```
Start the 9.1 server with:
```
PGV=9.1 TZ=UTC XA=no SSL=no SCRAM=no CREATE_REPLICAS=no docker/bin/postgres-server
```
Then starting at the HEAD of this PR (or any other "bad") commit:
```
git bisect start
git bisect bad
git bisect good 79b784e3 # This is REL42.7.9
git bisect run build/database-encoding-test
```
That commit message in 3fab588d93ae4cfeb13f75ba08894b804b256d51 is deceiving as well. It's labeled `test: add autosave=... to the CI jobs` but it's actually making some changes to the `QueryExecutorImpl` as well. Still have to step through to understand why, but it's something in that commit that changed either when we're creating a save point or when we're cleaning them up.
That combined with the lower defaults for 9.1 is causing this to fail over there for some specific combo of parameters.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-24 16:27 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2026-02-24 16:27 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
> I'm able to reproduce this error locally and tracked it down with git bisect:
>
> ```
> 3fab588d93ae4cfeb13f75ba08894b804b256d51 is the first bad commit
> commit 3fab588d93ae4cfeb13f75ba08894b804b256d51
> Author: Vladimir Sitnikov <[email protected]>
> Date: Mon Jan 19 12:16:57 2026 +0300
>
> test: add autosave=always|never|conservative and cleanupSavepoints=true|false to the randomized CI jobs
>
> :040000 040000 f663e7608969c1d7de2dbcc4df3bf8003a51c112 a2ace7b120c0c1e16f7640077410ce511aec429d M .github
> :040000 040000 93916651a74db1749db76dba81068287f9c87e0a c44dce9d078a5385473e9eccf9143057f0f38937 M build-logic
> :040000 040000 d1201b2777e25f2e89f51c6729e97511fb352e02 fa625fdb5b472f40550997b24dd625da8d265b09 M pgjdbc
> bisect run success
> ```
>
> Here's the steps if you want to try this locally:
>
> Save this script as `build/database-encoding-test` and `chmod +x`:
>
> ```
> #!/usr/bin/env bash
>
> main () {
> ./gradlew -Duser.country=de -D.user.language=DE -DreWriteBatchedInserts=true -Dautosave=always cleanTest postgresql:test --tests DatabaseEncodingTest
> }
>
> main "$@"
> ```
>
> Start the 9.1 server with:
>
> ```
> PGV=9.1 TZ=UTC XA=no SSL=no SCRAM=no CREATE_REPLICAS=no docker/bin/postgres-server
> ```
>
> Then starting at the HEAD of this PR (or any other "bad") commit:
>
> ```
> git bisect start
> git bisect bad
> git bisect good 79b784e3 # This is REL42.7.9
> git bisect run build/database-encoding-test
> ```
>
> That commit message in [3fab588](https://github.com/pgjdbc/pgjdbc/commit/3fab588d93ae4cfeb13f75ba08894b804b256d51) is deceiving as well. It's labeled `test: add autosave=... to the CI jobs` but it's actually making some changes to the `QueryExecutorImpl` as well. Still have to step through to understand why, but it's something in that commit that changed either when we're creating a save point or when we're cleaning them up.
>
> That combined with the lower defaults for 9.1 is causing this to fail over there for some specific combo of parameters.
I expect this is just the extra options in the test combined with the lower defaults in 9.1
@vlsi ?
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-24 16:32 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2026-02-24 16:32 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
By the way, I asked Opus 4.6 to identify the reason for the failure (I gave it a stacktrace and a link to the CI log), and it ended up with virtually the same analysis (I haven't had time to test it manually, so I did not post the results):
> The failure occurs in only one CI configuration:
> Java 21, PG 9.1, autosave always, rewrite_batch_inserts, query_timeout 15
> The key parameter is **autosave=always**.
> **What's happening**
> 1. DatabaseEncodingTest.encoding() performs ~11,100 INSERTs in a single transaction (autoCommit=false):
> - 1..0xd800 step 100 → ~553 inserts
> - 0xe000..0x10000 → ~82 inserts
> - 0x10000..0x110000 → ~10,486 inserts
> 2. With autosave=always, pgjdbc wraps every statement in SAVEPOINT / RELEASE SAVEPOINT. That's ~11,100 nested subtransactions.
> 3. In PostgreSQL, each savepoint (subtransaction) consumes entries in the shared lock table. Even after RELEASE SAVEPOINT, lock entries are merged into the parent transaction, but shared memory slots are not freed until commit.
> 4. The lock table size = max_locks_per_transaction × max_connections = 64 × 100 = 6,400 slots (defaults). With 11,000+ subtransactions, the table overflows.
> 5. PG 9.1 is a very old version with less efficient subtransaction lock management. The same test passes on newer PG versions (9.3, 11, 14, 17).
> **How to investigate further**
> 1. Check if it reproduces without autosave=always — most likely it won't
> 2. Check history — this may be a flaky test unrelated to PR #3895. Look at whether this job also fails intermittently on master
> 3. Reproduce locally — run PG 9.1 with default max_locks_per_transaction=64 and autosave=always, then run the encoding test
> **Possible fixes**
> - Increase max_locks_per_transaction in the CI entrypoint.sh (e.g., to 256)
> - Add periodic con.commit() calls inside the test loop to release locks
> - Skip the test when autosave=always on old PG versions
> - Drop PG 9.1 from the test matrix — it has been EOL since 2016
> The simplest and most correct fix is either increasing max_locks_per_transaction in the CI PostgreSQL configuration, or removing PG 9.1 from the test matrix entirely.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-24 16:36 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2026-02-24 16:36 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Why 3fab588 is the first bad commit
The commit does two things:
1. Adds `autosave=always` to the CI test matrix
Before this commit, tests never ran with autosave=always.
The unlucky random combination assigned autosave=always to the PG 9.1 job.
2. Changes shouldCreateAutomaticSavepoint() to create more savepoints
The old code checked isTransactionActive() before checking AutoSave.NEVER, meaning it would bail out early in some cases. The new code reorders the checks and also expands isSpecialQuery() — but the net effect is that with autosave=always, every non-special statement now gets wrapped in SAVEPOINT / RELEASE SAVEPOINT.
The combined effect
DatabaseEncodingTest.encoding() does ~11,100 INSERTs in one transaction. With autosave=always, each INSERT creates a subtransaction. PG 9.1's default max_locks_per_transaction=64 gives only ~6,400 lock table slots — far fewer than the ~11,100 needed.
---
Frankly, I think the test did the right thing: it identified a bug.
Then we could have several workarounds:
1. Try if increase max_locks_per_transaction in the CI PostgreSQL 9.1 config helps (e.g., -c max_locks_per_transaction=256 in entrypoint.sh).
2. Exclude autosave=always from PG 9.1 in the matrix — add a rule like:
`matrix.imply({pg_version: '9.1'}, {autosave: {value: 'never'}});`
3. Add periodic commits in `DatabaseEncodingTest` to keep subtransaction count manageable
I am leaning towards 1 or 2.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-24 16:44 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2026-02-24 16:44 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
> Why [3fab588](https://github.com/pgjdbc/pgjdbc/commit/3fab588d93ae4cfeb13f75ba08894b804b256d51) is the first bad commit
>
> The commit does two things:
>
> 1. Adds `autosave=always` to the CI test matrix
> Before this commit, tests never ran with autosave=always.
> The unlucky random combination assigned autosave=always to the PG 9.1 job.
> 2. Changes shouldCreateAutomaticSavepoint() to create more savepoints
>
> The old code checked isTransactionActive() before checking AutoSave.NEVER, meaning it would bail out early in some cases. The new code reorders the checks and also expands isSpecialQuery() — but the net effect is that with autosave=always, every non-special statement now gets wrapped in SAVEPOINT / RELEASE SAVEPOINT.
>
> The combined effect
>
> DatabaseEncodingTest.encoding() does ~11,100 INSERTs in one transaction. With autosave=always, each INSERT creates a subtransaction. PG 9.1's default max_locks_per_transaction=64 gives only ~6,400 lock table slots — far fewer than the ~11,100 needed.
>
> Frankly, I think the test did the right thing: it identified a bug.
>
> Then we could have several workarounds:
>
> 1. Try if increase max_locks_per_transaction in the CI PostgreSQL 9.1 config helps (e.g., -c max_locks_per_transaction=256 in entrypoint.sh).
> 2. Exclude autosave=always from PG 9.1 in the matrix — add a rule like:
> `matrix.imply({pg_version: '9.1'}, {autosave: {value: 'never'}});`
> 3. Add periodic commits in `DatabaseEncodingTest` to keep subtransaction count manageable
>
> I am leaning towards 1 or 2.
easy enough to try 1 in the PR.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-25 11:51 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2026-02-25 11:51 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
OK, tests are passing, any objections to merging this ?
@vlsi ?
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-27 14:00 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2026-02-27 14:00 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java:840)
This mimics the server https://github.com/postgres/postgres/blob/16743db061e431d40522547c6436af6616026caa/src/interfaces/li...
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-27 14:14 ` "vlsi (@vlsi)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: vlsi (@vlsi) @ 2026-02-27 14:14 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java:985)
There's server-side comment: https://github.com/postgres/postgres/blob/16743db061e431d40522547c6436af6616026caa/src/interfaces/li...
"If implicit GSS auth has already been performed via GSS
* encryption, we don't need to have performed an
* AUTH_REQ_GSS exchange"
It implies GSS auth can indeed happen via GSS encryption.
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this.
@ 2026-02-27 14:43 ` "davecramer (@davecramer)" <[email protected]>
40 siblings, 0 replies; 42+ messages in thread
From: davecramer (@davecramer) @ 2026-02-27 14:43 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java:985)
Correct. Which is why you need to check after the stream has been encrypted.
^ permalink raw reply [nested|flat] 42+ messages in thread
end of thread, other threads:[~2026-02-27 14:43 UTC | newest]
Thread overview: 42+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-12-16 18:04 [pgjdbc/pgjdbc] PR #3895: implement require_auth, this is pretty much how libpq does this. "davecramer (@davecramer)" <[email protected]>
2025-12-16 18:04 ` "gitguardian[bot] (bot) (@gitguardian[bot])" <noreply+gitguardian[bot]@github.com>
2025-12-16 18:21 ` "vlsi (@vlsi)" <[email protected]>
2025-12-16 18:26 ` "vlsi (@vlsi)" <[email protected]>
2025-12-16 19:04 ` "davecramer (@davecramer)" <[email protected]>
2025-12-16 19:04 ` "davecramer (@davecramer)" <[email protected]>
2025-12-16 19:32 ` "sehrope (@sehrope)" <[email protected]>
2025-12-16 19:35 ` "sehrope (@sehrope)" <[email protected]>
2025-12-16 20:17 ` "davecramer (@davecramer)" <[email protected]>
2025-12-16 20:17 ` "davecramer (@davecramer)" <[email protected]>
2025-12-16 20:18 ` "sehrope (@sehrope)" <[email protected]>
2025-12-16 20:19 ` "sehrope (@sehrope)" <[email protected]>
2025-12-16 20:46 ` "davecramer (@davecramer)" <[email protected]>
2025-12-17 05:47 ` "vlsi (@vlsi)" <[email protected]>
2025-12-17 05:47 ` "vlsi (@vlsi)" <[email protected]>
2025-12-17 13:00 ` "sehrope (@sehrope)" <[email protected]>
2025-12-24 14:20 ` "vlsi (@vlsi)" <[email protected]>
2025-12-24 14:23 ` "vlsi (@vlsi)" <[email protected]>
2025-12-24 14:24 ` "vlsi (@vlsi)" <[email protected]>
2025-12-24 14:26 ` "vlsi (@vlsi)" <[email protected]>
2025-12-24 15:17 ` "davecramer (@davecramer)" <[email protected]>
2025-12-24 15:42 ` "vlsi (@vlsi)" <[email protected]>
2025-12-24 15:55 ` "davecramer (@davecramer)" <[email protected]>
2026-02-23 12:49 ` "davecramer (@davecramer)" <[email protected]>
2026-02-23 13:02 ` "vlsi (@vlsi)" <[email protected]>
2026-02-23 13:11 ` "vlsi (@vlsi)" <[email protected]>
2026-02-23 13:16 ` "vlsi (@vlsi)" <[email protected]>
2026-02-23 13:17 ` "vlsi (@vlsi)" <[email protected]>
2026-02-23 13:21 ` "vlsi (@vlsi)" <[email protected]>
2026-02-23 16:23 ` "davecramer (@davecramer)" <[email protected]>
2026-02-23 19:15 ` "vlsi (@vlsi)" <[email protected]>
2026-02-23 19:28 ` "davecramer (@davecramer)" <[email protected]>
2026-02-23 19:46 ` "davecramer (@davecramer)" <[email protected]>
2026-02-24 16:16 ` "sehrope (@sehrope)" <[email protected]>
2026-02-24 16:27 ` "davecramer (@davecramer)" <[email protected]>
2026-02-24 16:32 ` "vlsi (@vlsi)" <[email protected]>
2026-02-24 16:36 ` "vlsi (@vlsi)" <[email protected]>
2026-02-24 16:44 ` "davecramer (@davecramer)" <[email protected]>
2026-02-25 11:51 ` "davecramer (@davecramer)" <[email protected]>
2026-02-27 14:00 ` "davecramer (@davecramer)" <[email protected]>
2026-02-27 14:14 ` "vlsi (@vlsi)" <[email protected]>
2026-02-27 14:43 ` "davecramer (@davecramer)" <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox