From 8ce403bde7ea6d069c51b43a2ef7a710883e4af8 Mon Sep 17 00:00:00 2001
From: David Calavera <david.calavera@gmail.com>
Date: Fri, 28 Aug 2015 11:03:04 -0400
Subject: [PATCH] Fix server header parsing.

Signed-off-by: David Calavera <david.calavera@gmail.com>
---
 httputils/httputils.go      | 30 +++++++++++--------------
 httputils/httputils_test.go | 44 +++++++++++++++++++------------------
 2 files changed, 36 insertions(+), 38 deletions(-)

diff --git a/httputils/httputils.go b/httputils/httputils.go
index c586c02..0fd90ea 100644
--- a/httputils/httputils.go
+++ b/httputils/httputils.go
@@ -5,11 +5,15 @@ import (
 	"fmt"
 	"net/http"
 	"regexp"
-	"strings"
 
 	"github.com/docker/docker/pkg/jsonmessage"
 )
 
+var (
+	headerRegexp     = regexp.MustCompile(`^(?:(.+)/(.+))\s\((.+)\).*$`)
+	errInvalidHeader = errors.New("Bad header, should be in format `docker/version (platform)`")
+)
+
 // Download requests a given URL and returns an io.Reader.
 func Download(url string) (resp *http.Response, err error) {
 	if resp, err = http.Get(url); err != nil {
@@ -39,21 +43,13 @@ type ServerHeader struct {
 // ParseServerHeader extracts pieces from an HTTP server header
 // which is in the format "docker/version (os)" eg docker/1.8.0-dev (windows).
 func ParseServerHeader(hdr string) (*ServerHeader, error) {
-	re := regexp.MustCompile(`.*\((.+)\).*$`)
-	r := &ServerHeader{}
-	if matches := re.FindStringSubmatch(hdr); matches != nil {
-		r.OS = matches[1]
-		parts := strings.Split(hdr, "/")
-		if len(parts) != 2 {
-			return nil, errors.New("Bad header: '/' missing")
-		}
-		r.App = parts[0]
-		v := strings.Split(parts[1], " ")
-		if len(v) != 2 {
-			return nil, errors.New("Bad header: Expected single space")
-		}
-		r.Ver = v[0]
-		return r, nil
+	matches := headerRegexp.FindStringSubmatch(hdr)
+	if len(matches) != 4 {
+		return nil, errInvalidHeader
 	}
-	return nil, errors.New("Bad header: Failed regex match")
+	return &ServerHeader{
+		App: matches[1],
+		Ver: matches[2],
+		OS:  matches[3],
+	}, nil
 }
diff --git a/httputils/httputils_test.go b/httputils/httputils_test.go
index 2214624..61e98ba 100644
--- a/httputils/httputils_test.go
+++ b/httputils/httputils_test.go
@@ -1,19 +1,17 @@
 package httputils
 
-import (
-	"testing"
-)
+import "testing"
 
 func TestDownload(t *testing.T) {
 	_, err := Download("http://docker.com")
 
 	if err != nil {
-		t.Errorf("Expected error to not exist when Download(http://docker.com)")
+		t.Fatalf("Expected error to not exist when Download(http://docker.com)")
 	}
 
 	// Expected status code = 404
 	if _, err = Download("http://docker.com/abc1234567"); err == nil {
-		t.Errorf("Expected error to exist when Download(http://docker.com/abc1234567)")
+		t.Fatalf("Expected error to exist when Download(http://docker.com/abc1234567)")
 	}
 }
 
@@ -21,37 +19,41 @@ func TestNewHTTPRequestError(t *testing.T) {
 	errorMessage := "Some error message"
 	httpResponse, _ := Download("http://docker.com")
 	if err := NewHTTPRequestError(errorMessage, httpResponse); err.Error() != errorMessage {
-		t.Errorf("Expected err to equal error Message")
+		t.Fatalf("Expected err to equal error Message")
 	}
 }
 
 func TestParseServerHeader(t *testing.T) {
-	serverHeader, err := ParseServerHeader("bad header")
-	if err.Error() != "Bad header: Failed regex match" {
-		t.Errorf("Should fail when header can not be parsed")
+	if _, err := ParseServerHeader("bad header"); err != errInvalidHeader {
+		t.Fatalf("Should fail when header can not be parsed")
 	}
 
-	if serverHeader, err = ParseServerHeader("(bad header)"); err.Error() != "Bad header: '/' missing" {
-		t.Errorf("Should fail when header can not be parsed")
+	if _, err := ParseServerHeader("(bad header)"); err != errInvalidHeader {
+		t.Fatalf("Should fail when header can not be parsed")
 	}
 
-	if serverHeader, err = ParseServerHeader("(without/spaces)"); err.Error() != "Bad header: Expected single space" {
-		t.Errorf("Should fail when header can not be parsed")
+	if _, err := ParseServerHeader("(without/spaces)"); err != errInvalidHeader {
+		t.Fatalf("Should fail when header can not be parsed")
 	}
 
-	if serverHeader, err = ParseServerHeader("(header/with space)"); err != nil {
-		t.Errorf("Expected err to not exist when ParseServerHeader(\"(header/with space)\")")
+	if _, err := ParseServerHeader("(header/with space)"); err != errInvalidHeader {
+		t.Fatalf("Expected err to not exist when ParseServerHeader(\"(header/with space)\")")
 	}
 
-	if serverHeader.App != "(header" {
-		t.Errorf("Expected serverHeader.App to equal \"(header\"")
+	serverHeader, err := ParseServerHeader("foo/bar (baz)")
+	if err != nil {
+		t.Fatal(err)
 	}
 
-	if serverHeader.Ver != "with" {
-		t.Errorf("Expected serverHeader.Ver to equal \"with\"")
+	if serverHeader.App != "foo" {
+		t.Fatalf("Expected serverHeader.App to equal \"foo\", got %s", serverHeader.App)
 	}
 
-	if serverHeader.OS != "header/with space" {
-		t.Errorf("Expected serverHeader.OS to equal \"header/with space\"")
+	if serverHeader.Ver != "bar" {
+		t.Fatalf("Expected serverHeader.Ver to equal \"bar\", got %s", serverHeader.Ver)
+	}
+
+	if serverHeader.OS != "baz" {
+		t.Fatalf("Expected serverHeader.OS to equal \"baz\", got %s", serverHeader.OS)
 	}
 }