jackc / pgx

PostgreSQL driver and toolkit for Go
MIT License
10.83k stars 845 forks source link

add message body size limits in frontend and backend #2138

Closed zenkovev closed 1 month ago

zenkovev commented 1 month ago

Refers to https://github.com/jackc/pgx/issues/2139

jackc commented 1 month ago

I'm not opposed to the underlying idea, but globals aren't the correct approach. You can control the frontend creation via the BuildFrontend function in pgconn.Config. And you can also directly access the frontend via https://pkg.go.dev/github.com/jackc/pgx/v5@v5.7.1/pgconn#PgConn.Frontend. So any setting should be controlled via one of those means. Not a global with atomics.

zenkovev commented 1 month ago

Thanks for the reply. Regarding the control over the frontend creation, yes, it's fair, then global atomics are really not needed. There remains control over the length of the frontend message at the structure level - PR has been updated.

jackc commented 1 month ago

The Frontend changes look okay. Seem to be just porting the existing system from the backend. But I'm not sure why there are any changes on Backend.

Also, the comment style of having the comments below the field instead of above or beside looks odd to me (Frontend.maxBodyLen).

zenkovev commented 1 month ago

I fixed the comments. About the changes in the Backend: there is one change transferred from the Frontend, checking that a non-negative number is set in b.bodyLen, that is, that the specified length of the message is correct.

jackc commented 1 month ago

👍