Merge pull request #383 from Mytherin/dontleaksecret

Fix #368: display only attach path in connection error message, instead of the full DSN that is used to connect

Author: Mytherin

.github/workflows/Linux.yml

@@ -27,6 +27,18 @@ jobs:
       VCPKG_TOOLCHAIN_PATH: ${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake
 
     steps:
+    - name: Free disk space 1
+      uses: endersonmenezes/[email protected]
+      continue-on-error: true
+      with:
+#          remove_android: true # ~9.5GB in #52s
+#          remove_dotnet: true
+        remove_haskell: true # ~6.5GB in ~3s
+        remove_tool_cache: true # ~4.8GB in ~17s
+#          remove_swap: true
+#          remove_packages_one_command: true # ~7.5 GB in ~94s
+        testing: false
+
     - name: Install required ubuntu packages
       run: |
         sudo apt-get update -y -qq

src/include/postgres_connection.hpp

@@ -44,7 +44,7 @@ class PostgresConnection {
 	PostgresConnection &operator=(PostgresConnection &&) noexcept;
 
 public:
-	static PostgresConnection Open(const string &connection_string);
+	static PostgresConnection Open(const string &dsn, const string &attach_path);
 	void Execute(const string &query);
 	unique_ptr<PostgresResult> TryQuery(const string &query, optional_ptr<string> error_message = nullptr);
 	unique_ptr<PostgresResult> Query(const string &query);

src/include/postgres_scanner.hpp

@@ -38,6 +38,7 @@ struct PostgresBindData : public FunctionData {
 
 	idx_t pages_per_task = DEFAULT_PAGES_PER_TASK;
 	string dsn;
+	string attach_path;
 
 	bool requires_materialization = true;
 	bool can_use_main_thread = true;

src/include/postgres_utils.hpp

@@ -58,7 +58,7 @@ enum class PostgresIsolationLevel { READ_COMMITTED, REPEATABLE_READ, SERIALIZABL
 
 class PostgresUtils {
 public:
-	static PGconn *PGConnect(const string &dsn);
+	static PGconn *PGConnect(const string &dsn, const string &attach_path);
 
 	static LogicalType ToPostgresType(const LogicalType &input);
 	static LogicalType TypeToLogicalType(optional_ptr<PostgresTransaction> transaction,

src/postgres_attach.cpp

@@ -47,7 +47,7 @@ static void AttachFunction(ClientContext &context, TableFunctionInput &data_p, D
 		return;
 	}
 
-	auto conn = PostgresConnection::Open(data.dsn);
+	auto conn = PostgresConnection::Open(data.dsn, data.dsn);
 	auto dconn = Connection(context.db->GetDatabase(context));
 	auto fetch_table_query = StringUtil::Format(
 	    R"(

src/postgres_connection.cpp

@@ -40,10 +40,10 @@ PostgresConnection &PostgresConnection::operator=(PostgresConnection &&other) no
 	return *this;
 }
 
-PostgresConnection PostgresConnection::Open(const string &connection_string) {
+PostgresConnection PostgresConnection::Open(const string &dsn, const string &attach_path) {
 	PostgresConnection result;
-	result.connection = make_shared_ptr<OwnedPostgresConnection>(PostgresUtils::PGConnect(connection_string));
-	result.dsn = connection_string;
+	result.connection = make_shared_ptr<OwnedPostgresConnection>(PostgresUtils::PGConnect(dsn, attach_path));
+	result.dsn = dsn;
 	return result;
 }
 

src/postgres_scanner.cpp

@@ -177,8 +177,9 @@ static unique_ptr<FunctionData> PostgresBind(ClientContext &context, TableFuncti
 	bind_data->dsn = input.inputs[0].GetValue<string>();
 	bind_data->schema_name = input.inputs[1].GetValue<string>();
 	bind_data->table_name = input.inputs[2].GetValue<string>();
+	bind_data->attach_path = bind_data->dsn;
 
-	auto con = PostgresConnection::Open(bind_data->dsn);
+	auto con = PostgresConnection::Open(bind_data->dsn, bind_data->attach_path);
 	auto version = con.GetPostgresVersion();
 	// query the table schema so we can interpret the bits in the pages
 	auto info = PostgresTableSet::GetTableInfo(con, bind_data->schema_name, bind_data->table_name);
@@ -317,7 +318,7 @@ static unique_ptr<GlobalTableFunctionState> PostgresInitGlobalState(ClientContex
 		    bind_data.use_transaction ? transaction.GetConnection() : transaction.GetConnectionWithoutTransaction();
 		result->SetConnection(con.GetConnection());
 	} else {
-		auto con = PostgresConnection::Open(bind_data.dsn);
+		auto con = PostgresConnection::Open(bind_data.dsn, bind_data.attach_path);
 		if (bind_data.use_transaction) {
 			PostgresScanConnect(con, string());
 		}
@@ -401,7 +402,7 @@ bool PostgresGlobalState::TryOpenNewConnection(ClientContext &context, PostgresL
 		}
 		lstate.connection = PostgresConnection(lstate.pool_connection.GetConnection().GetConnection());
 	} else {
-		lstate.connection = PostgresConnection::Open(bind_data.dsn);
+		lstate.connection = PostgresConnection::Open(bind_data.dsn, bind_data.attach_path);
 	}
 	PostgresScanConnect(lstate.connection, snapshot);
 	return true;

src/postgres_utils.cpp

@@ -8,12 +8,12 @@ namespace duckdb {
 static void PGNoticeProcessor(void *arg, const char *message) {
 }
 
-PGconn *PostgresUtils::PGConnect(const string &dsn) {
+PGconn *PostgresUtils::PGConnect(const string &dsn, const string &attach_path) {
 	PGconn *conn = PQconnectdb(dsn.c_str());
 
 	// both PQStatus and PQerrorMessage check for nullptr
 	if (PQstatus(conn) == CONNECTION_BAD) {
-		throw IOException("Unable to connect to Postgres at %s: %s", dsn, string(PQerrorMessage(conn)));
+		throw IOException("Unable to connect to Postgres at \"%s\": %s", attach_path, string(PQerrorMessage(conn)));
 	}
 	PQsetNoticeProcessor(conn, PGNoticeProcessor, nullptr);
 	return conn;

src/storage/postgres_connection_pool.cpp

@@ -54,7 +54,8 @@ PostgresPoolConnection PostgresConnectionPool::GetConnectionInternal() {
 	}
 
 	// no cached connections left but there is space to open a new one - open it
-	return PostgresPoolConnection(this, PostgresConnection::Open(postgres_catalog.connection_string));
+	return PostgresPoolConnection(
+	    this, PostgresConnection::Open(postgres_catalog.connection_string, postgres_catalog.attach_path));
 }
 
 PostgresPoolConnection PostgresConnectionPool::ForceGetConnection() {

src/storage/postgres_table_entry.cpp

@@ -45,6 +45,7 @@ TableFunction PostgresTableEntry::GetScanFunction(ClientContext &context, unique
 	result->schema_name = schema.name;
 	result->table_name = name;
 	result->dsn = transaction.GetDSN();
+	result->attach_path = pg_catalog.attach_path;
 	result->SetCatalog(pg_catalog);
 	result->SetTable(*this);
 	for (auto &col : columns.Logical()) {