Do not scan binary values into strings

refs #219 and #228

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:

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()

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

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