stop reading/writing null byte for empty strings

server/oscar/auth.go

@@ -72,7 +72,12 @@ func (rt AuthServer) handleNewConnection(rwc io.ReadWriteCloser) error {
 		return err
 	}
 
-	if _, hasRoastedPassword := signonFrame.Uint16(wire.LoginTLVTagsRoastedPassword); hasRoastedPassword {
+	// decide whether the client is using BUCP or FLAP authentication based on
+	// the presence of the screen name TLV. this block used to check for the
+	// presence of the roasted password TLV, however that proved an unreliable
+	// indicator of FLAP-auth because older ICQ clients appear to omit the
+	// roasted password TLV when the password is not stored client-side.
+	if _, hasScreenName := signonFrame.Uint16(wire.LoginTLVTagsScreenName); hasScreenName {
 		return rt.processFLAPAuth(signonFrame, flapc)
 	}
 

wire/decode.go

@@ -155,13 +155,12 @@ func unmarshalString(v reflect.Value, oscTag oscarTag, r io.Reader, order binary
 		if _, err := io.ReadFull(r, buf); err != nil {
 			return err
 		}
-	}
-
-	if oscTag.nullTerminated {
-		if buf[len(buf)-1] != 0x00 {
-			return errNotNullTerminated
+		if oscTag.nullTerminated {
+			if buf[len(buf)-1] != 0x00 {
+				return errNotNullTerminated
+			}
+			buf = buf[0 : len(buf)-1] // remove null terminator
 		}
-		buf = buf[0 : len(buf)-1] // remove null terminator
 	}
 
 	// todo is there a more efficient way?

wire/decode_test.go

@@ -146,6 +146,20 @@ func TestUnmarshal(t *testing.T) {
 				[]byte{0x0, 0xb}, /* len prefix */
 				[]byte{0x74, 0x65, 0x73, 0x74, 0x2d, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x00}...), /* str val */
 		},
+		{
+			name: "null-terminated string16 with len 0",
+			prototype: &struct {
+				Val string `oscar:"len_prefix=uint16,nullterm"`
+			}{},
+			want: &struct {
+				Val string `oscar:"len_prefix=uint16,nullterm"`
+			}{
+				Val: "",
+			},
+			given: append(
+				[]byte{0x0, 0x00}, /* len prefix */
+			),
+		},
 		{
 			name: "null-terminated string16 without null terminator",
 			prototype: &struct {

wire/encode.go

@@ -104,7 +104,7 @@ func marshalSlice(t reflect.Type, v reflect.Value, oscTag oscarTag, w io.Writer,
 
 func marshalString(oscTag oscarTag, v reflect.Value, w io.Writer, order binary.ByteOrder) error {
 	str := v.String()
-	if oscTag.nullTerminated {
+	if oscTag.nullTerminated && str != "" {
 		str = str + "\x00"
 	}
 	if oscTag.hasLenPrefix {
@@ -112,6 +112,9 @@ func marshalString(oscTag oscarTag, v reflect.Value, w io.Writer, order binary.B
 			return err
 		}
 	}
+	if str == "" {
+		return nil
+	}
 	return binary.Write(w, order, []byte(str))
 }
 

wire/encode_test.go

@@ -119,6 +119,18 @@ func TestMarshal(t *testing.T) {
 				[]byte{0x0, 0xb}, /* len prefix */
 				[]byte{0x74, 0x65, 0x73, 0x74, 0x2d, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x00}...), /* str val */
 		},
+		{
+			name: "null-terminated string16 with len 0",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val string `oscar:"len_prefix=uint16,nullterm"`
+			}{
+				Val: "",
+			},
+			want: append(
+				[]byte{0x0, 0x0}, /* len prefix */
+			),
+		},
 		{
 			name: "string16 write error",
 			w:    errWriter{},