Repository: supabase/cli
Pull request #512 (2022-10-12 11:34:22)

fix: improper url-escaping of db password
## What kind of change does this PR introduce?

Bug fix

## What is the current behavior?

db password `pass word` is parsed as `pass+word` because of `url.QueryEscape()`

## What is the new behavior?

db password `pass word` is parsed as `pass word`


coveralls: ## Pull Request Test Coverage Report for [Build 3234327762](https://coveralls.io/builds/53206398)

* **11** of **15**   **(73.33%)**  changed or added relevant lines in **4** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage increased (+**0.5%**) to **47.096%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [internal/db/remote/commit/commit.go](https://coveralls.io/builds/53206398/source?filename=internal%2Fdb%2Fremote%2Fcommit%2Fcommit.go#L97) | 11 | 12 | 91.67%
| [internal/link/link.go](https://coveralls.io/builds/53206398/source?filename=internal%2Flink%2Flink.go#L24) | 0 | 1 | 0.0%
| [internal/migration/list/list.go](https://coveralls.io/builds/53206398/source?filename=internal%2Fmigration%2Flist%2Flist.go#L26) | 0 | 1 | 0.0%
| [internal/utils/api.go](https://coveralls.io/builds/53206398/source?filename=internal%2Futils%2Fapi.go#L205) | 0 | 1 | 0.0%
<!-- | **Total:** | **11** | **15** | **73.33%** | -->


|  Totals | [![Coverage Status](https://coveralls.io/builds/53206398/badge)](https://coveralls.io/builds/53206398) |
| :-- | --: |
| Change from base [Build 3232559127](https://coveralls.io/builds/53198301): |  0.5% |
| Covered Lines: | 2327 |
| Relevant Lines: | 4941 |

---
##### 💛  - [Coveralls](https://coveralls.io)

soedirgo: +1 to that! 😄

Patch:
diff --git a/internal/db/remote/commit/commit.go b/internal/db/remote/commit/commit.go
index b1b3cae5..dd9be3b9 100644
--- a/internal/db/remote/commit/commit.go
+++ b/internal/db/remote/commit/commit.go
@@ -7,7 +7,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"net/url"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -95,7 +94,7 @@ func run(p utils.Program, ctx context.Context, username, password, database stri
 	if err != nil {
 		return err
 	}
-	conn, err := ConnectRemotePostgres(ctx, username, password, database, projectRef)
+	conn, err := ConnectRemotePostgres(ctx, username, password, database, utils.GetSupabaseDbHost(projectRef))
 	if err != nil {
 		return err
 	}
@@ -429,21 +428,19 @@ func AssertPostgresVersionMatch(conn *pgx.Conn) error {
 }
 
 // Connnect to remote Postgres with optimised settings. The caller is responsible for closing the connection returned.
-func ConnectRemotePostgres(ctx context.Context, username, password, database, host string) (*pgx.Conn, error) {
+func ConnectRemotePostgres(ctx context.Context, username, password, database, host string, options ...func(*pgx.ConnConfig)) (*pgx.Conn, error) {
 	// Build connection string
-	pgUrl := fmt.Sprintf(
-		// Use port 6543 for connection pooling
-		"postgresql://%s:%s@%s:6543/%s?connect_timeout=3",
-		url.QueryEscape(username),
-		url.QueryEscape(password),
-		utils.GetSupabaseDbHost(url.QueryEscape(host)),
-		url.QueryEscape(database),
-	)
+	// Use port 6543 for connection pooling
+	pgUrl := "postgresql://:@:6543/?connect_timeout=3"
 	// Parse connection url
 	config, err := pgx.ParseConfig(pgUrl)
 	if err != nil {
 		return nil, err
 	}
+	config.User = username
+	config.Password = password
+	config.Host = host
+	config.Database = database
 	// Simple protocol is preferred over pgx default Parse -> Bind flow because
 	//   1. Using a single command for each query reduces RTT over an Internet connection.
 	//   2. Performance gains from using the alternate binary protocol is negligible because
@@ -452,6 +449,10 @@ func ConnectRemotePostgres(ctx context.Context, username, password, database, ho
 	//      Since CLI workloads are one-off scripts, we don't use connection pooling and hence
 	//      don't benefit from per connection server side cache.
 	config.PreferSimpleProtocol = true
+	// Apply config overrides
+	for _, op := range options {
+		op(config)
+	}
 	if viper.GetBool("DEBUG") {
 		debug.SetupPGX(config)
 	}
diff --git a/internal/link/link.go b/internal/link/link.go
index 806bb8ec..402f832c 100644
--- a/internal/link/link.go
+++ b/internal/link/link.go
@@ -21,7 +21,7 @@ func Run(ctx context.Context, projectRef, username, password, database string, f
 
 	// 2. Check database connection
 	{
-		conn, err := commit.ConnectRemotePostgres(ctx, username, password, database, projectRef)
+		conn, err := commit.ConnectRemotePostgres(ctx, username, password, database, utils.GetSupabaseDbHost(projectRef))
 		if err != nil {
 			return err
 		}
diff --git a/internal/migration/list/list.go b/internal/migration/list/list.go
index b1261b24..86e04c2a 100644
--- a/internal/migration/list/list.go
+++ b/internal/migration/list/list.go
@@ -23,7 +23,7 @@ func Run(ctx context.Context, username, password, database string, fsys afero.Fs
 	if err != nil {
 		return err
 	}
-	conn, err := commit.ConnectRemotePostgres(ctx, username, password, database, projectRef)
+	conn, err := commit.ConnectRemotePostgres(ctx, username, password, database, utils.GetSupabaseDbHost(projectRef))
 	if err != nil {
 		return err
 	}
diff --git a/internal/utils/api.go b/internal/utils/api.go
index 125ac444..01bf5b2c 100644
--- a/internal/utils/api.go
+++ b/internal/utils/api.go
@@ -202,6 +202,6 @@ func GetSupabaseDbHost(projectRef string) string {
 	case "https://api.supabase.green":
 		return fmt.Sprintf("db.%s.supabase.red", projectRef)
 	default:
-		return "http://localhost:5432"
+		return fmt.Sprintf("db.%s.supabase.red", projectRef)
 	}
 }
diff --git a/internal/utils/parser/token.go b/internal/utils/parser/token.go
index 05910097..98e3f88e 100644
--- a/internal/utils/parser/token.go
+++ b/internal/utils/parser/token.go
@@ -17,30 +17,29 @@ const (
 
 // State transition table for tokenizer:
 //
-//   Ready -> Ready (default)
-//   Ready -> Error (on invalid syntax)
-//   Ready -> Done (on ;, emit token)
-//   Ready -> Done (on EOF, emit token)
+//	Ready -> Ready (default)
+//	Ready -> Error (on invalid syntax)
+//	Ready -> Done (on ;, emit token)
+//	Ready -> Done (on EOF, emit token)
 //
-//   Ready -> Comment (on --)
-//   Comment -> Comment (default)
-//   Comment -> Ready (on \n)
+//	Ready -> Comment (on --)
+//	Comment -> Comment (default)
+//	Comment -> Ready (on \n)
 //
-//   Ready -> Block (on /*)
-//   Block -> Block (on /*, +-depth)
-//   Block -> Ready (on */, depth 0)
+//	Ready -> Block (on /*)
+//	Block -> Block (on /*, +-depth)
+//	Block -> Ready (on */, depth 0)
 //
-//   Ready -> Quote (on ')
-//   Quote -> Quote (on '', default)
-//   Quote -> Ready (on ')
+//	Ready -> Quote (on ')
+//	Quote -> Quote (on '', default)
+//	Quote -> Ready (on ')
 //
-//   Ready -> Dollar (on $tag$)
-//   Dollar -> Dollar (default)
-//   Dollar -> Ready (on $tag$)
-//
-//   Ready -> Escape (on \)
-//   Escape -> Ready (on next)
+//	Ready -> Dollar (on $tag$)
+//	Dollar -> Dollar (default)
+//	Dollar -> Ready (on $tag$)
 //
+//	Ready -> Escape (on \)
+//	Escape -> Ready (on next)
 type tokenizer struct {
 	state State
 	last  int
diff --git a/tools/docgen/main.go b/tools/docgen/main.go
index 16dd7b73..779fbcd6 100644
--- a/tools/docgen/main.go
+++ b/tools/docgen/main.go
@@ -107,17 +107,17 @@ func GenYamlDoc(cmd *cobra.Command, root *SpecDoc) CmdDoc {
 
 // Wraps a command string in markdown style code block, ie.
 //
-//   ```sh
-//   echo "hello world"
-//   ```
+//	```sh
+//	echo "hello world"
+//	```
 func mdCodeBlock(script string, language string) string {
 	return "```" + language + "\n" + strings.Trim(script, "\n") + "\n```"
 }
 
 // Yaml lib generates incorrect yaml with long strings that do not contain \n.
 //
-//   example: 'a a a a a a a a a a a a a a a a a a a a a a a a a a a a a a a a a a a
-//     a a a a a a '
+//	example: 'a a a a a a a a a a a a a a a a a a a a a a a a a a a a a a a a a a a
+//	  a a a a a a '
 func forceMultiLine(s string) string {
 	if len(s) > 60 && !strings.Contains(s, "\n") {
 		s = s + "\n"


Test patch:
diff --git a/internal/db/remote/commit/commit_test.go b/internal/db/remote/commit/commit_test.go
new file mode 100644
index 00000000..f2eeaad1
--- /dev/null
+++ b/internal/db/remote/commit/commit_test.go
@@ -0,0 +1,37 @@
+package commit
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/supabase/cli/internal/testing/pgtest"
+)
+
+func TestConnectRemotePostgres(t *testing.T) {
+	t.Run("connects to remote postgres successfully", func(t *testing.T) {
+		// Setup mock postgres
+		conn := pgtest.NewConn()
+		defer conn.Close(t)
+
+		// Run test
+		c, err := ConnectRemotePostgres(context.Background(), "username", "password", "database", "localhost", conn.Intercept)
+		require.NoError(t, err)
+		defer c.Close(context.Background())
+		assert.NoError(t, err)
+	})
+
+	t.Run("preserves db password", func(t *testing.T) {
+		// Setup mock postgres
+		conn := pgtest.NewConn()
+		defer conn.Close(t)
+
+		// Run test
+		password := "pass word"
+		c, err := ConnectRemotePostgres(context.Background(), "username", password, "database", "localhost", conn.Intercept)
+		require.NoError(t, err)
+		defer c.Close(context.Background())
+		assert.Equal(t, password, c.Config().Password)
+	})
+}
diff --git a/test/db_test.go b/test/db_test.go
index 4fd35610..58850003 100644
--- a/test/db_test.go
+++ b/test/db_test.go
@@ -2,7 +2,7 @@ package integration
 
 // Basic imports
 import (
-	"io/ioutil"
+	"io"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -43,7 +43,7 @@ func (suite *DBTestSuite) SetupTest() {
 	// add docker mock handlers
 	DockerMock.ExecCreateHandler = func(c *gin.Context) {
 		suite.addParams(c.Copy())
-		body, err := ioutil.ReadAll(c.Request.Body)
+		body, err := io.ReadAll(c.Request.Body)
 		if err != nil {
 			c.JSON(http.StatusBadRequest, gin.H{
 				"message": "error reading body",
@@ -61,7 +61,7 @@ func (suite *DBTestSuite) SetupTest() {
 
 	DockerMock.ExecStartHandler = func(c *gin.Context) {
 		suite.addParams(c.Copy())
-		body, err := ioutil.ReadAll(c.Request.Body)
+		body, err := io.ReadAll(c.Request.Body)
 		if err != nil {
 			c.JSON(http.StatusBadRequest, gin.H{
 				"message": "error reading body",
diff --git a/test/link_test.go b/test/link_test.go
index cb2e1ff9..1d0704ce 100644
--- a/test/link_test.go
+++ b/test/link_test.go
@@ -4,7 +4,6 @@ package integration
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 	"os"
 	"sync"
@@ -56,7 +55,7 @@ func (suite *LinkTestSuite) TestLink() {
 	})
 	_, err = os.Stat(utils.ProjectRefPath)
 	require.NoError(suite.T(), err)
-	ref, err := ioutil.ReadFile(utils.ProjectRefPath)
+	ref, err := os.ReadFile(utils.ProjectRefPath)
 	require.NoError(suite.T(), err)
 	require.Equal(suite.T(), id, string(ref))
 }
diff --git a/test/login_test.go b/test/login_test.go
index f5130174..84bc2528 100644
--- a/test/login_test.go
+++ b/test/login_test.go
@@ -2,7 +2,6 @@ package integration
 
 // Basic imports
 import (
-	"io/ioutil"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -38,7 +37,7 @@ func (suite *LoginTestSuite) TestLink() {
 
 	// change stdin to read from a file
 	content := []byte(key)
-	tmpfile, err := ioutil.TempFile(suite.tempDir, "key")
+	tmpfile, err := os.CreateTemp(suite.tempDir, "key")
 	require.NoError(suite.T(), err)
 	defer os.Remove(tmpfile.Name()) // clean up
 
@@ -58,7 +57,7 @@ func (suite *LoginTestSuite) TestLink() {
 	require.NoError(suite.T(), err)
 	_, err = os.Stat(filepath.Join(home, ".supabase/access-token"))
 	require.NoError(suite.T(), err)
-	token, err := ioutil.ReadFile(filepath.Join(home, ".supabase/access-token"))
+	token, err := os.ReadFile(filepath.Join(home, ".supabase/access-token"))
 	require.NoError(suite.T(), err)
 	require.Equal(suite.T(), key, string(token))
 }
diff --git a/test/secrets_test.go b/test/secrets_test.go
index 527f0554..7aed4492 100644
--- a/test/secrets_test.go
+++ b/test/secrets_test.go
@@ -4,7 +4,6 @@ package integration
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 	"os"
 	"sync"
@@ -39,10 +38,10 @@ func (suite *SecretsTestSuite) TestList() {
 	require.NoError(suite.T(), err)
 
 	ref := gonanoid.MustGenerate(supabase.IDAlphabet, supabase.IDLength)
-	require.NoError(suite.T(), ioutil.WriteFile(utils.ProjectRefPath, []byte(ref), os.FileMode(0755)))
+	require.NoError(suite.T(), os.WriteFile(utils.ProjectRefPath, []byte(ref), os.FileMode(0755)))
 
 	// set stdout to write into file so we can capture cmd output
-	tmpfile, err := ioutil.TempFile(suite.tempDir, "output")
+	tmpfile, err := os.CreateTemp(suite.tempDir, "output")
 	require.NoError(suite.T(), err)
 	defer os.Remove(tmpfile.Name()) // clean up
 	oldStdout := os.Stdout
@@ -61,7 +60,7 @@ func (suite *SecretsTestSuite) TestList() {
 		"User-Agent":      []string{"Go-http-client/1.1"},
 	})
 
-	contents, err := ioutil.ReadFile(tmpfile.Name())
+	contents, err := os.ReadFile(tmpfile.Name())
 	require.NoError(suite.T(), err)
 	require.Contains(suite.T(), string(contents), "some-key")
 	require.Contains(suite.T(), string(contents), "another")
diff --git a/test/status_test.go b/test/status_test.go
index 5e18c7b2..3b540531 100644
--- a/test/status_test.go
+++ b/test/status_test.go
@@ -3,7 +3,6 @@ package integration
 // Basic imports
 import (
 	"context"
-	"io/ioutil"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -35,7 +34,7 @@ func (suite *StatusTestSuite) TestStatus() {
 	require.NoError(suite.T(), err)
 
 	// set stdout to write into file so we can capture cmd output
-	tmpfile, err := ioutil.TempFile(suite.tempDir, "output")
+	tmpfile, err := os.CreateTemp(suite.tempDir, "output")
 	require.NoError(suite.T(), err)
 	defer os.Remove(tmpfile.Name()) // clean up
 	oldStdout := os.Stdout
@@ -55,7 +54,7 @@ func (suite *StatusTestSuite) TestStatus() {
 		},
 	})
 
-	contents, err := ioutil.ReadFile(tmpfile.Name())
+	contents, err := os.ReadFile(tmpfile.Name())
 	require.NoError(suite.T(), err)
 	require.Contains(suite.T(), string(contents), "API URL: http://localhost:54321")
 	require.Contains(suite.T(), string(contents), "DB URL: postgresql://postgres:postgres@localhost:54322/postgres")