From 79612d488d8cc5c1dcc917a4965790323719e5b2 Mon Sep 17 00:00:00 2001
From: Peter Downs <github@peterdowns.com>
Date: Wed, 25 Sep 2024 14:12:26 -0400
Subject: [PATCH] fix: randomID returns shorter IDs with same amount of entropy
 (#17)

As reported in https://github.com/peterldowns/pgtestdb/issues/16, the
previous implementation of `randomID` used a hashing construction (a)
inappropriately, (b) incorrectly. As a result, the IDs that were being
generated had a random prefix and then a consistent suffix, like this:

```
3aa05350d41d8cd98f00b204e9800998ecf8427e
f1c841c1d41d8cd98f00b204e9800998ecf8427e
e5dde3f7d41d8cd98f00b204e9800998ecf8427e
```

This PR fixes the function to essentially just return the randomly
generated prefixes, and omit the suffix entirely. The fixed-suffix was
always a mistake and never intended, I just wasn't paying close
attention beyond "are the instance database names colliding".

I tested this change by running some tests that used `pgtestdb` and
intentionally failed, then checking the connection strings that were
printed as part of the failing test's logs. Here's an example:

```
testdbconf: postgres://pgtdbuser:pgtdbpass@localhost:5433/testdb_tpl_c2b33a22a68750f84b5af76e9774be0e_inst_a4a3ce6c?sslmode=disabl
```

The key part is `inst_a4a3ce6c`, which is now (a) shorter than before,
(b) just as random.
---
 internal/withdb/withdb.go | 9 +++++----
 testdb.go                 | 4 +---
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/internal/withdb/withdb.go b/internal/withdb/withdb.go
index 7d0726e..04a3748 100644
--- a/internal/withdb/withdb.go
+++ b/internal/withdb/withdb.go
@@ -4,7 +4,6 @@ package withdb
 
 import (
 	"context"
-	"crypto/md5"
 	"crypto/rand"
 	"database/sql"
 	"encoding/hex"
@@ -60,14 +59,16 @@ func WithDB(ctx context.Context, driverName string, cb func(*sql.DB) error) (fin
 	return cb(testDB)
 }
 
+// randomID is a helper for coming up with the names of the instance databases.
+// It uses 32 random bits in the name, which means collisions are unlikely.
 func randomID(prefix string) (string, error) {
-	bytes := make([]byte, 32)
-	hash := md5.New()
+	bytes := make([]byte, 4)
 	_, err := rand.Read(bytes)
 	if err != nil {
 		return "", err
 	}
-	return fmt.Sprintf("%s_%s", prefix, hex.EncodeToString(hash.Sum(bytes))), nil
+	suffix := hex.EncodeToString(bytes)
+	return fmt.Sprintf("%s_%s", prefix, suffix), nil
 }
 
 func connectionString(dbname string) string {
diff --git a/testdb.go b/testdb.go
index a9a29a8..95c014a 100644
--- a/testdb.go
+++ b/testdb.go
@@ -2,7 +2,6 @@ package pgtestdb
 
 import (
 	"context"
-	"crypto/md5"
 	"crypto/rand"
 	"database/sql"
 	"encoding/hex"
@@ -467,12 +466,11 @@ func createInstance(
 // It uses 32 random bits in the name, which means collisions are unlikely.
 func randomID() string {
 	bytes := make([]byte, 4)
-	hash := md5.New()
 	_, err := rand.Read(bytes)
 	if err != nil {
 		panic(err)
 	}
-	return hex.EncodeToString(hash.Sum(bytes))
+	return hex.EncodeToString(bytes)
 }
 
 // NoopMigrator fulfills the Migrator interface but does absolutely nothing.