From 4d5622186822a1c14592e2183b030448f063af67 Mon Sep 17 00:00:00 2001
From: Jack Christensen <jack@jackchristensen.com>
Date: Thu, 16 Feb 2017 19:19:45 -0600
Subject: [PATCH] Do not scan binary values into strings

refs #219 and #228
---
 query.go      | 14 +++++++++++++-
 query_test.go | 17 +++++++++++++++++
 v3.md         |  4 ++--
 values.go     | 21 +++++++++++++++------
 4 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/query.go b/query.go
index efb039d5..99b383e0 100644
--- a/query.go
+++ b/query.go
@@ -301,7 +301,19 @@ func (rows *Rows) Values() ([]interface{}, error) {
 		// All intrinsic types (except string) are encoded with binary
 		// encoding so anything else should be treated as a string
 		case TextFormatCode:
-			values = append(values, vr.ReadString(vr.Len()))
+			switch vr.Type().DataType {
+			case JSONOID:
+				var d interface{}
+				decodeJSON(vr, &d)
+				values = append(values, d)
+			case JSONBOID:
+				var d interface{}
+				decodeJSONB(vr, &d)
+				values = append(values, d)
+			default:
+				values = append(values, vr.ReadString(vr.Len()))
+			}
+
 		case BinaryFormatCode:
 			switch vr.Type().DataType {
 			case TextOID, VarcharOID:
diff --git a/query_test.go b/query_test.go
index f2942951..a78914b6 100644
--- a/query_test.go
+++ b/query_test.go
@@ -100,6 +100,23 @@ func TestConnQueryValues(t *testing.T) {
 	}
 }
 
+// https://github.com/jackc/pgx/issues/228
+func TestRowsScanDoesNotAllowScanningBinaryFormatValuesIntoString(t *testing.T) {
+	t.Parallel()
+
+	conn := mustConnect(t, *defaultConnConfig)
+	defer closeConn(t, conn)
+
+	var s string
+
+	err := conn.QueryRow("select 1").Scan(&s)
+	if err == nil || !strings.Contains(err.Error(), "cannot decode binary value into string") {
+		t.Fatalf("Expected Scan to fail to encode binary value into string but: %v", err)
+	}
+
+	ensureConnValid(t, conn)
+}
+
 // Test that a connection stays valid when query results are closed early
 func TestConnQueryCloseEarly(t *testing.T) {
 	t.Parallel()
diff --git a/v3.md b/v3.md
index 4663cfd9..68619d4d 100644
--- a/v3.md
+++ b/v3.md
@@ -30,6 +30,8 @@ Conn.WaitForNotification no longer automatically pings internally every 15 secon
 
 ReplicationConn.WaitForReplicationMessage now takes context.Context instead of time.Duration for cancellation support.
 
+Reject scanning binary format values into a string (e.g. binary encoded timestamptz to string). See https://github.com/jackc/pgx/issues/219 and https://github.com/jackc/pgx/issues/228
+
 ## TODO / Possible / Investigate
 
 Organize errors better
@@ -56,8 +58,6 @@ Every field that should not be set by user should be replaced by accessor method
 
 Investigate strongly typed queries. i.e. Some sort of interface where varargs of Query, Exec, and Scan wouldn't happen. Need to be some low-level interface where (probably generated) functions could (more or less) directly read and write to the connection. Clean code and type-safety / control would be the benefits. Row scanning performance is already so fast there is little to improve (go_db_bench shows under 1 microsecond per row).
 
-Reject scanning non-string like things into a string (e.g. binary encoded timestamptz to string). See https://github.com/jackc/pgx/issues/223
-
 Further clean up logging interface -- still some pre-loglevel code in place
 Possibly integrate internal logging support with context. Possibly add method that adds arbitrary pgx log data to context. Or add ability to configure what key(s) pgx looks at for additional log context.
 Consider whether to switch to logrus style or stick with log15 style logs
diff --git a/values.go b/values.go
index a59ca0c3..4255f5ea 100644
--- a/values.go
+++ b/values.go
@@ -102,20 +102,15 @@ func init() {
 		"date":         BinaryFormatCode,
 		"float4":       BinaryFormatCode,
 		"float8":       BinaryFormatCode,
-		"json":         BinaryFormatCode,
-		"jsonb":        BinaryFormatCode,
 		"inet":         BinaryFormatCode,
 		"int2":         BinaryFormatCode,
 		"int4":         BinaryFormatCode,
 		"int8":         BinaryFormatCode,
-		"name":         BinaryFormatCode,
 		"oid":          BinaryFormatCode,
 		"record":       BinaryFormatCode,
-		"text":         BinaryFormatCode,
 		"tid":          BinaryFormatCode,
 		"timestamp":    BinaryFormatCode,
 		"timestamptz":  BinaryFormatCode,
-		"varchar":      BinaryFormatCode,
 		"xid":          BinaryFormatCode,
 	}
 }
@@ -2022,6 +2017,20 @@ func decodeText(vr *ValueReader) string {
 		return ""
 	}
 
+	if vr.Type().FormatCode == BinaryFormatCode {
+		vr.Fatal(ProtocolError("cannot decode binary value into string"))
+		return ""
+	}
+
+	return vr.ReadString(vr.Len())
+}
+
+func decodeTextAllowBinary(vr *ValueReader) string {
+	if vr.Len() == -1 {
+		vr.Fatal(ProtocolError("Cannot decode null into string"))
+		return ""
+	}
+
 	return vr.ReadString(vr.Len())
 }
 
@@ -2370,7 +2379,7 @@ func decodeRecord(vr *ValueReader) []interface{} {
 		case InetOID, CidrOID:
 			record = append(record, decodeInet(&fieldVR))
 		case TextOID, VarcharOID, UnknownOID:
-			record = append(record, decodeText(&fieldVR))
+			record = append(record, decodeTextAllowBinary(&fieldVR))
 		default:
 			vr.Fatal(fmt.Errorf("decodeRecord cannot decode oid %d", fd.DataType))
 			return nil