From 4605fdd5514286f2ebdda0e457a41917985ca78f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lars=20Sj=C3=B6str=C3=B6m?= <lars.sjostrom@derivco.se>
Date: Thu, 28 Sep 2017 15:43:42 +0200
Subject: [PATCH] connector/gitlab: Fix regexp in Link parser

---
 connector/gitlab/gitlab.go      | 42 ++++++++++++++++++++-------------
 connector/gitlab/gitlab_test.go | 30 +++++++++++++++++++++++
 2 files changed, 55 insertions(+), 17 deletions(-)
 create mode 100644 connector/gitlab/gitlab_test.go

diff --git a/connector/gitlab/gitlab.go b/connector/gitlab/gitlab.go
index 60d2cdc2..a4da0197 100644
--- a/connector/gitlab/gitlab.go
+++ b/connector/gitlab/gitlab.go
@@ -22,6 +22,11 @@ const (
 	scopeAPI  = "api"
 )
 
+var (
+	reNext = regexp.MustCompile("<([^>]+)>; rel=\"next\"")
+	reLast = regexp.MustCompile("<([^>]+)>; rel=\"last\"")
+)
+
 // Config holds configuration options for gilab logins.
 type Config struct {
 	BaseURL      string `json:"baseURL"`
@@ -236,9 +241,6 @@ func (c *gitlabConnector) groups(ctx context.Context, client *http.Client) ([]st
 
 	apiURL := c.baseURL + "/api/v4/groups"
 
-	reNext := regexp.MustCompile("<(.*)>; rel=\"next\"")
-	reLast := regexp.MustCompile("<(.*)>; rel=\"last\"")
-
 	groups := []string{}
 	var gitlabGroups []gitlabGroup
 	for {
@@ -272,22 +274,28 @@ func (c *gitlabConnector) groups(ctx context.Context, client *http.Client) ([]st
 
 		link := resp.Header.Get("Link")
 
-		if len(reLast.FindStringSubmatch(link)) > 1 {
-			lastPageURL := reLast.FindStringSubmatch(link)[1]
-
-			if apiURL == lastPageURL {
-				break
-			}
-		} else {
+		apiURL = nextURL(apiURL, link)
+		if apiURL == "" {
 			break
 		}
-
-		if len(reNext.FindStringSubmatch(link)) > 1 {
-			apiURL = reNext.FindStringSubmatch(link)[1]
-		} else {
-			break
-		}
-
 	}
 	return groups, nil
 }
+
+func nextURL(url string, link string) string {
+	if len(reLast.FindStringSubmatch(link)) > 1 {
+		lastPageURL := reLast.FindStringSubmatch(link)[1]
+
+		if url == lastPageURL {
+			return ""
+		}
+	} else {
+		return ""
+	}
+
+	if len(reNext.FindStringSubmatch(link)) > 1 {
+		return reNext.FindStringSubmatch(link)[1]
+	}
+
+	return ""
+}
diff --git a/connector/gitlab/gitlab_test.go b/connector/gitlab/gitlab_test.go
new file mode 100644
index 00000000..ef669261
--- /dev/null
+++ b/connector/gitlab/gitlab_test.go
@@ -0,0 +1,30 @@
+package gitlab
+
+import "testing"
+
+var nextURLTests = []struct {
+	link     string
+	expected string
+}{
+	{"<https://gitlab.com/api/v4/groups?page=2&per_page=20>; rel=\"next\", " +
+		"<https://gitlab.com/api/v4/groups?page=1&per_page=20>; rel=\"prev\"; pet=\"cat\", " +
+		"<https://gitlab.com/api/v4/groups?page=3&per_page=20>; rel=\"last\"",
+		"https://gitlab.com/api/v4/groups?page=2&per_page=20"},
+	{"<https://gitlab.com/api/v4/groups?page=3&per_page=20>; rel=\"next\", " +
+		"<https://gitlab.com/api/v4/groups?page=2&per_page=20>; rel=\"prev\"; pet=\"dog\", " +
+		"<https://gitlab.com/api/v4/groups?page=3&per_page=20>; rel=\"last\"",
+		"https://gitlab.com/api/v4/groups?page=3&per_page=20"},
+	{"<https://gitlab.com/api/v4/groups?page=3&per_page=20>; rel=\"prev\"; pet=\"bunny\", " +
+		"<https://gitlab.com/api/v4/groups?page=3&per_page=20>; rel=\"last\"",
+		""},
+}
+
+func TestNextURL(t *testing.T) {
+	apiURL := "https://gitlab.com/api/v4/groups"
+	for _, tt := range nextURLTests {
+		apiURL = nextURL(apiURL, tt.link)
+		if apiURL != tt.expected {
+			t.Errorf("Should have returned %s, got %s", tt.expected, apiURL)
+		}
+	}
+}