Skip to content

Commit

Permalink
fix: randomID returns shorter IDs with same amount of entropy (#17)
Browse files Browse the repository at this point in the history
As reported in #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.
  • Loading branch information
peterldowns authored Sep 25, 2024
1 parent 5b0be70 commit 79612d4
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 7 deletions.
9 changes: 5 additions & 4 deletions internal/withdb/withdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package withdb

import (
"context"
"crypto/md5"
"crypto/rand"
"database/sql"
"encoding/hex"
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 1 addition & 3 deletions testdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package pgtestdb

import (
"context"
"crypto/md5"
"crypto/rand"
"database/sql"
"encoding/hex"
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 79612d4

Please sign in to comment.