Skip to content

Add pg connection options creation without environment #3832

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add pg connection options creation without environment #3832

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 36 additions & 10 deletions sqlx-postgres/src/options/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,12 @@ impl PgConnectOptions {

let database = var("PGDATABASE").ok();

let ssl_mode = var("PGSSLMODE")
.ok()
.and_then(|v| v.parse().ok())
.unwrap_or_default();

PgConnectOptions {
port,
host,
socket: None,
username,
password: var("PGPASSWORD").ok(),
database,
ssl_root_cert: var("PGSSLROOTCERT").ok().map(CertificateInput::from),
Expand All @@ -81,15 +82,40 @@ impl PgConnectOptions {
// `-----BEGIN CERTIFICATE-----` and so will not attempt to parse
// a PEM-encoded private key.
ssl_client_key: var("PGSSLKEY").ok().map(CertificateInput::from),
ssl_mode: var("PGSSLMODE")
.ok()
.and_then(|v| v.parse().ok())
.unwrap_or_default(),
statement_cache_capacity: 100,
application_name: var("PGAPPNAME").ok(),
options: var("PGOPTIONS").ok(),
..Self::new_without_environment(host, port, username, ssl_mode)
}
}

/// Create a minimal set of connection options _without_ applying any environment.
///
/// To be used in situations the environment is not controlled enough to be relied upon.
/// Does not respect any `PG*` environment variables.
///
/// See the type level-documentation for details.
pub fn new_without_environment(
host: String,
port: u16,
username: String,
ssl_mode: PgSslMode,
Comment on lines +98 to +101
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

port and ssl_mode have reasonable defaults: 5432 and Prefer, respectively.

host could arguably default to localhost or the standard Unix socket path for the platform; we already have that logic implemented in this file.

username is the hardest to choose a reasonable default for. Defaulting to the OS username doesn't really make sense to me anymore since that assumes the application is running on the same system as the database, which I don't expect is all that common these days; we only default to it because that's what libpq does. I would say postgres since that's generally the username that the Postgres server is installed under.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mostly did a mechanical transformation of the existing new_without_pgpass function. Very happy to go beyond that.

Yeah happy to add a default port.

But I think we should reconsider on ssl_mode, if SSL is supported by the server it behaves like require which verifies nothing. Having users explicitly think about ssl_mode may be the best option.

Alternatively we go for: 2 functions: new which would default to ssl_mode = Disable and new_ssl with ssl_mode = verify-full both documented.

This way we can steer users to the most appropriate defaults and make them explicitly think about what the needs of their environment and its reuqirement is. I'd love to do my part to avoid having to flag (and demo, and report) the MITM-ability of ssl_mode = require.

) -> Self {
PgConnectOptions {
host,
port,
socket: None,
username,
password: None,
database: None,
ssl_mode,
ssl_root_cert: None,
ssl_client_cert: None,
ssl_client_key: None,
statement_cache_capacity: 100,
application_name: None,
extra_float_digits: Some("2".into()),
log_settings: Default::default(),
options: var("PGOPTIONS").ok(),
options: None,
}
}

Expand Down