From 7ea110ec5c8e41c1d220d903e2b4aad67a9dd664 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Tue, 29 Aug 2017 22:48:17 -0700
Subject: [PATCH 01/39] =?UTF-8?q?Edit=20app=5Fconfig.go=20to=20add=20an=20?=
 =?UTF-8?q?ArtifactServer=20attribute=20to=20the=20appConfig=20struct=20th?=
 =?UTF-8?q?at=20is=20the=20url.URL=20type.=20Add=20new=20command=20line=20?=
 =?UTF-8?q?flag=20for=20the=20setting=20of=20the=20artifact-server=20optio?=
 =?UTF-8?q?n,=20then=20adjust=20the=20configFromFlags=20function=20to=20re?=
 =?UTF-8?q?ad=20the=20new=20artifact-server=20flag=20and=20include=20it=20?=
 =?UTF-8?q?while=20building=20the=20programs=20appConfig=20value.=20Creati?=
 =?UTF-8?q?on=20of=20new=20artifact=20struct=20and=20related=20functions?=
 =?UTF-8?q?=20with=20within=20artifact.go=20that=20is=20used=20to=20build?=
 =?UTF-8?q?=20url=E2=80=99s=20from=20the=20artifact-server=20flag=20value,?=
 =?UTF-8?q?=20values=20from=20within=20the=20host=20name,=20and=20the=20su?=
 =?UTF-8?q?bsequent=20http=20requests=20to=20this=20program.=20Integration?=
 =?UTF-8?q?=20of=20this=20artifact=20type=20within=20app.go=E2=80=99s=20se?=
 =?UTF-8?q?rveContent=20function=20to=20proxy=20requests=20where=20applica?=
 =?UTF-8?q?ble.=20Create=20unit=20tests=20accordingly=20#78?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 app.go           |  19 +++++++
 app_config.go    |   5 +-
 artifact.go      |  90 +++++++++++++++++++++++++++++++++
 artifact_test.go | 128 +++++++++++++++++++++++++++++++++++++++++++++++
 main.go          |  27 ++++++----
 5 files changed, 258 insertions(+), 11 deletions(-)
 create mode 100644 artifact.go
 create mode 100644 artifact_test.go

diff --git a/app.go b/app.go
index 61b1176c5..5fbfd39cc 100644
--- a/app.go
+++ b/app.go
@@ -19,6 +19,7 @@ import (
 
 const xForwardedProto = "X-Forwarded-Proto"
 const xForwardedProtoHTTPS = "https"
+const ArtifactPrefix = "artifact~"
 
 var (
 	corsHandler = cors.New(cors.Options{AllowedMethods: []string{"GET"}})
@@ -79,6 +80,24 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 	if err != nil {
 		host = r.Host
 	}
+
+	// In the event a host is prefixed with the artifact prefix an artifact
+	// value is created, and an attempt to proxy the request is made
+	if strings.HasPrefix(host, ArtifactPrefix) {
+		artifact, err := NewArtifact(
+			a.appConfig.ArtifactServer.String(),
+			artifactURLBody(host),
+			r.URL.Path,
+		)
+		if err != nil {
+			log.Println("Building of Artifact failed", err)
+			serve404(&w) // TODO: Should we respond with 500 here?
+			return
+		}
+		artifact.MakeRequest(&w)
+		return
+	}
+
 	domain := a.domain(host)
 	if domain == nil {
 		serve404(&w)
diff --git a/app_config.go b/app_config.go
index e45dda24f..24101ee1d 100644
--- a/app_config.go
+++ b/app_config.go
@@ -1,7 +1,10 @@
 package main
 
+import "net/url"
+
 type appConfig struct {
-	Domain string
+	Domain         string
+	ArtifactServer *url.URL
 
 	RootCertificate []byte
 	RootKey         []byte
diff --git a/artifact.go b/artifact.go
new file mode 100644
index 000000000..e472660f5
--- /dev/null
+++ b/artifact.go
@@ -0,0 +1,90 @@
+package main
+
+import (
+	"bytes"
+	"fmt"
+	"log"
+	"net/http"
+	"net/url"
+	"strconv"
+	"strings"
+	"time"
+)
+
+const (
+	artifactBodyBase = "/projects/%v/artifacts/%v/raw"
+)
+
+// Artifact is used for the composition of new URL's. In the event this program
+// is running on a host that is prefixed with artifact~ we attempt to proxy
+// requests  to a new URL that is composed of the Host, the artifacts-server
+// flag value, and the path of the request. This struct and functions surrounding
+// it provide support and functionality for these cases.
+type Artifact struct {
+	Prefix string
+	Body   string
+	Suffix string
+	*url.URL
+}
+
+// func NewArtifact is used as a builder to produce a pointer to a new Artifact.
+// Errors returned are related to the building of an underlying url.URL pointer
+// that is constructed from the arguments defined herein.
+func NewArtifact(prefix, body, suffix string) (*Artifact, error) {
+	urlString := fmt.Sprintf("%v%v%v", prefix, body, suffix)
+	u, err := url.Parse(urlString)
+	if err != nil {
+		return &Artifact{}, err
+	}
+
+	return &Artifact{
+		Prefix: prefix,
+		Body:   body,
+		Suffix: suffix,
+		URL:    u,
+	}, nil
+}
+
+// func MakeRequest given an http.ResponseWriter will proxy a request to the
+// underlying url.URL pointer's address on the related Artifact pointer. Making
+// a GET request, it then maps the Content-Type, Content-Length, and response
+// body from said GET request to the http.ResponseWriter. A 404 will be
+// served in the event that an error is encountered.
+func (a *Artifact) MakeRequest(w http.ResponseWriter) {
+	//custom http.Client{} is needed to specify request timeout
+	var client = &http.Client{
+		Timeout: time.Second * 10,
+	}
+
+	log.Println("Attempting to proxy request to", a.URL.String())
+
+	resp, err := client.Get(a.URL.String())
+	if err != nil {
+		log.Println("Error while proxying artifact request", err)
+		serve404(w) // TODO: Should we respond with 500 here?
+		return
+	}
+
+	defer resp.Body.Close()
+	buff := new(bytes.Buffer)
+	contentLength, err := buff.ReadFrom(resp.Body)
+	if err != nil {
+		log.Println("Error reading response from proxied request", err)
+		serve404(w) // TODO: Should we respond with 500 here?
+		return
+	}
+
+	w.Header().Set("Content-Type", resp.Header.Get("Content-Type"))
+	w.Header().Set("Content-Length", strconv.FormatInt(contentLength, 10))
+	w.Header().Set("Cache-Control", "max-age=600")
+	w.Write(buff.Bytes())
+}
+
+// func artifactURLBody returns a string that represents the body of a url an
+// artifact will ultimately proxy a request to. An expected host argument would
+// be close to "artifact~1~2". 1 and 2 represent Id's that need to be extracted
+// and interpolated into the const artifactBodyBase.
+func artifactURLBody(host string) string {
+	ids := strings.Split(strings.TrimPrefix(host, ArtifactPrefix), "~")
+	return fmt.Sprintf(artifactBodyBase, ids[0], ids[1])
+}
diff --git a/artifact_test.go b/artifact_test.go
new file mode 100644
index 000000000..dd41a90e6
--- /dev/null
+++ b/artifact_test.go
@@ -0,0 +1,128 @@
+package main
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestArtifactMakeRequest(t *testing.T) {
+	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/html; charset=utf-8")
+		fmt.Fprintln(w, "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
+	}))
+	defer testServer.Close()
+	artifactUrl, err := url.Parse(testServer.URL)
+	assert.NoError(t, err)
+	artifact := &Artifact{
+		URL: artifactUrl,
+	}
+
+	result := httptest.NewRecorder()
+	artifact.MakeRequest(result)
+
+	assert.Equal(t, result.Header().Get("Content-Type"), "text/html; charset=utf-8")
+	assert.Equal(t, result.Header().Get("Content-Length"), "91")
+	assert.Equal(t, result.Header().Get("Cache-Control"), "max-age=600")
+	assert.Equal(t, string(result.Body.Bytes()), "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>\n")
+}
+
+func TestArtifactMakeRequest_bad_request(t *testing.T) {
+	testServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+	defer testServer.Close()
+	artifactUrl, err := url.Parse(testServer.URL)
+	assert.NoError(t, err)
+	artifact := &Artifact{
+		URL: artifactUrl,
+	}
+
+	result := httptest.NewRecorder()
+	artifact.MakeRequest(result)
+
+	assert.Equal(t, 404, result.Code)
+}
+
+func TestNewArtifact(t *testing.T) {
+	cases := []struct {
+		Prefix      string
+		Body        string
+		Suffix      string
+		Expected    string
+		Description string
+	}{
+		{
+			"https://gitlab.com/api/v4",
+			"/projects/1/artifacts/2/raw",
+			"/index.html",
+			"https://gitlab.com/api/v4/projects/1/artifacts/2/raw/index.html",
+			"Basic case",
+		},
+		{
+			"http://some.otherhost.com/api/33",
+			"/projects/1/artifacts/2/raw",
+			"/file.txt",
+			"http://some.otherhost.com/api/33/projects/1/artifacts/2/raw/file.txt",
+			"Additional test wtih different domains",
+		},
+	}
+
+	for _, c := range cases {
+		a, err := NewArtifact(c.Prefix, c.Body, c.Suffix)
+		assert.NoError(t, err)
+		actual := a.URL.String()
+		assert.Equal(t, actual, c.Expected, c.Description)
+	}
+}
+
+func TestArtifactURLBody(t *testing.T) {
+	cases := []struct {
+		UrlHost     string
+		Expected    string
+		Description string
+	}{
+		{
+			"artifact~1~2",
+			"/projects/1/artifacts/2/raw",
+			"basic case",
+		},
+		{
+			"artifact~10000~20000",
+			"/projects/10000/artifacts/20000/raw",
+			"expanded case",
+		},
+		{
+			"artifact~86753095555~55550935768",
+			"/projects/86753095555/artifacts/55550935768/raw",
+			"large number case",
+		},
+		{
+			"artifact~one~two",
+			"/projects/one/artifacts/two/raw",
+			"letters rather than numbers",
+		},
+		{
+			"artifact~One111~tWo222",
+			"/projects/One111/artifacts/tWo222/raw",
+			"Mixture of alphanumeric",
+		},
+		{
+			"artifact~!@#$%~%$#@!",
+			"/projects/!@#$%/artifacts/%$#@!/raw",
+			"special characters",
+		},
+		{
+			"artifact~~",
+			"/projects//artifacts//raw", //@WIP how should this be handled, is it something to worry about?
+			"no ids",
+		},
+	}
+
+	for _, c := range cases {
+		actual := artifactURLBody(c.UrlHost)
+		assert.Equal(t, actual, c.Expected, c.Description)
+	}
+}
diff --git a/main.go b/main.go
index cacddb4f7..92d7dcc44 100644
--- a/main.go
+++ b/main.go
@@ -3,6 +3,7 @@ package main
 import (
 	"flag"
 	"log"
+	"net/url"
 	"os"
 	"strings"
 )
@@ -14,16 +15,17 @@ var VERSION = "dev"
 var REVISION = "HEAD"
 
 var (
-	pagesRootCert  = flag.String("root-cert", "", "The default path to file certificate to serve static pages")
-	pagesRootKey   = flag.String("root-key", "", "The default path to file certificate to serve static pages")
-	redirectHTTP   = flag.Bool("redirect-http", false, "Redirect pages from HTTP to HTTPS")
-	useHTTP2       = flag.Bool("use-http2", true, "Enable HTTP2 support")
-	pagesRoot      = flag.String("pages-root", "shared/pages", "The directory where pages are stored")
-	pagesDomain    = flag.String("pages-domain", "gitlab-example.com", "The domain to serve static pages")
-	pagesStatus    = flag.String("pages-status", "", "The url path for a status page, e.g., /@status")
-	metricsAddress = flag.String("metrics-address", "", "The address to listen on for metrics requests")
-	daemonUID      = flag.Uint("daemon-uid", 0, "Drop privileges to this user")
-	daemonGID      = flag.Uint("daemon-gid", 0, "Drop privileges to this group")
+	pagesRootCert   = flag.String("root-cert", "", "The default path to file certificate to serve static pages")
+	pagesRootKey    = flag.String("root-key", "", "The default path to file certificate to serve static pages")
+	redirectHTTP    = flag.Bool("redirect-http", false, "Redirect pages from HTTP to HTTPS")
+	useHTTP2        = flag.Bool("use-http2", true, "Enable HTTP2 support")
+	pagesRoot       = flag.String("pages-root", "shared/pages", "The directory where pages are stored")
+	pagesDomain     = flag.String("pages-domain", "gitlab-example.com", "The domain to serve static pages")
+	artifactsServer = flag.String("artifacts-server", "", "https://gitlab.com/api/v4")
+	pagesStatus     = flag.String("pages-status", "", "The url path for a status page, e.g., /@status")
+	metricsAddress  = flag.String("metrics-address", "", "The address to listen on for metrics requests")
+	daemonUID       = flag.Uint("daemon-uid", 0, "Drop privileges to this user")
+	daemonGID       = flag.Uint("daemon-gid", 0, "Drop privileges to this group")
 
 	disableCrossOriginRequests = flag.Bool("disable-cross-origin-requests", false, "Disable cross-origin requests")
 )
@@ -45,6 +47,11 @@ func configFromFlags() appConfig {
 		config.RootKey = readFile(*pagesRootKey)
 	}
 
+	u, err := url.Parse(strings.ToLower(*artifactsServer))
+	if err != nil {
+		log.Fatal(err)
+	}
+	config.ArtifactServer = u
 	return config
 }
 
-- 
GitLab


From 5d578f0bcab20936fc5b16c7d3e2e9a97b553855 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Wed, 30 Aug 2017 21:22:02 -0700
Subject: [PATCH 02/39] move 404.go to new file clled internal/httperrors.go,
 update the corresponding calles to previously implemented serve404 function
 accordingly. Refactor the httperrors.go accordingly to expand reusability for
 serving different error pages. #78

---
 app.go                           |  5 +--
 artifact.go                      |  6 ++--
 domain.go                        |  5 +--
 404.go => internal/httperrors.go | 55 ++++++++++++++++++++++++++------
 4 files changed, 56 insertions(+), 15 deletions(-)
 rename 404.go => internal/httperrors.go (78%)

diff --git a/app.go b/app.go
index 5fbfd39cc..fd4aca4a8 100644
--- a/app.go
+++ b/app.go
@@ -14,6 +14,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/rs/cors"
 
+	"gitlab.com/gitlab-org/gitlab-pages/internal"
 	"gitlab.com/gitlab-org/gitlab-pages/metrics"
 )
 
@@ -91,7 +92,7 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 		)
 		if err != nil {
 			log.Println("Building of Artifact failed", err)
-			serve404(&w) // TODO: Should we respond with 500 here?
+			internal.Serve404(&w) // TODO: Should we respond with 500 here?
 			return
 		}
 		artifact.MakeRequest(&w)
@@ -100,7 +101,7 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 
 	domain := a.domain(host)
 	if domain == nil {
-		serve404(&w)
+		internal.Serve404(&w)
 		return
 	}
 
diff --git a/artifact.go b/artifact.go
index e472660f5..1a24569d4 100644
--- a/artifact.go
+++ b/artifact.go
@@ -9,6 +9,8 @@ import (
 	"strconv"
 	"strings"
 	"time"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal"
 )
 
 const (
@@ -61,7 +63,7 @@ func (a *Artifact) MakeRequest(w http.ResponseWriter) {
 	resp, err := client.Get(a.URL.String())
 	if err != nil {
 		log.Println("Error while proxying artifact request", err)
-		serve404(w) // TODO: Should we respond with 500 here?
+		internal.Serve404(w) // TODO: Should we respond with 500 here?
 		return
 	}
 
@@ -70,7 +72,7 @@ func (a *Artifact) MakeRequest(w http.ResponseWriter) {
 	contentLength, err := buff.ReadFrom(resp.Body)
 	if err != nil {
 		log.Println("Error reading response from proxied request", err)
-		serve404(w) // TODO: Should we respond with 500 here?
+		internal.Serve404(w) // TODO: Should we respond with 500 here?
 		return
 	}
 
diff --git a/domain.go b/domain.go
index 45e59f946..56f67a172 100644
--- a/domain.go
+++ b/domain.go
@@ -13,6 +13,7 @@ import (
 	"strings"
 	"time"
 
+	"gitlab.com/gitlab-org/gitlab-pages/internal"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httputil"
 )
 
@@ -215,7 +216,7 @@ func (d *domain) serveFromGroup(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Serve generic not found
-	serve404(w)
+	internal.Serve404(w)
 }
 
 func (d *domain) serveFromConfig(w http.ResponseWriter, r *http.Request) {
@@ -230,7 +231,7 @@ func (d *domain) serveFromConfig(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Serve generic not found
-	serve404(w)
+	internal.Serve404(w)
 }
 
 func (d *domain) ensureCertificate() (*tls.Certificate, error) {
diff --git a/404.go b/internal/httperrors.go
similarity index 78%
rename from 404.go
rename to internal/httperrors.go
index 0ac0be540..a9c362546 100644
--- a/404.go
+++ b/internal/httperrors.go
@@ -1,15 +1,40 @@
-package main
+package internal
 
 import (
 	"fmt"
 	"net/http"
 )
 
-const predefined404 = `
+type errorContent struct {
+	status       int
+	title        string
+	statusString string
+	header       string
+	subHeader    string
+}
+
+var (
+	content404 = errorContent{
+		http.StatusNotFound,
+		"The page you're looking for could not be found (404)",
+		"404",
+		"The page you're looking for could not be found.",
+		"Make sure the address is correct and that the page hasn't moved.",
+	}
+	content500 = errorContent{
+		http.StatusInternalServerError,
+		"Internal server error (500)",
+		"500",
+		"Internal server error.",
+		"We encountered an error while processing your request, please try again.",
+	}
+)
+
+const predefinedErrorPage = `
 <!DOCTYPE html>
 <html>
   <head>
-    <title>The page you're looking for could not be found (404)</title>
+    <title>%v</title>
     <style>
       body {
         color: #666;
@@ -53,18 +78,30 @@ const predefined404 = `
   <body>
     <h1>
       <img src="data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiIHN0YW5kYWxvbmU9Im5vIj8+Cjxzdmcgd2lkdGg9IjIxMHB4IiBoZWlnaHQ9IjIxMHB4IiB2aWV3Qm94PSIwIDAgMjEwIDIxMCIgdmVyc2lvbj0iMS4xIiB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHhtbG5zOnhsaW5rPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5L3hsaW5rIiB4bWxuczpza2V0Y2g9Imh0dHA6Ly93d3cuYm9oZW1pYW5jb2RpbmcuY29tL3NrZXRjaC9ucyI+CiAgICA8IS0tIEdlbmVyYXRvcjogU2tldGNoIDMuMy4yICgxMjA0MykgLSBodHRwOi8vd3d3LmJvaGVtaWFuY29kaW5nLmNvbS9za2V0Y2ggLS0+CiAgICA8dGl0bGU+U2xpY2UgMTwvdGl0bGU+CiAgICA8ZGVzYz5DcmVhdGVkIHdpdGggU2tldGNoLjwvZGVzYz4KICAgIDxkZWZzPjwvZGVmcz4KICAgIDxnIGlkPSJQYWdlLTEiIHN0cm9rZT0ibm9uZSIgc3Ryb2tlLXdpZHRoPSIxIiBmaWxsPSJub25lIiBmaWxsLXJ1bGU9ImV2ZW5vZGQiIHNrZXRjaDp0eXBlPSJNU1BhZ2UiPgogICAgICAgIDxnIGlkPSJsb2dvIiBza2V0Y2g6dHlwZT0iTVNMYXllckdyb3VwIiB0cmFuc2Zvcm09InRyYW5zbGF0ZSgwLjAwMDAwMCwgMTAuMDAwMDAwKSI+CiAgICAgICAgICAgIDxnIGlkPSJQYWdlLTEiIHNrZXRjaDp0eXBlPSJNU1NoYXBlR3JvdXAiPgogICAgICAgICAgICAgICAgPGcgaWQ9IkZpbGwtMS0rLUdyb3VwLTI0Ij4KICAgICAgICAgICAgICAgICAgICA8ZyBpZD0iR3JvdXAtMjQiPgogICAgICAgICAgICAgICAgICAgICAgICA8ZyBpZD0iR3JvdXAiPgogICAgICAgICAgICAgICAgICAgICAgICAgICAgPHBhdGggZD0iTTEwNS4wNjE0LDE5My42NTUgTDEwNS4wNjE0LDE5My42NTUgTDE0My43MDE0LDc0LjczNCBMNjYuNDIxNCw3NC43MzQgTDEwNS4wNjE0LDE5My42NTUgTDEwNS4wNjE0LDE5My42NTUgWiIgaWQ9IkZpbGwtNCIgZmlsbD0iI0UyNDMyOSI+PC9wYXRoPgogICAgICAgICAgICAgICAgICAgICAgICAgICAgPHBhdGggZD0iTTEwNS4wNjE0LDE5My42NTQ4IEw2Ni40MjE0LDc0LjczMzggTDEyLjI2ODQsNzQuNzMzOCBMMTA1LjA2MTQsMTkzLjY1NDggTDEwNS4wNjE0LDE5My42NTQ4IFoiIGlkPSJGaWxsLTgiIGZpbGw9IiNGQzZEMjYiPjwvcGF0aD4KICAgICAgICAgICAgICAgICAgICAgICAgICAgIDxwYXRoIGQ9Ik0xMi4yNjg1LDc0LjczNDEgTDEyLjI2ODUsNzQuNzM0MSBMMC41MjY1LDExMC44NzMxIEMtMC41NDQ1LDExNC4xNjkxIDAuNjI4NSwxMTcuNzgwMSAzLjQzMjUsMTE5LjgxNzEgTDEwNS4wNjE1LDE5My42NTUxIEwxMi4yNjg1LDc0LjczNDEgTDEyLjI2ODUsNzQuNzM0MSBaIiBpZD0iRmlsbC0xMiIgZmlsbD0iI0ZDQTMyNiI+PC9wYXRoPgogICAgICAgICAgICAgICAgICAgICAgICAgICAgPHBhdGggZD0iTTEyLjI2ODUsNzQuNzM0MiBMNjYuNDIxNSw3NC43MzQyIEw0My4xNDg1LDMuMTA5MiBDNDEuOTUxNSwtMC41NzY4IDM2LjczNzUsLTAuNTc1OCAzNS41NDA1LDMuMTA5MiBMMTIuMjY4NSw3NC43MzQyIEwxMi4yNjg1LDc0LjczNDIgWiIgaWQ9IkZpbGwtMTYiIGZpbGw9IiNFMjQzMjkiPjwvcGF0aD4KICAgICAgICAgICAgICAgICAgICAgICAgICAgIDxwYXRoIGQ9Ik0xMDUuMDYxNCwxOTMuNjU0OCBMMTQzLjcwMTQsNzQuNzMzOCBMMTk3Ljg1NDQsNzQuNzMzOCBMMTA1LjA2MTQsMTkzLjY1NDggTDEwNS4wNjE0LDE5My42NTQ4IFoiIGlkPSJGaWxsLTE4IiBmaWxsPSIjRkM2RDI2Ij48L3BhdGg+CiAgICAgICAgICAgICAgICAgICAgICAgICAgICA8cGF0aCBkPSJNMTk3Ljg1NDQsNzQuNzM0MSBMMTk3Ljg1NDQsNzQuNzM0MSBMMjA5LjU5NjQsMTEwLjg3MzEgQzIxMC42Njc0LDExNC4xNjkxIDIwOS40OTQ0LDExNy43ODAxIDIwNi42OTA0LDExOS44MTcxIEwxMDUuMDYxNCwxOTMuNjU1MSBMMTk3Ljg1NDQsNzQuNzM0MSBMMTk3Ljg1NDQsNzQuNzM0MSBaIiBpZD0iRmlsbC0yMCIgZmlsbD0iI0ZDQTMyNiI+PC9wYXRoPgogICAgICAgICAgICAgICAgICAgICAgICAgICAgPHBhdGggZD0iTTE5Ny44NTQ0LDc0LjczNDIgTDE0My43MDE0LDc0LjczNDIgTDE2Ni45NzQ0LDMuMTA5MiBDMTY4LjE3MTQsLTAuNTc2OCAxNzMuMzg1NCwtMC41NzU4IDE3NC41ODI0LDMuMTA5MiBMMTk3Ljg1NDQsNzQuNzM0MiBMMTk3Ljg1NDQsNzQuNzM0MiBaIiBpZD0iRmlsbC0yMiIgZmlsbD0iI0UyNDMyOSI+PC9wYXRoPgogICAgICAgICAgICAgICAgICAgICAgICA8L2c+CiAgICAgICAgICAgICAgICAgICAgPC9nPgogICAgICAgICAgICAgICAgPC9nPgogICAgICAgICAgICA8L2c+CiAgICAgICAgPC9nPgogICAgPC9nPgo8L3N2Zz4=" /><br />
-      404
+      %v
     </h1>
-    <h3>The page you're looking for could not be found.</h3>
+    <h3>%v</h3>
     <hr/>
-    <p>Make sure the address is correct and that the page hasn't moved.</p>
+    <p>%v</p>
   </body>
 </html>
 `
 
-func serve404(w http.ResponseWriter) {
+func generateErrorHTML(e errorContent) string {
+	return fmt.Sprintf(predefinedErrorPage, e.title, e.statusString, e.header, e.subHeader)
+}
+
+func serveErrorPage(w http.ResponseWriter, e errorContent) {
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
 	w.Header().Set("X-Content-Type-Options", "nosniff")
-	w.WriteHeader(http.StatusNotFound)
-	fmt.Fprintln(w, predefined404)
+	w.WriteHeader(e.status)
+	fmt.Fprintln(w, generateErrorHTML(e))
+}
+
+func Serve404(w http.ResponseWriter) {
+	serveErrorPage(w, content404)
+}
+
+func Serve500(w http.ResponseWriter) {
+	serveErrorPage(w, content500)
 }
-- 
GitLab


From bf09069f382b8daf415760a8faa777fa904fd6bb Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Wed, 30 Aug 2017 21:33:34 -0700
Subject: [PATCH 03/39] extract artifact.go into its own package called
 internal/artifact. Adjust the app.go file within main as it had calls to
 functionality that are now located within the newly created artifcat package.
 Build passing #78

---
 app.go                                                 | 8 ++++----
 artifact.go => internal/artifact/artifact.go           | 7 ++++---
 artifact_test.go => internal/artifact/artifact_test.go | 4 ++--
 3 files changed, 10 insertions(+), 9 deletions(-)
 rename artifact.go => internal/artifact/artifact.go (94%)
 rename artifact_test.go => internal/artifact/artifact_test.go (98%)

diff --git a/app.go b/app.go
index fd4aca4a8..5571c46f5 100644
--- a/app.go
+++ b/app.go
@@ -15,12 +15,12 @@ import (
 	"github.com/rs/cors"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
 	"gitlab.com/gitlab-org/gitlab-pages/metrics"
 )
 
 const xForwardedProto = "X-Forwarded-Proto"
 const xForwardedProtoHTTPS = "https"
-const ArtifactPrefix = "artifact~"
 
 var (
 	corsHandler = cors.New(cors.Options{AllowedMethods: []string{"GET"}})
@@ -84,10 +84,10 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 
 	// In the event a host is prefixed with the artifact prefix an artifact
 	// value is created, and an attempt to proxy the request is made
-	if strings.HasPrefix(host, ArtifactPrefix) {
-		artifact, err := NewArtifact(
+	if strings.HasPrefix(host, artifact.ArtifactPrefix) {
+		artifact, err := artifact.NewArtifact(
 			a.appConfig.ArtifactServer.String(),
-			artifactURLBody(host),
+			artifact.ArtifactURLBody(host),
 			r.URL.Path,
 		)
 		if err != nil {
diff --git a/artifact.go b/internal/artifact/artifact.go
similarity index 94%
rename from artifact.go
rename to internal/artifact/artifact.go
index 1a24569d4..1b5ffc9ad 100644
--- a/artifact.go
+++ b/internal/artifact/artifact.go
@@ -1,4 +1,4 @@
-package main
+package artifact
 
 import (
 	"bytes"
@@ -14,6 +14,7 @@ import (
 )
 
 const (
+	ArtifactPrefix   = "artifact~"
 	artifactBodyBase = "/projects/%v/artifacts/%v/raw"
 )
 
@@ -82,11 +83,11 @@ func (a *Artifact) MakeRequest(w http.ResponseWriter) {
 	w.Write(buff.Bytes())
 }
 
-// func artifactURLBody returns a string that represents the body of a url an
+// func ArtifactURLBody returns a string that represents the body of a url an
 // artifact will ultimately proxy a request to. An expected host argument would
 // be close to "artifact~1~2". 1 and 2 represent Id's that need to be extracted
 // and interpolated into the const artifactBodyBase.
-func artifactURLBody(host string) string {
+func ArtifactURLBody(host string) string {
 	ids := strings.Split(strings.TrimPrefix(host, ArtifactPrefix), "~")
 	return fmt.Sprintf(artifactBodyBase, ids[0], ids[1])
 }
diff --git a/artifact_test.go b/internal/artifact/artifact_test.go
similarity index 98%
rename from artifact_test.go
rename to internal/artifact/artifact_test.go
index dd41a90e6..dec884871 100644
--- a/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -1,4 +1,4 @@
-package main
+package artifact
 
 import (
 	"fmt"
@@ -122,7 +122,7 @@ func TestArtifactURLBody(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		actual := artifactURLBody(c.UrlHost)
+		actual := ArtifactURLBody(c.UrlHost)
 		assert.Equal(t, actual, c.Expected, c.Description)
 	}
 }
-- 
GitLab


From e9c41b660a6ed74b2a8c09e1e9126b83dea75cd0 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Wed, 30 Aug 2017 23:10:59 -0700
Subject: [PATCH 04/39] refactoring of artifact package and its usage
 throughout the rest of the program. Removal of artifact type, and make
 artifact function. Adjustments to tests accordingly. Add commenting to
 internal/httperrors.go. #78

---
 app.go                             | 15 ++++----
 internal/artifact/artifact.go      | 53 +++++++---------------------
 internal/artifact/artifact_test.go | 56 +++++-------------------------
 internal/httperrors.go             |  2 ++
 4 files changed, 30 insertions(+), 96 deletions(-)

diff --git a/app.go b/app.go
index 5571c46f5..c86a9ab40 100644
--- a/app.go
+++ b/app.go
@@ -2,9 +2,11 @@ package main
 
 import (
 	"crypto/tls"
+	"fmt"
 	"log"
 	"net"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 	"sync"
@@ -85,17 +87,14 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 	// In the event a host is prefixed with the artifact prefix an artifact
 	// value is created, and an attempt to proxy the request is made
 	if strings.HasPrefix(host, artifact.ArtifactPrefix) {
-		artifact, err := artifact.NewArtifact(
-			a.appConfig.ArtifactServer.String(),
-			artifact.ArtifactURLBody(host),
-			r.URL.Path,
-		)
+		urlString := fmt.Sprintf("%v%v%v", a.ArtifactServer.String(), artifact.URLBody(host), r.URL.Path)
+		artifactURL, err := url.Parse(urlString)
 		if err != nil {
-			log.Println("Building of Artifact failed", err)
-			internal.Serve404(&w) // TODO: Should we respond with 500 here?
+			log.Println("Building of Artifact URL failed", err)
+			internal.Serve500(&w)
 			return
 		}
-		artifact.MakeRequest(&w)
+		artifact.MakeRequest(&w, artifactURL)
 		return
 	}
 
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 1b5ffc9ad..b1cdd2266 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -18,53 +18,24 @@ const (
 	artifactBodyBase = "/projects/%v/artifacts/%v/raw"
 )
 
-// Artifact is used for the composition of new URL's. In the event this program
-// is running on a host that is prefixed with artifact~ we attempt to proxy
-// requests  to a new URL that is composed of the Host, the artifacts-server
-// flag value, and the path of the request. This struct and functions surrounding
-// it provide support and functionality for these cases.
-type Artifact struct {
-	Prefix string
-	Body   string
-	Suffix string
-	*url.URL
-}
-
-// func NewArtifact is used as a builder to produce a pointer to a new Artifact.
-// Errors returned are related to the building of an underlying url.URL pointer
-// that is constructed from the arguments defined herein.
-func NewArtifact(prefix, body, suffix string) (*Artifact, error) {
-	urlString := fmt.Sprintf("%v%v%v", prefix, body, suffix)
-	u, err := url.Parse(urlString)
-	if err != nil {
-		return &Artifact{}, err
+var (
+	artifactClient = &http.Client{
+		Timeout: time.Second * 10,
 	}
+)
 
-	return &Artifact{
-		Prefix: prefix,
-		Body:   body,
-		Suffix: suffix,
-		URL:    u,
-	}, nil
-}
-
-// func MakeRequest given an http.ResponseWriter will proxy a request to the
+// MakeRequest given an http.ResponseWriter will proxy a request to the
 // underlying url.URL pointer's address on the related Artifact pointer. Making
 // a GET request, it then maps the Content-Type, Content-Length, and response
 // body from said GET request to the http.ResponseWriter. A 404 will be
 // served in the event that an error is encountered.
-func (a *Artifact) MakeRequest(w http.ResponseWriter) {
-	//custom http.Client{} is needed to specify request timeout
-	var client = &http.Client{
-		Timeout: time.Second * 10,
-	}
-
-	log.Println("Attempting to proxy request to", a.URL.String())
+func MakeRequest(w http.ResponseWriter, u *url.URL) {
+	log.Println("Attempting to proxy request to", u.String())
 
-	resp, err := client.Get(a.URL.String())
+	resp, err := artifactClient.Get(u.String())
 	if err != nil {
 		log.Println("Error while proxying artifact request", err)
-		internal.Serve404(w) // TODO: Should we respond with 500 here?
+		internal.Serve500(w)
 		return
 	}
 
@@ -73,7 +44,7 @@ func (a *Artifact) MakeRequest(w http.ResponseWriter) {
 	contentLength, err := buff.ReadFrom(resp.Body)
 	if err != nil {
 		log.Println("Error reading response from proxied request", err)
-		internal.Serve404(w) // TODO: Should we respond with 500 here?
+		internal.Serve500(w)
 		return
 	}
 
@@ -83,11 +54,11 @@ func (a *Artifact) MakeRequest(w http.ResponseWriter) {
 	w.Write(buff.Bytes())
 }
 
-// func ArtifactURLBody returns a string that represents the body of a url an
+// URLBody returns a string that represents the body of a url an
 // artifact will ultimately proxy a request to. An expected host argument would
 // be close to "artifact~1~2". 1 and 2 represent Id's that need to be extracted
 // and interpolated into the const artifactBodyBase.
-func ArtifactURLBody(host string) string {
+func URLBody(host string) string {
 	ids := strings.Split(strings.TrimPrefix(host, ArtifactPrefix), "~")
 	return fmt.Sprintf(artifactBodyBase, ids[0], ids[1])
 }
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index dec884871..554c9ca0a 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -10,20 +10,17 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestArtifactMakeRequest(t *testing.T) {
+func TestMakeRequest(t *testing.T) {
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "text/html; charset=utf-8")
 		fmt.Fprintln(w, "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
 	}))
 	defer testServer.Close()
-	artifactUrl, err := url.Parse(testServer.URL)
+	artifactURL, err := url.Parse(testServer.URL)
 	assert.NoError(t, err)
-	artifact := &Artifact{
-		URL: artifactUrl,
-	}
 
 	result := httptest.NewRecorder()
-	artifact.MakeRequest(result)
+	MakeRequest(result, artifactURL)
 
 	assert.Equal(t, result.Header().Get("Content-Type"), "text/html; charset=utf-8")
 	assert.Equal(t, result.Header().Get("Content-Length"), "91")
@@ -34,53 +31,18 @@ func TestArtifactMakeRequest(t *testing.T) {
 func TestArtifactMakeRequest_bad_request(t *testing.T) {
 	testServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
 	defer testServer.Close()
-	artifactUrl, err := url.Parse(testServer.URL)
+	artifactURL, err := url.Parse(testServer.URL)
 	assert.NoError(t, err)
-	artifact := &Artifact{
-		URL: artifactUrl,
-	}
 
 	result := httptest.NewRecorder()
-	artifact.MakeRequest(result)
+	MakeRequest(result, artifactURL)
 
-	assert.Equal(t, 404, result.Code)
-}
-
-func TestNewArtifact(t *testing.T) {
-	cases := []struct {
-		Prefix      string
-		Body        string
-		Suffix      string
-		Expected    string
-		Description string
-	}{
-		{
-			"https://gitlab.com/api/v4",
-			"/projects/1/artifacts/2/raw",
-			"/index.html",
-			"https://gitlab.com/api/v4/projects/1/artifacts/2/raw/index.html",
-			"Basic case",
-		},
-		{
-			"http://some.otherhost.com/api/33",
-			"/projects/1/artifacts/2/raw",
-			"/file.txt",
-			"http://some.otherhost.com/api/33/projects/1/artifacts/2/raw/file.txt",
-			"Additional test wtih different domains",
-		},
-	}
-
-	for _, c := range cases {
-		a, err := NewArtifact(c.Prefix, c.Body, c.Suffix)
-		assert.NoError(t, err)
-		actual := a.URL.String()
-		assert.Equal(t, actual, c.Expected, c.Description)
-	}
+	assert.Equal(t, 500, result.Code)
 }
 
-func TestArtifactURLBody(t *testing.T) {
+func TestURLBody(t *testing.T) {
 	cases := []struct {
-		UrlHost     string
+		URLHost     string
 		Expected    string
 		Description string
 	}{
@@ -122,7 +84,7 @@ func TestArtifactURLBody(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		actual := ArtifactURLBody(c.UrlHost)
+		actual := URLBody(c.URLHost)
 		assert.Equal(t, actual, c.Expected, c.Description)
 	}
 }
diff --git a/internal/httperrors.go b/internal/httperrors.go
index a9c362546..b2a541dd0 100644
--- a/internal/httperrors.go
+++ b/internal/httperrors.go
@@ -98,10 +98,12 @@ func serveErrorPage(w http.ResponseWriter, e errorContent) {
 	fmt.Fprintln(w, generateErrorHTML(e))
 }
 
+// Serve404 returns a 404 error response / HTML page to the http.ResponseWriter
 func Serve404(w http.ResponseWriter) {
 	serveErrorPage(w, content404)
 }
 
+// Serve500 returns a 500 error response / HTML page to the http.ResponseWriter
 func Serve500(w http.ResponseWriter) {
 	serveErrorPage(w, content500)
 }
-- 
GitLab


From 77d6a554772fb7d51f766198abee6f98dede324a Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Thu, 31 Aug 2017 19:58:12 -0700
Subject: [PATCH 05/39] adjustments within app.go with the creation of a
 shouldproxy mthod on theApp struct, along with intergration of said method
 into the serveContent method to be a more ridged check to proxy when
 conditions are correct. Create unit tests wtihin newly create app_test.go
 file accordingly. Create unit tests for all fundtions within the
 internal/httperrors.go file within newly created intern/httperrors_test.go
 file. Refactoring of URLBody function within artifact package along with some
 changes to testing, it now uses regex for matching. Also create MatchHost
 function as well. #78

---
 app.go                             |  6 +-
 app_test.go                        | 51 ++++++++++++++++
 internal/artifact/artifact.go      | 26 +++++++--
 internal/artifact/artifact_test.go | 72 +++++++++++++++++++++--
 internal/httperrors_test.go        | 93 ++++++++++++++++++++++++++++++
 5 files changed, 237 insertions(+), 11 deletions(-)
 create mode 100644 app_test.go
 create mode 100644 internal/httperrors_test.go

diff --git a/app.go b/app.go
index c86a9ab40..77aa40e49 100644
--- a/app.go
+++ b/app.go
@@ -55,6 +55,10 @@ func (a *theApp) ServeTLS(ch *tls.ClientHelloInfo) (*tls.Certificate, error) {
 	return nil, nil
 }
 
+func (a *theApp) shouldProxy(host string) bool {
+	return (a.ArtifactServer.String() != "") && artifact.MatchHost(host)
+}
+
 func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https bool) {
 	w := newLoggingResponseWriter(ww)
 	defer w.Log(r)
@@ -86,7 +90,7 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 
 	// In the event a host is prefixed with the artifact prefix an artifact
 	// value is created, and an attempt to proxy the request is made
-	if strings.HasPrefix(host, artifact.ArtifactPrefix) {
+	if a.shouldProxy(host) {
 		urlString := fmt.Sprintf("%v%v%v", a.ArtifactServer.String(), artifact.URLBody(host), r.URL.Path)
 		artifactURL, err := url.Parse(urlString)
 		if err != nil {
diff --git a/app_test.go b/app_test.go
new file mode 100644
index 000000000..0932276a0
--- /dev/null
+++ b/app_test.go
@@ -0,0 +1,51 @@
+package main
+
+import (
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestShouldProxy(t *testing.T) {
+	cases := []struct {
+		ArtifactServer string
+		Host           string
+		Expected       bool
+		Description    string
+	}{
+		{
+			"https://gitlab.com/api/v4",
+			"artifact~1~2",
+			true,
+			"basic case",
+		},
+		{
+			"",
+			"artifact~1~2",
+			false,
+			"Artifact server is not provided",
+		},
+		{
+			"https://gitlab.com/api/v4",
+			"artifact~1~2~3~4",
+			false,
+			"host does not match the expected pattern",
+		},
+		{
+			"",
+			"artifact~1~2~3~4",
+			false,
+			"host does not match the expected pattern, and there is not an artifact server provided",
+		},
+	}
+
+	for _, c := range cases {
+		URL, err := url.Parse(c.ArtifactServer)
+		assert.NoError(t, err)
+
+		app := &theApp{appConfig: appConfig{ArtifactServer: URL}}
+		actual := app.shouldProxy(c.Host)
+		assert.Equal(t, actual, c.Expected, c.Description)
+	}
+}
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index b1cdd2266..c353b6693 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -6,22 +6,23 @@ import (
 	"log"
 	"net/http"
 	"net/url"
+	"regexp"
 	"strconv"
-	"strings"
 	"time"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal"
 )
 
 const (
-	ArtifactPrefix   = "artifact~"
-	artifactBodyBase = "/projects/%v/artifacts/%v/raw"
+	baseURL = "/projects/%v/artifacts/%v/raw"
 )
 
 var (
 	artifactClient = &http.Client{
 		Timeout: time.Second * 10,
 	}
+	hostPattern = regexp.MustCompile("(?i)\\Aartifact~(\\d+)~(\\d+)\\z")
+	idsPattern  = regexp.MustCompile(`~(\d+)`)
 )
 
 // MakeRequest given an http.ResponseWriter will proxy a request to the
@@ -57,8 +58,21 @@ func MakeRequest(w http.ResponseWriter, u *url.URL) {
 // URLBody returns a string that represents the body of a url an
 // artifact will ultimately proxy a request to. An expected host argument would
 // be close to "artifact~1~2". 1 and 2 represent Id's that need to be extracted
-// and interpolated into the const artifactBodyBase.
+// and interpolated into the const baseURL.
 func URLBody(host string) string {
-	ids := strings.Split(strings.TrimPrefix(host, ArtifactPrefix), "~")
-	return fmt.Sprintf(artifactBodyBase, ids[0], ids[1])
+	ids := idsPattern.FindAllStringSubmatch(host, 2)
+	strippedIds := make([]string, 2)
+
+	for i, id := range ids {
+		strippedIds[i] = id[1]
+	}
+
+	return fmt.Sprintf(baseURL, strippedIds[0], strippedIds[1])
+}
+
+// MatchHost when provided a string checks to see if it matches the predefined
+// host pattern, returning a bool. Matching patters should look like the
+// following: "artifact~1~2"
+func MatchHost(s string) bool {
+	return hostPattern.MatchString(s)
 }
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index 554c9ca0a..cfff97b48 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -63,22 +63,22 @@ func TestURLBody(t *testing.T) {
 		},
 		{
 			"artifact~one~two",
-			"/projects/one/artifacts/two/raw",
+			"/projects//artifacts//raw",
 			"letters rather than numbers",
 		},
 		{
 			"artifact~One111~tWo222",
-			"/projects/One111/artifacts/tWo222/raw",
+			"/projects//artifacts//raw",
 			"Mixture of alphanumeric",
 		},
 		{
 			"artifact~!@#$%~%$#@!",
-			"/projects/!@#$%/artifacts/%$#@!/raw",
+			"/projects//artifacts//raw",
 			"special characters",
 		},
 		{
 			"artifact~~",
-			"/projects//artifacts//raw", //@WIP how should this be handled, is it something to worry about?
+			"/projects//artifacts//raw",
 			"no ids",
 		},
 	}
@@ -88,3 +88,67 @@ func TestURLBody(t *testing.T) {
 		assert.Equal(t, actual, c.Expected, c.Description)
 	}
 }
+
+func TestMatchHost(t *testing.T) {
+	cases := []struct {
+		URLHost     string
+		Expected    bool
+		Description string
+	}{
+		{
+			"artifact~1~2",
+			true,
+			"basic case",
+		},
+		{
+			"ARTIFACT~1~2",
+			true,
+			"capital letters case",
+		},
+		{
+			"ARTIFACT~11234~2908908",
+			true,
+			"additional capital letters case",
+		},
+		{
+			"artifact~10000~20000",
+			true,
+			"expanded case",
+		},
+		{
+			"artifact~86753095555~55550935768",
+			true,
+			"large number case",
+		},
+		{
+			"artifact~one~two",
+			false,
+			"letters rather than numbers",
+		},
+		{
+			"artifact~One111~tWo222",
+			false,
+			"Mixture of alphanumeric",
+		},
+		{
+			"artifact~!@#$%~%$#@!",
+			false,
+			"special characters",
+		},
+		{
+			"artifact~1~2~3",
+			false,
+			"too many ids",
+		},
+		{
+			"artifact~1",
+			false,
+			"not enough ids",
+		},
+	}
+
+	for _, c := range cases {
+		actual := MatchHost(c.URLHost)
+		assert.Equal(t, actual, c.Expected, c.Description)
+	}
+}
diff --git a/internal/httperrors_test.go b/internal/httperrors_test.go
new file mode 100644
index 000000000..79610cf8d
--- /dev/null
+++ b/internal/httperrors_test.go
@@ -0,0 +1,93 @@
+package internal
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// creates a new implementation of htt.ResponseWriter that allows the
+// casting of values in order to aid testing efforts.
+type testResponseWriter struct {
+	status  int
+	content string
+	http.ResponseWriter
+}
+
+func newTestResponseWriter(w http.ResponseWriter) *testResponseWriter {
+	return &testResponseWriter{0, "", w}
+}
+
+func (w *testResponseWriter) Status() int {
+	return w.status
+}
+
+func (w *testResponseWriter) Content() string {
+	return w.content
+}
+
+func (w *testResponseWriter) Header() http.Header {
+	return w.ResponseWriter.Header()
+}
+
+func (w *testResponseWriter) Write(data []byte) (int, error) {
+	w.content = string(data)
+	return w.ResponseWriter.Write(data)
+}
+
+func (w *testResponseWriter) WriteHeader(statusCode int) {
+	w.status = statusCode
+	w.ResponseWriter.WriteHeader(statusCode)
+}
+
+var (
+	testingContent = errorContent{
+		http.StatusNotFound,
+		"Title",
+		"533",
+		"Header test",
+		"subheader text",
+	}
+)
+
+func TestGenerateemailHTML(t *testing.T) {
+	actual := generateErrorHTML(testingContent)
+	assert.Contains(t, actual, testingContent.title)
+	assert.Contains(t, actual, testingContent.statusString)
+	assert.Contains(t, actual, testingContent.header)
+	assert.Contains(t, actual, testingContent.subHeader)
+}
+
+func TestServeErrorPage(t *testing.T) {
+	w := newTestResponseWriter(httptest.NewRecorder())
+	serveErrorPage(w, testingContent)
+	assert.Equal(t, w.Header().Get("Content-Type"), "text/html; charset=utf-8")
+	assert.Equal(t, w.Header().Get("X-Content-Type-Options"), "nosniff")
+	assert.Equal(t, w.Status(), testingContent.status)
+}
+
+func TestServe404(t *testing.T) {
+	w := newTestResponseWriter(httptest.NewRecorder())
+	Serve404(w)
+	assert.Equal(t, w.Header().Get("Content-Type"), "text/html; charset=utf-8")
+	assert.Equal(t, w.Header().Get("X-Content-Type-Options"), "nosniff")
+	assert.Equal(t, w.Status(), content404.status)
+	assert.Contains(t, w.Content(), content404.title)
+	assert.Contains(t, w.Content(), content404.statusString)
+	assert.Contains(t, w.Content(), content404.header)
+	assert.Contains(t, w.Content(), content404.subHeader)
+}
+
+func TestServe500(t *testing.T) {
+	w := newTestResponseWriter(httptest.NewRecorder())
+	Serve500(w)
+	assert.Equal(t, w.Header().Get("Content-Type"), "text/html; charset=utf-8")
+	assert.Equal(t, w.Header().Get("X-Content-Type-Options"), "nosniff")
+	assert.Equal(t, w.Status(), content500.status)
+	assert.Contains(t, w.Content(), content500.title)
+	assert.Contains(t, w.Content(), content500.statusString)
+	assert.Contains(t, w.Content(), content500.header)
+	assert.Contains(t, w.Content(), content500.subHeader)
+}
-- 
GitLab


From d9312a6eb6881a9f31318432f5e8c95ad783afae Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Thu, 31 Aug 2017 20:17:12 -0700
Subject: [PATCH 06/39] change implementation of the the makeRequset function
 within the MakeRequest function in the artifact package to utilize io.Copy
 rather than initizizing a buffer of bytes. #78

---
 internal/artifact/artifact.go | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index c353b6693..b17675e37 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -1,8 +1,8 @@
 package artifact
 
 import (
-	"bytes"
 	"fmt"
+	"io"
 	"log"
 	"net/http"
 	"net/url"
@@ -40,9 +40,7 @@ func MakeRequest(w http.ResponseWriter, u *url.URL) {
 		return
 	}
 
-	defer resp.Body.Close()
-	buff := new(bytes.Buffer)
-	contentLength, err := buff.ReadFrom(resp.Body)
+	contentLength, err := io.Copy(w, resp.Body)
 	if err != nil {
 		log.Println("Error reading response from proxied request", err)
 		internal.Serve500(w)
@@ -52,7 +50,6 @@ func MakeRequest(w http.ResponseWriter, u *url.URL) {
 	w.Header().Set("Content-Type", resp.Header.Get("Content-Type"))
 	w.Header().Set("Content-Length", strconv.FormatInt(contentLength, 10))
 	w.Header().Set("Cache-Control", "max-age=600")
-	w.Write(buff.Bytes())
 }
 
 // URLBody returns a string that represents the body of a url an
-- 
GitLab


From db873daeb28e2b7a00a7e8c292953e652d76cb73 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Thu, 31 Aug 2017 20:49:33 -0700
Subject: [PATCH 07/39] adjustment to main.go file wihtin the configFromFlags
 function, added logic that ensures that when n artifacts-server flag is set
 its scheme is either http or https. Created vars for the related regex and
 custom error to alert the user. #78

---
 internal/artifact/artifact.go |  8 +++-----
 main.go                       | 11 ++++++++++-
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index b17675e37..bd0117286 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -18,11 +18,9 @@ const (
 )
 
 var (
-	artifactClient = &http.Client{
-		Timeout: time.Second * 10,
-	}
-	hostPattern = regexp.MustCompile("(?i)\\Aartifact~(\\d+)~(\\d+)\\z")
-	idsPattern  = regexp.MustCompile(`~(\d+)`)
+	artifactClient = &http.Client{Timeout: time.Second * 10}
+	hostPattern    = regexp.MustCompile(`(?i)\Aartifact~(\d+)~(\d+)\z`)
+	idsPattern     = regexp.MustCompile(`~(\d+)`)
 )
 
 // MakeRequest given an http.ResponseWriter will proxy a request to the
diff --git a/main.go b/main.go
index 92d7dcc44..048269395 100644
--- a/main.go
+++ b/main.go
@@ -1,10 +1,12 @@
 package main
 
 import (
+	"errors"
 	"flag"
 	"log"
 	"net/url"
 	"os"
+	"regexp"
 	"strings"
 )
 
@@ -28,6 +30,8 @@ var (
 	daemonGID       = flag.Uint("daemon-gid", 0, "Drop privileges to this group")
 
 	disableCrossOriginRequests = flag.Bool("disable-cross-origin-requests", false, "Disable cross-origin requests")
+	artifactSchemaUnsupported  = errors.New("artifacts-server schema must be either http:// or https://")
+	approvedArtifactScheme     = regexp.MustCompile(`(\Ahttp\z)|(\Ahttps\z)`)
 )
 
 func configFromFlags() appConfig {
@@ -47,10 +51,15 @@ func configFromFlags() appConfig {
 		config.RootKey = readFile(*pagesRootKey)
 	}
 
-	u, err := url.Parse(strings.ToLower(*artifactsServer))
+	u, err := url.Parse(*artifactsServer)
 	if err != nil {
 		log.Fatal(err)
 	}
+
+	if u.Scheme != "" && !approvedArtifactScheme.MatchString(u.Scheme) {
+		log.Fatal(artifactSchemaUnsupported)
+	}
+
 	config.ArtifactServer = u
 	return config
 }
-- 
GitLab


From 2f2b95850e4576c5fe7f0343999fb1d4f2e5358e Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Thu, 31 Aug 2017 23:51:23 -0700
Subject: [PATCH 08/39] add proxy request to acceptance_test.go made changes
 within main.go according to golint #78

---
 acceptance_test.go | 21 +++++++++++++++++++++
 main.go            |  8 ++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index 8adef2a61..d8f9c1755 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -2,8 +2,10 @@ package main
 
 import (
 	"flag"
+	"fmt"
 	"io/ioutil"
 	"net/http"
+	"net/http/httptest"
 	"os"
 	"testing"
 
@@ -207,3 +209,22 @@ func TestStatusPage(t *testing.T) {
 	defer rsp.Body.Close()
 	assert.Equal(t, http.StatusOK, rsp.StatusCode)
 }
+
+func TestProxyRequest(t *testing.T) {
+	content := "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>"
+	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/html; charset=utf-8")
+		fmt.Fprintln(w, content)
+	}))
+	defer testServer.Close()
+	skipUnlessEnabled(t)
+	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL)
+	defer teardown()
+
+	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2", "index.html")
+	assert.NoError(t, err)
+	defer resp.Body.Close()
+	body, _ := ioutil.ReadAll(resp.Body)
+	assert.Contains(t, string(body), content)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+}
diff --git a/main.go b/main.go
index 048269395..a2ad38f0d 100644
--- a/main.go
+++ b/main.go
@@ -29,9 +29,9 @@ var (
 	daemonUID       = flag.Uint("daemon-uid", 0, "Drop privileges to this user")
 	daemonGID       = flag.Uint("daemon-gid", 0, "Drop privileges to this group")
 
-	disableCrossOriginRequests = flag.Bool("disable-cross-origin-requests", false, "Disable cross-origin requests")
-	artifactSchemaUnsupported  = errors.New("artifacts-server schema must be either http:// or https://")
-	approvedArtifactScheme     = regexp.MustCompile(`(\Ahttp\z)|(\Ahttps\z)`)
+	disableCrossOriginRequests   = flag.Bool("disable-cross-origin-requests", false, "Disable cross-origin requests")
+	errArtifactSchemaUnsupported = errors.New("artifacts-server schema must be either http:// or https://")
+	approvedArtifactScheme       = regexp.MustCompile(`(\Ahttp\z)|(\Ahttps\z)`)
 )
 
 func configFromFlags() appConfig {
@@ -57,7 +57,7 @@ func configFromFlags() appConfig {
 	}
 
 	if u.Scheme != "" && !approvedArtifactScheme.MatchString(u.Scheme) {
-		log.Fatal(artifactSchemaUnsupported)
+		log.Fatal(errArtifactSchemaUnsupported)
 	}
 
 	config.ArtifactServer = u
-- 
GitLab


From c76c76ccca6f42866e469321fa3dc7b591daa321 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Thu, 31 Aug 2017 23:53:57 -0700
Subject: [PATCH 09/39] change commenting with artifact/artifact.go #78

---
 internal/artifact/artifact.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index bd0117286..b52360832 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -26,7 +26,7 @@ var (
 // MakeRequest given an http.ResponseWriter will proxy a request to the
 // underlying url.URL pointer's address on the related Artifact pointer. Making
 // a GET request, it then maps the Content-Type, Content-Length, and response
-// body from said GET request to the http.ResponseWriter. A 404 will be
+// body from said GET request to the http.ResponseWriter. A 500 will be
 // served in the event that an error is encountered.
 func MakeRequest(w http.ResponseWriter, u *url.URL) {
 	log.Println("Attempting to proxy request to", u.String())
-- 
GitLab


From b708d525af86db77144563bfa4518a5ace7b3720 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Fri, 1 Sep 2017 21:19:32 -0700
Subject: [PATCH 10/39] made change to tset wthin artifact_test.go. Movement of
 internal/httperrors.go to internal/httperrors/httperrors.go so it is now in
 its own package. Adjust calls to its functions accordingly. Build passing.
 #78

---
 app.go                                       |  6 +++---
 domain.go                                    |  6 +++---
 internal/artifact/artifact.go                |  6 +++---
 internal/artifact/artifact_test.go           |  2 +-
 internal/{ => httperrors}/httperrors.go      | 18 +++++++++---------
 internal/{ => httperrors}/httperrors_test.go |  4 ++--
 6 files changed, 21 insertions(+), 21 deletions(-)
 rename internal/{ => httperrors}/httperrors.go (93%)
 rename internal/{ => httperrors}/httperrors_test.go (98%)

diff --git a/app.go b/app.go
index 77aa40e49..2b440d51f 100644
--- a/app.go
+++ b/app.go
@@ -16,8 +16,8 @@ import (
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/rs/cors"
 
-	"gitlab.com/gitlab-org/gitlab-pages/internal"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
 	"gitlab.com/gitlab-org/gitlab-pages/metrics"
 )
 
@@ -95,7 +95,7 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 		artifactURL, err := url.Parse(urlString)
 		if err != nil {
 			log.Println("Building of Artifact URL failed", err)
-			internal.Serve500(&w)
+			httperrors.Serve500(&w)
 			return
 		}
 		artifact.MakeRequest(&w, artifactURL)
@@ -104,7 +104,7 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 
 	domain := a.domain(host)
 	if domain == nil {
-		internal.Serve404(&w)
+		httperrors.Serve404(&w)
 		return
 	}
 
diff --git a/domain.go b/domain.go
index 56f67a172..fc6611e15 100644
--- a/domain.go
+++ b/domain.go
@@ -13,7 +13,7 @@ import (
 	"strings"
 	"time"
 
-	"gitlab.com/gitlab-org/gitlab-pages/internal"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httputil"
 )
 
@@ -216,7 +216,7 @@ func (d *domain) serveFromGroup(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Serve generic not found
-	internal.Serve404(w)
+	httperrors.Serve404(w)
 }
 
 func (d *domain) serveFromConfig(w http.ResponseWriter, r *http.Request) {
@@ -231,7 +231,7 @@ func (d *domain) serveFromConfig(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Serve generic not found
-	internal.Serve404(w)
+	httperrors.Serve404(w)
 }
 
 func (d *domain) ensureCertificate() (*tls.Certificate, error) {
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index b52360832..b6eb7e733 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -10,7 +10,7 @@ import (
 	"strconv"
 	"time"
 
-	"gitlab.com/gitlab-org/gitlab-pages/internal"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
 )
 
 const (
@@ -34,14 +34,14 @@ func MakeRequest(w http.ResponseWriter, u *url.URL) {
 	resp, err := artifactClient.Get(u.String())
 	if err != nil {
 		log.Println("Error while proxying artifact request", err)
-		internal.Serve500(w)
+		httperrors.Serve500(w)
 		return
 	}
 
 	contentLength, err := io.Copy(w, resp.Body)
 	if err != nil {
 		log.Println("Error reading response from proxied request", err)
-		internal.Serve500(w)
+		httperrors.Serve500(w)
 		return
 	}
 
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index cfff97b48..2f6d56894 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -37,7 +37,7 @@ func TestArtifactMakeRequest_bad_request(t *testing.T) {
 	result := httptest.NewRecorder()
 	MakeRequest(result, artifactURL)
 
-	assert.Equal(t, 500, result.Code)
+	assert.Equal(t, http.StatusInternalServerError, result.Code)
 }
 
 func TestURLBody(t *testing.T) {
diff --git a/internal/httperrors.go b/internal/httperrors/httperrors.go
similarity index 93%
rename from internal/httperrors.go
rename to internal/httperrors/httperrors.go
index b2a541dd0..9f04c1c45 100644
--- a/internal/httperrors.go
+++ b/internal/httperrors/httperrors.go
@@ -1,11 +1,11 @@
-package internal
+package httperrors
 
 import (
 	"fmt"
 	"net/http"
 )
 
-type errorContent struct {
+type content struct {
 	status       int
 	title        string
 	statusString string
@@ -14,14 +14,14 @@ type errorContent struct {
 }
 
 var (
-	content404 = errorContent{
+	content404 = content{
 		http.StatusNotFound,
 		"The page you're looking for could not be found (404)",
 		"404",
 		"The page you're looking for could not be found.",
 		"Make sure the address is correct and that the page hasn't moved.",
 	}
-	content500 = errorContent{
+	content500 = content{
 		http.StatusInternalServerError,
 		"Internal server error (500)",
 		"500",
@@ -87,15 +87,15 @@ const predefinedErrorPage = `
 </html>
 `
 
-func generateErrorHTML(e errorContent) string {
-	return fmt.Sprintf(predefinedErrorPage, e.title, e.statusString, e.header, e.subHeader)
+func generateErrorHTML(c content) string {
+	return fmt.Sprintf(predefinedErrorPage, c.title, c.statusString, c.header, c.subHeader)
 }
 
-func serveErrorPage(w http.ResponseWriter, e errorContent) {
+func serveErrorPage(w http.ResponseWriter, c content) {
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
 	w.Header().Set("X-Content-Type-Options", "nosniff")
-	w.WriteHeader(e.status)
-	fmt.Fprintln(w, generateErrorHTML(e))
+	w.WriteHeader(c.status)
+	fmt.Fprintln(w, generateErrorHTML(c))
 }
 
 // Serve404 returns a 404 error response / HTML page to the http.ResponseWriter
diff --git a/internal/httperrors_test.go b/internal/httperrors/httperrors_test.go
similarity index 98%
rename from internal/httperrors_test.go
rename to internal/httperrors/httperrors_test.go
index 79610cf8d..af9b4bcd6 100644
--- a/internal/httperrors_test.go
+++ b/internal/httperrors/httperrors_test.go
@@ -1,4 +1,4 @@
-package internal
+package httperrors
 
 import (
 	"net/http"
@@ -43,7 +43,7 @@ func (w *testResponseWriter) WriteHeader(statusCode int) {
 }
 
 var (
-	testingContent = errorContent{
+	testingContent = content{
 		http.StatusNotFound,
 		"Title",
 		"533",
-- 
GitLab


From ca669f8249cfd79547fb60b613f610c7d8c9b6ed Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Fri, 1 Sep 2017 22:38:53 -0700
Subject: [PATCH 11/39] fix various comemnting issues and issues witin tests.
 #78

---
 acceptance_test.go                     | 9 +++++----
 internal/httperrors/httperrors_test.go | 2 +-
 main.go                                | 8 ++++----
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index d8f9c1755..95074a1f9 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -214,7 +214,7 @@ func TestProxyRequest(t *testing.T) {
 	content := "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>"
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "text/html; charset=utf-8")
-		fmt.Fprintln(w, content)
+		fmt.Fprint(w, content)
 	}))
 	defer testServer.Close()
 	skipUnlessEnabled(t)
@@ -222,9 +222,10 @@ func TestProxyRequest(t *testing.T) {
 	defer teardown()
 
 	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2", "index.html")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	defer resp.Body.Close()
-	body, _ := ioutil.ReadAll(resp.Body)
-	assert.Contains(t, string(body), content)
+	body, err := ioutil.ReadAll(resp.Body)
+	require.NoError(t, err)
+	assert.Equal(t, string(body), content)
 	assert.Equal(t, http.StatusOK, resp.StatusCode)
 }
diff --git a/internal/httperrors/httperrors_test.go b/internal/httperrors/httperrors_test.go
index af9b4bcd6..47ea31a7e 100644
--- a/internal/httperrors/httperrors_test.go
+++ b/internal/httperrors/httperrors_test.go
@@ -8,7 +8,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-// creates a new implementation of htt.ResponseWriter that allows the
+// creates a new implementation of http.ResponseWriter that allows the
 // casting of values in order to aid testing efforts.
 type testResponseWriter struct {
 	status  int
diff --git a/main.go b/main.go
index a2ad38f0d..6d5780e1a 100644
--- a/main.go
+++ b/main.go
@@ -30,8 +30,8 @@ var (
 	daemonGID       = flag.Uint("daemon-gid", 0, "Drop privileges to this group")
 
 	disableCrossOriginRequests   = flag.Bool("disable-cross-origin-requests", false, "Disable cross-origin requests")
-	errArtifactSchemaUnsupported = errors.New("artifacts-server schema must be either http:// or https://")
-	approvedArtifactScheme       = regexp.MustCompile(`(\Ahttp\z)|(\Ahttps\z)`)
+	errArtifactSchemaUnsupported = errors.New("artifacts-server scheme must be either http:// or https://")
+	approvedArtifactScheme       = regexp.MustCompile(`\Ahttps?\z`)
 )
 
 func configFromFlags() appConfig {
@@ -55,8 +55,8 @@ func configFromFlags() appConfig {
 	if err != nil {
 		log.Fatal(err)
 	}
-
-	if u.Scheme != "" && !approvedArtifactScheme.MatchString(u.Scheme) {
+	// url.Parse ensures that the Scheme arttribute is always lower case.
+	if u.Scheme != "" && u.Scheme != "http" && u.Scheme != "https" {
 		log.Fatal(errArtifactSchemaUnsupported)
 	}
 
-- 
GitLab


From dea69bea21eaa9da026d27fc15b5cca77c3fde6b Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Fri, 1 Sep 2017 23:34:34 -0700
Subject: [PATCH 12/39] work on acceptance test to add more assertions.
 Additional small adjustments in internal/artifact/artifact_test.go #78

---
 acceptance_test.go                 | 4 ++++
 internal/artifact/artifact_test.go | 6 +++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index 95074a1f9..24c7a3f71 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -212,6 +212,7 @@ func TestStatusPage(t *testing.T) {
 
 func TestProxyRequest(t *testing.T) {
 	content := "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>"
+	contentLength := int64(len(content))
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "text/html; charset=utf-8")
 		fmt.Fprint(w, content)
@@ -228,4 +229,7 @@ func TestProxyRequest(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, string(body), content)
 	assert.Equal(t, http.StatusOK, resp.StatusCode)
+	t.Log(resp.ContentLength, "contentlentth")
+	assert.Equal(t, contentLength, resp.ContentLength)
+	// assert.Equal(t, "max-age=600", resp.Header.Get("Cache-Control")) TODO: having issue with this assertion
 }
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index 2f6d56894..cb9cb6efc 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -13,7 +13,7 @@ import (
 func TestMakeRequest(t *testing.T) {
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "text/html; charset=utf-8")
-		fmt.Fprintln(w, "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
+		fmt.Fprint(w, "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
 	}))
 	defer testServer.Close()
 	artifactURL, err := url.Parse(testServer.URL)
@@ -23,9 +23,9 @@ func TestMakeRequest(t *testing.T) {
 	MakeRequest(result, artifactURL)
 
 	assert.Equal(t, result.Header().Get("Content-Type"), "text/html; charset=utf-8")
-	assert.Equal(t, result.Header().Get("Content-Length"), "91")
+	assert.Equal(t, result.Header().Get("Content-Length"), "90")
 	assert.Equal(t, result.Header().Get("Cache-Control"), "max-age=600")
-	assert.Equal(t, string(result.Body.Bytes()), "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>\n")
+	assert.Equal(t, string(result.Body.Bytes()), "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
 }
 
 func TestArtifactMakeRequest_bad_request(t *testing.T) {
-- 
GitLab


From 3b29ae7fbcb1d1386ba66912aa6a86946e85be45 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Sat, 2 Sep 2017 13:38:14 -0700
Subject: [PATCH 13/39] adjust the URLBody to make use ofthe hostPattern regex
 rather than the now removed regex that only stripped ids. Add to tests
 accordignly. Adjustments to the httperrors.go to match other gitlab error
 pages as addressed in MR discussions. #78

---
 internal/artifact/artifact.go      |  13 +--
 internal/artifact/artifact_test.go |  10 +++
 internal/httperrors/httperrors.go  | 139 ++++++++++++++++++-----------
 3 files changed, 102 insertions(+), 60 deletions(-)

diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index b6eb7e733..157ddbdfa 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -3,7 +3,6 @@ package artifact
 import (
 	"fmt"
 	"io"
-	"log"
 	"net/http"
 	"net/url"
 	"regexp"
@@ -20,7 +19,6 @@ const (
 var (
 	artifactClient = &http.Client{Timeout: time.Second * 10}
 	hostPattern    = regexp.MustCompile(`(?i)\Aartifact~(\d+)~(\d+)\z`)
-	idsPattern     = regexp.MustCompile(`~(\d+)`)
 )
 
 // MakeRequest given an http.ResponseWriter will proxy a request to the
@@ -29,18 +27,14 @@ var (
 // body from said GET request to the http.ResponseWriter. A 500 will be
 // served in the event that an error is encountered.
 func MakeRequest(w http.ResponseWriter, u *url.URL) {
-	log.Println("Attempting to proxy request to", u.String())
-
 	resp, err := artifactClient.Get(u.String())
 	if err != nil {
-		log.Println("Error while proxying artifact request", err)
 		httperrors.Serve500(w)
 		return
 	}
 
 	contentLength, err := io.Copy(w, resp.Body)
 	if err != nil {
-		log.Println("Error reading response from proxied request", err)
 		httperrors.Serve500(w)
 		return
 	}
@@ -55,13 +49,12 @@ func MakeRequest(w http.ResponseWriter, u *url.URL) {
 // be close to "artifact~1~2". 1 and 2 represent Id's that need to be extracted
 // and interpolated into the const baseURL.
 func URLBody(host string) string {
-	ids := idsPattern.FindAllStringSubmatch(host, 2)
+	ids := hostPattern.FindAllStringSubmatch(host, -1)
 	strippedIds := make([]string, 2)
 
-	for i, id := range ids {
-		strippedIds[i] = id[1]
+	if len(ids) > 0 {
+		strippedIds[0], strippedIds[1] = ids[0][1], ids[0][2]
 	}
-
 	return fmt.Sprintf(baseURL, strippedIds[0], strippedIds[1])
 }
 
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index cb9cb6efc..00d1f813f 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -81,6 +81,16 @@ func TestURLBody(t *testing.T) {
 			"/projects//artifacts//raw",
 			"no ids",
 		},
+		{
+			"ArTifAcT~1~2",
+			"/projects/1/artifacts/2/raw",
+			"case insensitive",
+		},
+		{
+			"",
+			"/projects//artifacts//raw",
+			"empty host",
+		},
 	}
 
 	for _, c := range cases {
diff --git a/internal/httperrors/httperrors.go b/internal/httperrors/httperrors.go
index 9f04c1c45..fbdc240f9 100644
--- a/internal/httperrors/httperrors.go
+++ b/internal/httperrors/httperrors.go
@@ -19,71 +19,110 @@ var (
 		"The page you're looking for could not be found (404)",
 		"404",
 		"The page you're looking for could not be found.",
-		"Make sure the address is correct and that the page hasn't moved.",
+		`<p>The resource that you are attempting to access does not exist or you don't have the necessary permissions to view it.</p>
+                <p>Make sure the address is correct and that the page hasn't moved.</p>
+                <p>Please contact your GitLab administrator if you think this is a mistake.</p>`,
 	}
 	content500 = content{
 		http.StatusInternalServerError,
-		"Internal server error (500)",
+		"Something went wrong (500)",
 		"500",
-		"Internal server error.",
-		"We encountered an error while processing your request, please try again.",
+		"Whoops, something went wrong on our end.",
+		`<p>Try refreshing the page, or going back and attempting the action again.</p>
+                <p>Please contact your GitLab administrator if this problem persists.</p>`,
 	}
 )
 
 const predefinedErrorPage = `
 <!DOCTYPE html>
 <html>
-  <head>
-    <title>%v</title>
-    <style>
-      body {
-        color: #666;
-        text-align: center;
-        font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
-        margin: 0;
-        width: 800px;
-        margin: auto;
-        font-size: 14px;
-      }
+<head>
+  <meta content="width=device-width, initial-scale=1, maximum-scale=1" name="viewport">
+  <title>%v</title>
+  <style>
+    body {
+      color: #666;
+      text-align: center;
+      font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
+      margin: auto;
+      font-size: 14px;
+    }
 
-      h1 {
-        font-size: 56px;
-        line-height: 100px;
-        font-weight: normal;
-        color: #456;
-      }
+    h1 {
+      font-size: 56px;
+      line-height: 100px;
+      font-weight: 400;
+      color: #456;
+    }
 
-      h2 {
-        font-size: 24px;
-        color: #666;
-        line-height: 1.5em;
-      }
+    h2 {
+      font-size: 24px;
+      color: #666;
+      line-height: 1.5em;
+    }
 
-      h3 {
-        color: #456;
-        font-size: 20px;
-        font-weight: normal;
-        line-height: 28px;
-      }
+    h3 {
+      color: #456;
+      font-size: 20px;
+      font-weight: 400;
+      line-height: 28px;
+    }
 
-      hr {
-        margin: 18px 0;
-        border: 0;
-        border-top: 1px solid #EEE;
-        border-bottom: 1px solid white;
-      }
-    </style>
-  </head>
-
-  <body>
-    <h1>
-      <img src="data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiIHN0YW5kYWxvbmU9Im5vIj8+Cjxzdmcgd2lkdGg9IjIxMHB4IiBoZWlnaHQ9IjIxMHB4IiB2aWV3Qm94PSIwIDAgMjEwIDIxMCIgdmVyc2lvbj0iMS4xIiB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHhtbG5zOnhsaW5rPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5L3hsaW5rIiB4bWxuczpza2V0Y2g9Imh0dHA6Ly93d3cuYm9oZW1pYW5jb2RpbmcuY29tL3NrZXRjaC9ucyI+CiAgICA8IS0tIEdlbmVyYXRvcjogU2tldGNoIDMuMy4yICgxMjA0MykgLSBodHRwOi8vd3d3LmJvaGVtaWFuY29kaW5nLmNvbS9za2V0Y2ggLS0+CiAgICA8dGl0bGU+U2xpY2UgMTwvdGl0bGU+CiAgICA8ZGVzYz5DcmVhdGVkIHdpdGggU2tldGNoLjwvZGVzYz4KICAgIDxkZWZzPjwvZGVmcz4KICAgIDxnIGlkPSJQYWdlLTEiIHN0cm9rZT0ibm9uZSIgc3Ryb2tlLXdpZHRoPSIxIiBmaWxsPSJub25lIiBmaWxsLXJ1bGU9ImV2ZW5vZGQiIHNrZXRjaDp0eXBlPSJNU1BhZ2UiPgogICAgICAgIDxnIGlkPSJsb2dvIiBza2V0Y2g6dHlwZT0iTVNMYXllckdyb3VwIiB0cmFuc2Zvcm09InRyYW5zbGF0ZSgwLjAwMDAwMCwgMTAuMDAwMDAwKSI+CiAgICAgICAgICAgIDxnIGlkPSJQYWdlLTEiIHNrZXRjaDp0eXBlPSJNU1NoYXBlR3JvdXAiPgogICAgICAgICAgICAgICAgPGcgaWQ9IkZpbGwtMS0rLUdyb3VwLTI0Ij4KICAgICAgICAgICAgICAgICAgICA8ZyBpZD0iR3JvdXAtMjQiPgogICAgICAgICAgICAgICAgICAgICAgICA8ZyBpZD0iR3JvdXAiPgogICAgICAgICAgICAgICAgICAgICAgICAgICAgPHBhdGggZD0iTTEwNS4wNjE0LDE5My42NTUgTDEwNS4wNjE0LDE5My42NTUgTDE0My43MDE0LDc0LjczNCBMNjYuNDIxNCw3NC43MzQgTDEwNS4wNjE0LDE5My42NTUgTDEwNS4wNjE0LDE5My42NTUgWiIgaWQ9IkZpbGwtNCIgZmlsbD0iI0UyNDMyOSI+PC9wYXRoPgogICAgICAgICAgICAgICAgICAgICAgICAgICAgPHBhdGggZD0iTTEwNS4wNjE0LDE5My42NTQ4IEw2Ni40MjE0LDc0LjczMzggTDEyLjI2ODQsNzQuNzMzOCBMMTA1LjA2MTQsMTkzLjY1NDggTDEwNS4wNjE0LDE5My42NTQ4IFoiIGlkPSJGaWxsLTgiIGZpbGw9IiNGQzZEMjYiPjwvcGF0aD4KICAgICAgICAgICAgICAgICAgICAgICAgICAgIDxwYXRoIGQ9Ik0xMi4yNjg1LDc0LjczNDEgTDEyLjI2ODUsNzQuNzM0MSBMMC41MjY1LDExMC44NzMxIEMtMC41NDQ1LDExNC4xNjkxIDAuNjI4NSwxMTcuNzgwMSAzLjQzMjUsMTE5LjgxNzEgTDEwNS4wNjE1LDE5My42NTUxIEwxMi4yNjg1LDc0LjczNDEgTDEyLjI2ODUsNzQuNzM0MSBaIiBpZD0iRmlsbC0xMiIgZmlsbD0iI0ZDQTMyNiI+PC9wYXRoPgogICAgICAgICAgICAgICAgICAgICAgICAgICAgPHBhdGggZD0iTTEyLjI2ODUsNzQuNzM0MiBMNjYuNDIxNSw3NC43MzQyIEw0My4xNDg1LDMuMTA5MiBDNDEuOTUxNSwtMC41NzY4IDM2LjczNzUsLTAuNTc1OCAzNS41NDA1LDMuMTA5MiBMMTIuMjY4NSw3NC43MzQyIEwxMi4yNjg1LDc0LjczNDIgWiIgaWQ9IkZpbGwtMTYiIGZpbGw9IiNFMjQzMjkiPjwvcGF0aD4KICAgICAgICAgICAgICAgICAgICAgICAgICAgIDxwYXRoIGQ9Ik0xMDUuMDYxNCwxOTMuNjU0OCBMMTQzLjcwMTQsNzQuNzMzOCBMMTk3Ljg1NDQsNzQuNzMzOCBMMTA1LjA2MTQsMTkzLjY1NDggTDEwNS4wNjE0LDE5My42NTQ4IFoiIGlkPSJGaWxsLTE4IiBmaWxsPSIjRkM2RDI2Ij48L3BhdGg+CiAgICAgICAgICAgICAgICAgICAgICAgICAgICA8cGF0aCBkPSJNMTk3Ljg1NDQsNzQuNzM0MSBMMTk3Ljg1NDQsNzQuNzM0MSBMMjA5LjU5NjQsMTEwLjg3MzEgQzIxMC42Njc0LDExNC4xNjkxIDIwOS40OTQ0LDExNy43ODAxIDIwNi42OTA0LDExOS44MTcxIEwxMDUuMDYxNCwxOTMuNjU1MSBMMTk3Ljg1NDQsNzQuNzM0MSBMMTk3Ljg1NDQsNzQuNzM0MSBaIiBpZD0iRmlsbC0yMCIgZmlsbD0iI0ZDQTMyNiI+PC9wYXRoPgogICAgICAgICAgICAgICAgICAgICAgICAgICAgPHBhdGggZD0iTTE5Ny44NTQ0LDc0LjczNDIgTDE0My43MDE0LDc0LjczNDIgTDE2Ni45NzQ0LDMuMTA5MiBDMTY4LjE3MTQsLTAuNTc2OCAxNzMuMzg1NCwtMC41NzU4IDE3NC41ODI0LDMuMTA5MiBMMTk3Ljg1NDQsNzQuNzM0MiBMMTk3Ljg1NDQsNzQuNzM0MiBaIiBpZD0iRmlsbC0yMiIgZmlsbD0iI0UyNDMyOSI+PC9wYXRoPgogICAgICAgICAgICAgICAgICAgICAgICA8L2c+CiAgICAgICAgICAgICAgICAgICAgPC9nPgogICAgICAgICAgICAgICAgPC9nPgogICAgICAgICAgICA8L2c+CiAgICAgICAgPC9nPgogICAgPC9nPgo8L3N2Zz4=" /><br />
-      %v
-    </h1>
+    hr {
+      max-width: 800px;
+      margin: 18px auto;
+      border: 0;
+      border-top: 1px solid #EEE;
+      border-bottom: 1px solid white;
+    }
+
+    img {
+      max-width: 40vw;
+      display: block;
+      margin: 40px auto;
+    }
+
+    a {
+      line-height: 100px;
+      font-weight: 400;
+      color: #4A8BEE;
+      font-size: 18px;
+      text-decoration: none;
+    }
+
+    .container {
+      margin: auto 20px;
+    }
+
+    .go-back {
+      display: none;
+    }
+
+  </style>
+</head>
+
+<body>
+  <img src="data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iMjEwIiBoZWlnaHQ9IjIxMCIgdmlld0JveD0iMCAwIDIxMCAyMTAiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyI+CiAgPHBhdGggZD0iTTEwNS4wNjE0IDIwMy42NTVsMzguNjQtMTE4LjkyMWgtNzcuMjhsMzguNjQgMTE4LjkyMXoiIGZpbGw9IiNlMjQzMjkiLz4KICA8cGF0aCBkPSJNMTA1LjA2MTQgMjAzLjY1NDhsLTM4LjY0LTExOC45MjFoLTU0LjE1M2w5Mi43OTMgMTE4LjkyMXoiIGZpbGw9IiNmYzZkMjYiLz4KICA8cGF0aCBkPSJNMTIuMjY4NSA4NC43MzQxbC0xMS43NDIgMzYuMTM5Yy0xLjA3MSAzLjI5Ni4xMDIgNi45MDcgMi45MDYgOC45NDRsMTAxLjYyOSA3My44MzgtOTIuNzkzLTExOC45MjF6IiBmaWxsPSIjZmNhMzI2Ii8+CiAgPHBhdGggZD0iTTEyLjI2ODUgODQuNzM0Mmg1NC4xNTNsLTIzLjI3My03MS42MjVjLTEuMTk3LTMuNjg2LTYuNDExLTMuNjg1LTcuNjA4IDBsLTIzLjI3MiA3MS42MjV6IiBmaWxsPSIjZTI0MzI5Ii8+CiAgPHBhdGggZD0iTTEwNS4wNjE0IDIwMy42NTQ4bDM4LjY0LTExOC45MjFoNTQuMTUzbC05Mi43OTMgMTE4LjkyMXoiIGZpbGw9IiNmYzZkMjYiLz4KICA8cGF0aCBkPSJNMTk3Ljg1NDQgODQuNzM0MWwxMS43NDIgMzYuMTM5YzEuMDcxIDMuMjk2LS4xMDIgNi45MDctMi45MDYgOC45NDRsLTEwMS42MjkgNzMuODM4IDkyLjc5My0xMTguOTIxeiIgZmlsbD0iI2ZjYTMyNiIvPgogIDxwYXRoIGQ9Ik0xOTcuODU0NCA4NC43MzQyaC01NC4xNTNsMjMuMjczLTcxLjYyNWMxLjE5Ny0zLjY4NiA2LjQxMS0zLjY4NSA3LjYwOCAwbDIzLjI3MiA3MS42MjV6IiBmaWxsPSIjZTI0MzI5Ii8+Cjwvc3ZnPgo="
+       alt="GitLab Logo" />
+  <h1>
+    %v
+  </h1>
+  <div class="container">
     <h3>%v</h3>
-    <hr/>
-    <p>%v</p>
-  </body>
+    <hr />
+    %v
+    <a href="javascript:history.back()" class="js-go-back go-back">Go back</a>
+  </div>
+  <script>
+    (function () {
+      var goBack = document.querySelector('.js-go-back');
+
+      if (history.length > 1) {
+        goBack.style.display = 'inline';
+      }
+    })();
+  </script>
+</body>
 </html>
 `
 
-- 
GitLab


From ba2bb33c3be0969b506c153b849a291b746d7d4f Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Sun, 3 Sep 2017 22:07:20 -0700
Subject: [PATCH 14/39] adjustment to artifact.go and corresponsing specs as
 the MakeRequset functino with the artifacts package was change to return a
 bool and intake a string rather than url.Url type. Adjustments to app.go
 accordingl #78

---
 acceptance_test.go                 |  1 +
 app.go                             |  8 +-------
 internal/artifact/artifact.go      | 12 +++++++++---
 internal/artifact/artifact_test.go |  9 ++-------
 4 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index 24c7a3f71..24b44a054 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -211,6 +211,7 @@ func TestStatusPage(t *testing.T) {
 }
 
 func TestProxyRequest(t *testing.T) {
+	skipUnlessEnabled(t)
 	content := "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>"
 	contentLength := int64(len(content))
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
diff --git a/app.go b/app.go
index 2b440d51f..c286769c2 100644
--- a/app.go
+++ b/app.go
@@ -6,7 +6,6 @@ import (
 	"log"
 	"net"
 	"net/http"
-	"net/url"
 	"strconv"
 	"strings"
 	"sync"
@@ -92,14 +91,9 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 	// value is created, and an attempt to proxy the request is made
 	if a.shouldProxy(host) {
 		urlString := fmt.Sprintf("%v%v%v", a.ArtifactServer.String(), artifact.URLBody(host), r.URL.Path)
-		artifactURL, err := url.Parse(urlString)
-		if err != nil {
-			log.Println("Building of Artifact URL failed", err)
-			httperrors.Serve500(&w)
+		if artifact.MakeRequest(&w, urlString) {
 			return
 		}
-		artifact.MakeRequest(&w, artifactURL)
-		return
 	}
 
 	domain := a.domain(host)
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 157ddbdfa..b4edf58e5 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -26,22 +26,28 @@ var (
 // a GET request, it then maps the Content-Type, Content-Length, and response
 // body from said GET request to the http.ResponseWriter. A 500 will be
 // served in the event that an error is encountered.
-func MakeRequest(w http.ResponseWriter, u *url.URL) {
+func MakeRequest(w http.ResponseWriter, rawURL string) bool {
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return false
+	}
+
 	resp, err := artifactClient.Get(u.String())
 	if err != nil {
 		httperrors.Serve500(w)
-		return
+		return true
 	}
 
 	contentLength, err := io.Copy(w, resp.Body)
 	if err != nil {
 		httperrors.Serve500(w)
-		return
+		return true
 	}
 
 	w.Header().Set("Content-Type", resp.Header.Get("Content-Type"))
 	w.Header().Set("Content-Length", strconv.FormatInt(contentLength, 10))
 	w.Header().Set("Cache-Control", "max-age=600")
+	return true
 }
 
 // URLBody returns a string that represents the body of a url an
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index 00d1f813f..7262afa7b 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
-	"net/url"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -16,11 +15,9 @@ func TestMakeRequest(t *testing.T) {
 		fmt.Fprint(w, "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
 	}))
 	defer testServer.Close()
-	artifactURL, err := url.Parse(testServer.URL)
-	assert.NoError(t, err)
 
 	result := httptest.NewRecorder()
-	MakeRequest(result, artifactURL)
+	MakeRequest(result, testServer.URL)
 
 	assert.Equal(t, result.Header().Get("Content-Type"), "text/html; charset=utf-8")
 	assert.Equal(t, result.Header().Get("Content-Length"), "90")
@@ -31,11 +28,9 @@ func TestMakeRequest(t *testing.T) {
 func TestArtifactMakeRequest_bad_request(t *testing.T) {
 	testServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
 	defer testServer.Close()
-	artifactURL, err := url.Parse(testServer.URL)
-	assert.NoError(t, err)
 
 	result := httptest.NewRecorder()
-	MakeRequest(result, artifactURL)
+	MakeRequest(result, testServer.URL)
 
 	assert.Equal(t, http.StatusInternalServerError, result.Code)
 }
-- 
GitLab


From 2f98c6faea5042af2fc8e67dca3677276c12be35 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Sun, 3 Sep 2017 22:38:59 -0700
Subject: [PATCH 15/39] adjustments to internal/artifact/artifact.go to write
 to ehaders before calling io.copy. Adjustments to acceptance_test.go to
 ensure the Cache-Control header is set within TestProxyRequest #78

---
 acceptance_test.go            |  3 +--
 internal/artifact/artifact.go | 12 +++++-------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index 24b44a054..4c95543bb 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -230,7 +230,6 @@ func TestProxyRequest(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, string(body), content)
 	assert.Equal(t, http.StatusOK, resp.StatusCode)
-	t.Log(resp.ContentLength, "contentlentth")
 	assert.Equal(t, contentLength, resp.ContentLength)
-	// assert.Equal(t, "max-age=600", resp.Header.Get("Cache-Control")) TODO: having issue with this assertion
+	assert.Equal(t, "max-age=600", resp.Header.Get("Cache-Control"))
 }
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index b4edf58e5..02062cd72 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -38,15 +38,13 @@ func MakeRequest(w http.ResponseWriter, rawURL string) bool {
 		return true
 	}
 
-	contentLength, err := io.Copy(w, resp.Body)
-	if err != nil {
-		httperrors.Serve500(w)
-		return true
-	}
-
 	w.Header().Set("Content-Type", resp.Header.Get("Content-Type"))
-	w.Header().Set("Content-Length", strconv.FormatInt(contentLength, 10))
 	w.Header().Set("Cache-Control", "max-age=600")
+	w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10))
+	_, err = io.Copy(w, resp.Body)
+	if err != nil {
+		return false // TODO: false would have the request fall through, should we do that?
+	}
 	return true
 }
 
-- 
GitLab


From 5f4a8b172a56e37abe8f7154bcc589ac09298ee6 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Mon, 4 Sep 2017 11:57:06 -0700
Subject: [PATCH 16/39] adjustments to the internal/artifact package in order
 to the MatchHost function match the pagesDomain. Adjustments to the app.go
 file accordingly along with teaks to teh acceptance_test. Build passing #78

---
 acceptance_test.go                 |  4 +--
 app.go                             |  2 +-
 app_test.go                        | 20 +++++------
 internal/artifact/artifact.go      | 16 ++++-----
 internal/artifact/artifact_test.go | 53 ++++++++++++++++++++++--------
 5 files changed, 59 insertions(+), 36 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index 4c95543bb..6a73dae75 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -220,10 +220,10 @@ func TestProxyRequest(t *testing.T) {
 	}))
 	defer testServer.Close()
 	skipUnlessEnabled(t)
-	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL)
+	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL, "-pages-domain=gitlab.io")
 	defer teardown()
 
-	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2", "index.html")
+	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab.io", "index.html")
 	require.NoError(t, err)
 	defer resp.Body.Close()
 	body, err := ioutil.ReadAll(resp.Body)
diff --git a/app.go b/app.go
index c286769c2..7bc96f89d 100644
--- a/app.go
+++ b/app.go
@@ -55,7 +55,7 @@ func (a *theApp) ServeTLS(ch *tls.ClientHelloInfo) (*tls.Certificate, error) {
 }
 
 func (a *theApp) shouldProxy(host string) bool {
-	return (a.ArtifactServer.String() != "") && artifact.MatchHost(host)
+	return (a.ArtifactServer.String() != "") && artifact.MatchHost(a.Domain, host)
 }
 
 func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https bool) {
diff --git a/app_test.go b/app_test.go
index 0932276a0..9550569cc 100644
--- a/app_test.go
+++ b/app_test.go
@@ -11,41 +11,39 @@ func TestShouldProxy(t *testing.T) {
 	cases := []struct {
 		ArtifactServer string
 		Host           string
+		PagesDomain    string
 		Expected       bool
 		Description    string
 	}{
 		{
 			"https://gitlab.com/api/v4",
-			"artifact~1~2",
+			"artifact~1~2.gitlab.io",
+			"gitlab.io",
 			true,
 			"basic case",
 		},
 		{
 			"",
-			"artifact~1~2",
+			"artifact~1~2.gitlab.io",
+			"gitlab.io",
 			false,
 			"Artifact server is not provided",
 		},
 		{
 			"https://gitlab.com/api/v4",
-			"artifact~1~2~3~4",
+			"artifact~1~2.gitlab.io",
+			"someothersite.io",
 			false,
 			"host does not match the expected pattern",
 		},
-		{
-			"",
-			"artifact~1~2~3~4",
-			false,
-			"host does not match the expected pattern, and there is not an artifact server provided",
-		},
 	}
 
 	for _, c := range cases {
 		URL, err := url.Parse(c.ArtifactServer)
 		assert.NoError(t, err)
 
-		app := &theApp{appConfig: appConfig{ArtifactServer: URL}}
+		app := &theApp{appConfig: appConfig{ArtifactServer: URL, Domain: c.PagesDomain}}
 		actual := app.shouldProxy(c.Host)
-		assert.Equal(t, actual, c.Expected, c.Description)
+		assert.Equal(t, c.Expected, actual, c.Description)
 	}
 }
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 02062cd72..3722f6e67 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -18,7 +18,7 @@ const (
 
 var (
 	artifactClient = &http.Client{Timeout: time.Second * 10}
-	hostPattern    = regexp.MustCompile(`(?i)\Aartifact~(\d+)~(\d+)\z`)
+	hostPattern    = regexp.MustCompile(`(?i)\Aartifact~(\d+)~(\d+)`)
 )
 
 // MakeRequest given an http.ResponseWriter will proxy a request to the
@@ -41,11 +41,8 @@ func MakeRequest(w http.ResponseWriter, rawURL string) bool {
 	w.Header().Set("Content-Type", resp.Header.Get("Content-Type"))
 	w.Header().Set("Cache-Control", "max-age=600")
 	w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10))
-	_, err = io.Copy(w, resp.Body)
-	if err != nil {
-		return false // TODO: false would have the request fall through, should we do that?
-	}
-	return true
+	io.Copy(w, resp.Body)
+	return true // TODO: returning false here will call the logic to fall through, which may not be a good idea.
 }
 
 // URLBody returns a string that represents the body of a url an
@@ -63,8 +60,9 @@ func URLBody(host string) string {
 }
 
 // MatchHost when provided a string checks to see if it matches the predefined
-// host pattern, returning a bool. Matching patters should look like the
+// host pattern, returning a bool. Matching patterns should look like the
 // following: "artifact~1~2"
-func MatchHost(s string) bool {
-	return hostPattern.MatchString(s)
+func MatchHost(pagesDomain, s string) bool {
+	pattern := regexp.MustCompile(fmt.Sprintf(`%s\.%s\z`, hostPattern, pagesDomain))
+	return pattern.MatchString(s)
 }
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index 7262afa7b..b8ee9220d 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -86,6 +86,16 @@ func TestURLBody(t *testing.T) {
 			"/projects//artifacts//raw",
 			"empty host",
 		},
+		{
+			"artifact~1~2.gitlab.io",
+			"/projects/1/artifacts/2/raw",
+			"basic with host suffix",
+		},
+		{
+			"artifact~1~2~3~4",
+			"/projects/1/artifacts/2/raw",
+			"basic with host suffix",
+		},
 	}
 
 	for _, c := range cases {
@@ -97,63 +107,80 @@ func TestURLBody(t *testing.T) {
 func TestMatchHost(t *testing.T) {
 	cases := []struct {
 		URLHost     string
+		PagesDomain string
 		Expected    bool
 		Description string
 	}{
 		{
-			"artifact~1~2",
+			"artifact~1~2.gitlab.io",
+			"gitlab.io",
 			true,
 			"basic case",
 		},
 		{
-			"ARTIFACT~1~2",
+			"ARTIFACT~1~2.gitlab.io",
+			"gitlab.io",
 			true,
 			"capital letters case",
 		},
 		{
-			"ARTIFACT~11234~2908908",
+			"ARTIFACT~11234~2908908.gitlab.io",
+			"gitlab.io",
 			true,
 			"additional capital letters case",
 		},
 		{
-			"artifact~10000~20000",
+			"artifact~10000~20000.gitlab.io",
+			"gitlab.io",
 			true,
 			"expanded case",
 		},
 		{
-			"artifact~86753095555~55550935768",
+			"artifact~86753095555~55550935768.gitlab.io",
+			"gitlab.io",
 			true,
 			"large number case",
 		},
 		{
-			"artifact~one~two",
+			"artifact~one~two.gitlab.io",
+			"gitlab.io",
 			false,
 			"letters rather than numbers",
 		},
 		{
-			"artifact~One111~tWo222",
+			"artifact~One111~tWo222.gitlab.io",
+			"gitlab.io",
 			false,
 			"Mixture of alphanumeric",
 		},
 		{
-			"artifact~!@#$%~%$#@!",
+			"artifact~!@#$%~%$#@!.gitlab.io",
+			"gitlab.io",
 			false,
 			"special characters",
 		},
 		{
-			"artifact~1~2~3",
+			"artifact~1.gitlab.io",
+			"gitlab.io",
+			false,
+			"not enough ids",
+		},
+		{
+			"artifact~1~2~34444~1~4.gitlab.io",
+			"gitlab.io",
 			false,
 			"too many ids",
 		},
 		{
-			"artifact~1",
+			"artifact~1~2.gitlab.io",
+			"otherhost.io",
 			false,
-			"not enough ids",
+			"different domain / suffix",
 		},
 	}
 
 	for _, c := range cases {
-		actual := MatchHost(c.URLHost)
-		assert.Equal(t, actual, c.Expected, c.Description)
+		actual := MatchHost(c.PagesDomain, c.URLHost)
+		assert.Equal(t, c.Expected, actual, c.Description)
 	}
 }
-- 
GitLab


From 9aba863fe559a82f6d793f2d0f2988a7f51e8ee9 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Mon, 4 Sep 2017 12:25:07 -0700
Subject: [PATCH 17/39] adjustments to the internal/artifact package in order
 to the MatchHost function match the pagesDomain. Adjustments to the app.go
 file accordingly along with teaks to teh acceptance_test, slight change
 within app.go to only proxy when the domain value is set within theApp
 struct. #78

---
 app.go                             |  2 +-
 internal/artifact/artifact.go      |  2 +-
 internal/artifact/artifact_test.go | 27 +++++++++++----------------
 3 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/app.go b/app.go
index 7bc96f89d..1b32fde9f 100644
--- a/app.go
+++ b/app.go
@@ -55,7 +55,7 @@ func (a *theApp) ServeTLS(ch *tls.ClientHelloInfo) (*tls.Certificate, error) {
 }
 
 func (a *theApp) shouldProxy(host string) bool {
-	return (a.ArtifactServer.String() != "") && artifact.MatchHost(a.Domain, host)
+	return (a.ArtifactServer.String() != "") && a.Domain != "" && artifact.MatchHost(a.Domain, host)
 }
 
 func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https bool) {
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 3722f6e67..f3f6021a1 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -13,7 +13,7 @@ import (
 )
 
 const (
-	baseURL = "/projects/%v/artifacts/%v/raw"
+	baseURL = "/projects/%s/jobs/%s/artifacts"
 )
 
 var (
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index b8ee9220d..243b2bea7 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -43,64 +43,59 @@ func TestURLBody(t *testing.T) {
 	}{
 		{
 			"artifact~1~2",
-			"/projects/1/artifacts/2/raw",
+			"/projects/1/jobs/2/artifacts",
 			"basic case",
 		},
 		{
 			"artifact~10000~20000",
-			"/projects/10000/artifacts/20000/raw",
+			"/projects/10000/jobs/20000/artifacts",
 			"expanded case",
 		},
 		{
 			"artifact~86753095555~55550935768",
-			"/projects/86753095555/artifacts/55550935768/raw",
+			"/projects/86753095555/jobs/55550935768/artifacts",
 			"large number case",
 		},
 		{
 			"artifact~one~two",
-			"/projects//artifacts//raw",
+			"/projects//jobs//artifacts",
 			"letters rather than numbers",
 		},
 		{
 			"artifact~One111~tWo222",
-			"/projects//artifacts//raw",
+			"/projects//jobs//artifacts",
 			"Mixture of alphanumeric",
 		},
 		{
 			"artifact~!@#$%~%$#@!",
-			"/projects//artifacts//raw",
+			"/projects//jobs//artifacts",
 			"special characters",
 		},
 		{
 			"artifact~~",
-			"/projects//artifacts//raw",
+			"/projects//jobs//artifacts",
 			"no ids",
 		},
 		{
 			"ArTifAcT~1~2",
-			"/projects/1/artifacts/2/raw",
+			"/projects/1/jobs/2/artifacts",
 			"case insensitive",
 		},
 		{
 			"",
-			"/projects//artifacts//raw",
+			"/projects//jobs//artifacts",
 			"empty host",
 		},
 		{
 			"artifact~1~2.gitlab.io",
-			"/projects/1/artifacts/2/raw",
-			"basic with host suffix",
-		},
-		{
-			"artifact~1~2~3~4",
-			"/projects/1/artifacts/2/raw",
+			"/projects/1/jobs/2/artifacts",
 			"basic with host suffix",
 		},
 	}
 
 	for _, c := range cases {
 		actual := URLBody(c.URLHost)
-		assert.Equal(t, actual, c.Expected, c.Description)
+		assert.Equal(t, c.Expected, actual, c.Description)
 	}
 }
 
-- 
GitLab


From 4d36400a9b36a440f14c4ea1b2cbad38a73f5864 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Mon, 4 Sep 2017 20:57:40 -0700
Subject: [PATCH 18/39] Create new artifact type wtihin the internal/artifact
 package along with new builder function. Integrate this new type into the
 appConfig and made adjustments to the rest of the app where applicable.
 Adjust specs accordingly #78

---
 app.go                             |  4 +--
 app_config.go                      |  7 ++--
 app_test.go                        | 12 +++++--
 internal/artifact/artifact.go      | 27 ++++++++++----
 internal/artifact/artifact_test.go |  6 ++--
 main.go                            | 58 +++++++++++++++++-------------
 6 files changed, 71 insertions(+), 43 deletions(-)

diff --git a/app.go b/app.go
index 1b32fde9f..5ec57a1be 100644
--- a/app.go
+++ b/app.go
@@ -55,7 +55,7 @@ func (a *theApp) ServeTLS(ch *tls.ClientHelloInfo) (*tls.Certificate, error) {
 }
 
 func (a *theApp) shouldProxy(host string) bool {
-	return (a.ArtifactServer.String() != "") && a.Domain != "" && artifact.MatchHost(a.Domain, host)
+	return (a.Artifact != nil) && (a.Artifact.Server != nil) && (a.Artifact.Server.String() != "") && a.Artifact.Pattern.MatchString(host)
 }
 
 func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https bool) {
@@ -90,7 +90,7 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 	// In the event a host is prefixed with the artifact prefix an artifact
 	// value is created, and an attempt to proxy the request is made
 	if a.shouldProxy(host) {
-		urlString := fmt.Sprintf("%v%v%v", a.ArtifactServer.String(), artifact.URLBody(host), r.URL.Path)
+		urlString := fmt.Sprintf("%v%v%v", a.Artifact.Server.String(), artifact.URLBody(host), r.URL.Path)
 		if artifact.MakeRequest(&w, urlString) {
 			return
 		}
diff --git a/app_config.go b/app_config.go
index 24101ee1d..caa0220a2 100644
--- a/app_config.go
+++ b/app_config.go
@@ -1,11 +1,10 @@
 package main
 
-import "net/url"
+import "gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
 
 type appConfig struct {
-	Domain         string
-	ArtifactServer *url.URL
-
+	Domain          string
+	Artifact        *artifact.Artifact
 	RootCertificate []byte
 	RootKey         []byte
 
diff --git a/app_test.go b/app_test.go
index 9550569cc..b795f0bd4 100644
--- a/app_test.go
+++ b/app_test.go
@@ -2,9 +2,11 @@ package main
 
 import (
 	"net/url"
+	"regexp"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
 )
 
 func TestShouldProxy(t *testing.T) {
@@ -41,9 +43,13 @@ func TestShouldProxy(t *testing.T) {
 	for _, c := range cases {
 		URL, err := url.Parse(c.ArtifactServer)
 		assert.NoError(t, err)
+		art := &artifact.Artifact{
+			Server:  URL,
+			Pattern: regexp.MustCompile(c.PagesDomain),
+		}
 
-		app := &theApp{appConfig: appConfig{ArtifactServer: URL, Domain: c.PagesDomain}}
-		actual := app.shouldProxy(c.Host)
-		assert.Equal(t, c.Expected, actual, c.Description)
+		app := &theApp{appConfig: appConfig{Domain: c.PagesDomain, Artifact: art}}
+		t.Log(app.Artifact, "here", c.Description)
+		assert.Equal(t, c.Expected, app.shouldProxy(c.Host), c.Description)
 	}
 }
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index f3f6021a1..bf747b679 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -21,6 +21,25 @@ var (
 	hostPattern    = regexp.MustCompile(`(?i)\Aartifact~(\d+)~(\d+)`)
 )
 
+// Artifact is a struct that is made up of a url.URL, http.Client, and
+// regexp.Regexp that is used to proxy requests where applicable.
+type Artifact struct {
+	Server  *url.URL
+	Client  *http.Client
+	Pattern *regexp.Regexp
+}
+
+// New when provided the arguments defined herein, retruns a pointer to an
+// Artifact that is used to proxy requests.
+func New(u *url.URL, timeout int, pagesDomain string) *Artifact {
+	return &Artifact{
+		Server:  u,
+		Client:  &http.Client{Timeout: time.Second * time.Duration(timeout)},
+		Pattern: hostPatternGen(pagesDomain),
+	}
+
+}
+
 // MakeRequest given an http.ResponseWriter will proxy a request to the
 // underlying url.URL pointer's address on the related Artifact pointer. Making
 // a GET request, it then maps the Content-Type, Content-Length, and response
@@ -59,10 +78,6 @@ func URLBody(host string) string {
 	return fmt.Sprintf(baseURL, strippedIds[0], strippedIds[1])
 }
 
-// MatchHost when provided a string checks to see if it matches the predefined
-// host pattern, returning a bool. Matching patterns should look like the
-// following: "artifact~1~2"
-func MatchHost(pagesDomain, s string) bool {
-	pattern := regexp.MustCompile(fmt.Sprintf(`%s\.%s\z`, hostPattern, pagesDomain))
-	return pattern.MatchString(s)
+func hostPatternGen(pagesDomain string) *regexp.Regexp {
+	return regexp.MustCompile(fmt.Sprintf(`%s\.%s\z`, hostPattern, pagesDomain))
 }
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index 243b2bea7..ef3abe95a 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -99,7 +99,7 @@ func TestURLBody(t *testing.T) {
 	}
 }
 
-func TestMatchHost(t *testing.T) {
+func TestMatchHostGen(t *testing.T) {
 	cases := []struct {
 		URLHost     string
 		PagesDomain string
@@ -175,7 +175,7 @@ func TestMatchHost(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		actual := MatchHost(c.PagesDomain, c.URLHost)
-		assert.Equal(t, c.Expected, actual, c.Description)
+		reg := hostPatternGen(c.PagesDomain)
+		assert.Equal(t, c.Expected, reg.MatchString(c.URLHost), c.Description)
 	}
 }
diff --git a/main.go b/main.go
index 6d5780e1a..233f6c266 100644
--- a/main.go
+++ b/main.go
@@ -6,8 +6,9 @@ import (
 	"log"
 	"net/url"
 	"os"
-	"regexp"
 	"strings"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
 )
 
 // VERSION stores the information about the semantic version of application
@@ -17,21 +18,22 @@ var VERSION = "dev"
 var REVISION = "HEAD"
 
 var (
-	pagesRootCert   = flag.String("root-cert", "", "The default path to file certificate to serve static pages")
-	pagesRootKey    = flag.String("root-key", "", "The default path to file certificate to serve static pages")
-	redirectHTTP    = flag.Bool("redirect-http", false, "Redirect pages from HTTP to HTTPS")
-	useHTTP2        = flag.Bool("use-http2", true, "Enable HTTP2 support")
-	pagesRoot       = flag.String("pages-root", "shared/pages", "The directory where pages are stored")
-	pagesDomain     = flag.String("pages-domain", "gitlab-example.com", "The domain to serve static pages")
-	artifactsServer = flag.String("artifacts-server", "", "https://gitlab.com/api/v4")
-	pagesStatus     = flag.String("pages-status", "", "The url path for a status page, e.g., /@status")
-	metricsAddress  = flag.String("metrics-address", "", "The address to listen on for metrics requests")
-	daemonUID       = flag.Uint("daemon-uid", 0, "Drop privileges to this user")
-	daemonGID       = flag.Uint("daemon-gid", 0, "Drop privileges to this group")
-
-	disableCrossOriginRequests   = flag.Bool("disable-cross-origin-requests", false, "Disable cross-origin requests")
-	errArtifactSchemaUnsupported = errors.New("artifacts-server scheme must be either http:// or https://")
-	approvedArtifactScheme       = regexp.MustCompile(`\Ahttps?\z`)
+	pagesRootCert          = flag.String("root-cert", "", "The default path to file certificate to serve static pages")
+	pagesRootKey           = flag.String("root-key", "", "The default path to file certificate to serve static pages")
+	redirectHTTP           = flag.Bool("redirect-http", false, "Redirect pages from HTTP to HTTPS")
+	useHTTP2               = flag.Bool("use-http2", true, "Enable HTTP2 support")
+	pagesRoot              = flag.String("pages-root", "shared/pages", "The directory where pages are stored")
+	pagesDomain            = flag.String("pages-domain", "gitlab-example.com", "The domain to serve static pages")
+	artifactsServer        = flag.String("artifacts-server", "", "https://gitlab.com/api/v4")
+	artifactsServerTimeout = flag.Int("artifacts-server-timeout", 10, "Timeout (in seconds) for a proxied request to the artifacts server")
+	pagesStatus            = flag.String("pages-status", "", "The url path for a status page, e.g., /@status")
+	metricsAddress         = flag.String("metrics-address", "", "The address to listen on for metrics requests")
+	daemonUID              = flag.Uint("daemon-uid", 0, "Drop privileges to this user")
+	daemonGID              = flag.Uint("daemon-gid", 0, "Drop privileges to this group")
+
+	disableCrossOriginRequests     = flag.Bool("disable-cross-origin-requests", false, "Disable cross-origin requests")
+	errArtifactSchemaUnsupported   = errors.New("artifacts-server scheme must be either http:// or https://")
+	errArtifactsServerTimeoutValue = errors.New("artifacts-server-timeout must be greater than or equal to 1")
 )
 
 func configFromFlags() appConfig {
@@ -51,16 +53,22 @@ func configFromFlags() appConfig {
 		config.RootKey = readFile(*pagesRootKey)
 	}
 
-	u, err := url.Parse(*artifactsServer)
-	if err != nil {
-		log.Fatal(err)
+	if *artifactsServer != "" {
+		u, err := url.Parse(*artifactsServer)
+		if err != nil {
+			log.Fatal(err)
+		}
+		// url.Parse ensures that the Scheme arttribute is always lower case.
+		if u.Scheme != "" && u.Scheme != "http" && u.Scheme != "https" {
+			log.Fatal(errArtifactSchemaUnsupported)
+		}
+
+		if *artifactsServerTimeout < 1 {
+			log.Fatal(errArtifactsServerTimeoutValue)
+		}
+
+		config.Artifact = artifact.New(u, *artifactsServerTimeout, *pagesDomain)
 	}
-	// url.Parse ensures that the Scheme arttribute is always lower case.
-	if u.Scheme != "" && u.Scheme != "http" && u.Scheme != "https" {
-		log.Fatal(errArtifactSchemaUnsupported)
-	}
-
-	config.ArtifactServer = u
 	return config
 }
 
-- 
GitLab


From 2c36877618c32c4ff12c0bbe455f8e4c48cb30c9 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Mon, 4 Sep 2017 21:23:42 -0700
Subject: [PATCH 19/39] refactoring of internal/artifact and corresponding
 specs in order to have the MakeRequest function be a method on the Artifact
 type. Adjust the app.go file and specs accordingly #78

---
 app.go                             |  2 +-
 app_test.go                        |  1 -
 internal/artifact/artifact.go      |  9 ++++-----
 internal/artifact/artifact_test.go | 11 +++++++++--
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/app.go b/app.go
index 5ec57a1be..44043cf28 100644
--- a/app.go
+++ b/app.go
@@ -91,7 +91,7 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 	// value is created, and an attempt to proxy the request is made
 	if a.shouldProxy(host) {
 		urlString := fmt.Sprintf("%v%v%v", a.Artifact.Server.String(), artifact.URLBody(host), r.URL.Path)
-		if artifact.MakeRequest(&w, urlString) {
+		if a.Artifact.MakeRequest(&w, urlString) {
 			return
 		}
 	}
diff --git a/app_test.go b/app_test.go
index b795f0bd4..fa440b66a 100644
--- a/app_test.go
+++ b/app_test.go
@@ -49,7 +49,6 @@ func TestShouldProxy(t *testing.T) {
 		}
 
 		app := &theApp{appConfig: appConfig{Domain: c.PagesDomain, Artifact: art}}
-		t.Log(app.Artifact, "here", c.Description)
 		assert.Equal(t, c.Expected, app.shouldProxy(c.Host), c.Description)
 	}
 }
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index bf747b679..de93737d4 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -17,8 +17,7 @@ const (
 )
 
 var (
-	artifactClient = &http.Client{Timeout: time.Second * 10}
-	hostPattern    = regexp.MustCompile(`(?i)\Aartifact~(\d+)~(\d+)`)
+	hostPattern = regexp.MustCompile(`(?i)\Aartifact~(\d+)~(\d+)`)
 )
 
 // Artifact is a struct that is made up of a url.URL, http.Client, and
@@ -45,13 +44,13 @@ func New(u *url.URL, timeout int, pagesDomain string) *Artifact {
 // a GET request, it then maps the Content-Type, Content-Length, and response
 // body from said GET request to the http.ResponseWriter. A 500 will be
 // served in the event that an error is encountered.
-func MakeRequest(w http.ResponseWriter, rawURL string) bool {
+func (a *Artifact) MakeRequest(w http.ResponseWriter, rawURL string) bool {
 	u, err := url.Parse(rawURL)
 	if err != nil {
 		return false
 	}
 
-	resp, err := artifactClient.Get(u.String())
+	resp, err := a.Client.Get(u.String())
 	if err != nil {
 		httperrors.Serve500(w)
 		return true
@@ -61,7 +60,7 @@ func MakeRequest(w http.ResponseWriter, rawURL string) bool {
 	w.Header().Set("Cache-Control", "max-age=600")
 	w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10))
 	io.Copy(w, resp.Body)
-	return true // TODO: returning false here will call the logic to fall through, which may not be a good idea.
+	return true
 }
 
 // URLBody returns a string that represents the body of a url an
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index ef3abe95a..b9a5265ca 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -17,7 +18,10 @@ func TestMakeRequest(t *testing.T) {
 	defer testServer.Close()
 
 	result := httptest.NewRecorder()
-	MakeRequest(result, testServer.URL)
+	art := &Artifact{
+		Client: &http.Client{Timeout: time.Second * time.Duration(1)},
+	}
+	art.MakeRequest(result, testServer.URL)
 
 	assert.Equal(t, result.Header().Get("Content-Type"), "text/html; charset=utf-8")
 	assert.Equal(t, result.Header().Get("Content-Length"), "90")
@@ -30,7 +34,10 @@ func TestArtifactMakeRequest_bad_request(t *testing.T) {
 	defer testServer.Close()
 
 	result := httptest.NewRecorder()
-	MakeRequest(result, testServer.URL)
+	art := &Artifact{
+		Client: &http.Client{Timeout: time.Second * time.Duration(1)},
+	}
+	art.MakeRequest(result, testServer.URL)
 
 	assert.Equal(t, http.StatusInternalServerError, result.Code)
 }
-- 
GitLab


From b3d2871545a7a11b20d56dcc5c607b7cb9bfb4a4 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Mon, 4 Sep 2017 23:02:09 -0700
Subject: [PATCH 20/39] add Serve502 function to the internal/httperrors
 package and create tests accordingly. Updating to internal/artifact to serve
 newly create 502 to represent bad gateway in the event of a failed proxy,
 update tests accordingly. Update app.go and app_test.go to change name of
 shouldProxy method to shouldProxyViaArtifact. Create new tests within
 accceptance_tests.go that ensure a 404 or 502 is return where applicable
 while attempting to proxy a request via an artifact. #78

---
 acceptance_test.go                     | 23 ++++++++++++++++++++++-
 app.go                                 |  4 ++--
 app_test.go                            |  4 ++--
 internal/artifact/artifact.go          | 21 ++++++++++++---------
 internal/artifact/artifact_test.go     |  2 +-
 internal/httperrors/httperrors.go      | 20 +++++++++++++++++---
 internal/httperrors/httperrors_test.go | 12 ++++++++++++
 7 files changed, 68 insertions(+), 18 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index 6a73dae75..bb243778a 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -219,7 +219,6 @@ func TestProxyRequest(t *testing.T) {
 		fmt.Fprint(w, content)
 	}))
 	defer testServer.Close()
-	skipUnlessEnabled(t)
 	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL, "-pages-domain=gitlab.io")
 	defer teardown()
 
@@ -233,3 +232,25 @@ func TestProxyRequest(t *testing.T) {
 	assert.Equal(t, contentLength, resp.ContentLength)
 	assert.Equal(t, "max-age=600", resp.Header.Get("Cache-Control"))
 }
+
+func TestProxyRequest_404(t *testing.T) {
+	// falls through artifact logic in the event that it is unable to proxy the request.
+	// In this case returning a 404 as the pages domain does not match artifact~1~2.gitlab.io
+	skipUnlessEnabled(t)
+	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+"http://www.example.com", "-pages-domain=")
+	defer teardown()
+
+	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab.io", "index.html")
+	require.NoError(t, err)
+	assert.Equal(t, http.StatusNotFound, resp.StatusCode)
+}
+
+func TestProxyRequest_502(t *testing.T) {
+	// This test attempts to make a requst to a bad address and ultimately fails returning 502
+	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+"bad address", "-pages-domain=gitlab.io")
+	defer teardown()
+
+	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab.io", "index.html")
+	require.NoError(t, err)
+	assert.Equal(t, http.StatusBadGateway, resp.StatusCode)
+}
diff --git a/app.go b/app.go
index 44043cf28..4e6c03944 100644
--- a/app.go
+++ b/app.go
@@ -54,7 +54,7 @@ func (a *theApp) ServeTLS(ch *tls.ClientHelloInfo) (*tls.Certificate, error) {
 	return nil, nil
 }
 
-func (a *theApp) shouldProxy(host string) bool {
+func (a *theApp) shouldProxyViaArtifact(host string) bool {
 	return (a.Artifact != nil) && (a.Artifact.Server != nil) && (a.Artifact.Server.String() != "") && a.Artifact.Pattern.MatchString(host)
 }
 
@@ -89,7 +89,7 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 
 	// In the event a host is prefixed with the artifact prefix an artifact
 	// value is created, and an attempt to proxy the request is made
-	if a.shouldProxy(host) {
+	if a.shouldProxyViaArtifact(host) {
 		urlString := fmt.Sprintf("%v%v%v", a.Artifact.Server.String(), artifact.URLBody(host), r.URL.Path)
 		if a.Artifact.MakeRequest(&w, urlString) {
 			return
diff --git a/app_test.go b/app_test.go
index fa440b66a..c48414b16 100644
--- a/app_test.go
+++ b/app_test.go
@@ -9,7 +9,7 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
 )
 
-func TestShouldProxy(t *testing.T) {
+func TestShouldProxyViaArtifact(t *testing.T) {
 	cases := []struct {
 		ArtifactServer string
 		Host           string
@@ -49,6 +49,6 @@ func TestShouldProxy(t *testing.T) {
 		}
 
 		app := &theApp{appConfig: appConfig{Domain: c.PagesDomain, Artifact: art}}
-		assert.Equal(t, c.Expected, app.shouldProxy(c.Host), c.Description)
+		assert.Equal(t, c.Expected, app.shouldProxyViaArtifact(c.Host), c.Description)
 	}
 }
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index de93737d4..986dcfac5 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -28,7 +28,7 @@ type Artifact struct {
 	Pattern *regexp.Regexp
 }
 
-// New when provided the arguments defined herein, retruns a pointer to an
+// New when provided the arguments defined herein, returns a pointer to an
 // Artifact that is used to proxy requests.
 func New(u *url.URL, timeout int, pagesDomain string) *Artifact {
 	return &Artifact{
@@ -39,11 +39,10 @@ func New(u *url.URL, timeout int, pagesDomain string) *Artifact {
 
 }
 
-// MakeRequest given an http.ResponseWriter will proxy a request to the
-// underlying url.URL pointer's address on the related Artifact pointer. Making
-// a GET request, it then maps the Content-Type, Content-Length, and response
-// body from said GET request to the http.ResponseWriter. A 500 will be
-// served in the event that an error is encountered.
+// MakeRequest given an http.ResponseWriter and a corresponding url as a string
+// will make a request to the argument url. It does this using the http.Client
+// embeded in the calling Artifact pointer. Returning a bool that represents if
+// the http.ResponseWriter is written to in any capacity.
 func (a *Artifact) MakeRequest(w http.ResponseWriter, rawURL string) bool {
 	u, err := url.Parse(rawURL)
 	if err != nil {
@@ -52,7 +51,7 @@ func (a *Artifact) MakeRequest(w http.ResponseWriter, rawURL string) bool {
 
 	resp, err := a.Client.Get(u.String())
 	if err != nil {
-		httperrors.Serve500(w)
+		httperrors.Serve502(w)
 		return true
 	}
 
@@ -63,8 +62,8 @@ func (a *Artifact) MakeRequest(w http.ResponseWriter, rawURL string) bool {
 	return true
 }
 
-// URLBody returns a string that represents the body of a url an
-// artifact will ultimately proxy a request to. An expected host argument would
+// URLBody returns a string that represents the body of a url an Artifact{}
+// will ultimately proxy a request to. An expected host argument would
 // be close to "artifact~1~2". 1 and 2 represent Id's that need to be extracted
 // and interpolated into the const baseURL.
 func URLBody(host string) string {
@@ -77,6 +76,10 @@ func URLBody(host string) string {
 	return fmt.Sprintf(baseURL, strippedIds[0], strippedIds[1])
 }
 
+// hostPatternGen returns a pointer to a regexp.Regexp that is made up of
+// the constant hostPattern and the argument which represents the pages domain.
+// This is used to ensure that the requested page meets not only the hostPattern
+// requirements, but is suffixed with the proper pagesDomain.
 func hostPatternGen(pagesDomain string) *regexp.Regexp {
 	return regexp.MustCompile(fmt.Sprintf(`%s\.%s\z`, hostPattern, pagesDomain))
 }
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index b9a5265ca..36b3c048a 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -39,7 +39,7 @@ func TestArtifactMakeRequest_bad_request(t *testing.T) {
 	}
 	art.MakeRequest(result, testServer.URL)
 
-	assert.Equal(t, http.StatusInternalServerError, result.Code)
+	assert.Equal(t, http.StatusBadGateway, result.Code)
 }
 
 func TestURLBody(t *testing.T) {
diff --git a/internal/httperrors/httperrors.go b/internal/httperrors/httperrors.go
index fbdc240f9..82d43fc54 100644
--- a/internal/httperrors/httperrors.go
+++ b/internal/httperrors/httperrors.go
@@ -20,8 +20,8 @@ var (
 		"404",
 		"The page you're looking for could not be found.",
 		`<p>The resource that you are attempting to access does not exist or you don't have the necessary permissions to view it.</p>
-                <p>Make sure the address is correct and that the page hasn't moved.</p>
-                <p>Please contact your GitLab administrator if you think this is a mistake.</p>`,
+     <p>Make sure the address is correct and that the page hasn't moved.</p>
+     <p>Please contact your GitLab administrator if you think this is a mistake.</p>`,
 	}
 	content500 = content{
 		http.StatusInternalServerError,
@@ -29,7 +29,16 @@ var (
 		"500",
 		"Whoops, something went wrong on our end.",
 		`<p>Try refreshing the page, or going back and attempting the action again.</p>
-                <p>Please contact your GitLab administrator if this problem persists.</p>`,
+     <p>Please contact your GitLab administrator if this problem persists.</p>`,
+	}
+
+	content502 = content{
+		http.StatusBadGateway,
+		"Something went wrong (502)",
+		"502",
+		"Whoops, something went wrong on our end.",
+		`<p>Try refreshing the page, or going back and attempting the action again.</p>
+     <p>Please contact your GitLab administrator if this problem persists.</p>`,
 	}
 )
 
@@ -146,3 +155,8 @@ func Serve404(w http.ResponseWriter) {
 func Serve500(w http.ResponseWriter) {
 	serveErrorPage(w, content500)
 }
+
+// Serve502 returns a 502 error response / HTML page to the http.ResponseWriter
+func Serve502(w http.ResponseWriter) {
+	serveErrorPage(w, content502)
+}
diff --git a/internal/httperrors/httperrors_test.go b/internal/httperrors/httperrors_test.go
index 47ea31a7e..1a79d8506 100644
--- a/internal/httperrors/httperrors_test.go
+++ b/internal/httperrors/httperrors_test.go
@@ -91,3 +91,15 @@ func TestServe500(t *testing.T) {
 	assert.Contains(t, w.Content(), content500.header)
 	assert.Contains(t, w.Content(), content500.subHeader)
 }
+
+func TestServe502(t *testing.T) {
+	w := newTestResponseWriter(httptest.NewRecorder())
+	Serve502(w)
+	assert.Equal(t, w.Header().Get("Content-Type"), "text/html; charset=utf-8")
+	assert.Equal(t, w.Header().Get("X-Content-Type-Options"), "nosniff")
+	assert.Equal(t, w.Status(), content502.status)
+	assert.Contains(t, w.Content(), content502.title)
+	assert.Contains(t, w.Content(), content502.statusString)
+	assert.Contains(t, w.Content(), content502.header)
+	assert.Contains(t, w.Content(), content502.subHeader)
+}
-- 
GitLab


From a66e1b0be01ce1f0853460baf24c1306c6c591ca Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Mon, 4 Sep 2017 23:14:35 -0700
Subject: [PATCH 21/39] add skipUnlessEnabled to TestProxyRequest_502 within
 acceptance_test.go. Change within artifact.go to copy the response code of
 the proxied request, adjustment to tests to reflect this. #78

---
 acceptance_test.go                 | 1 +
 internal/artifact/artifact.go      | 1 +
 internal/artifact/artifact_test.go | 1 +
 3 files changed, 3 insertions(+)

diff --git a/acceptance_test.go b/acceptance_test.go
index bb243778a..936639458 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -247,6 +247,7 @@ func TestProxyRequest_404(t *testing.T) {
 
 func TestProxyRequest_502(t *testing.T) {
 	// This test attempts to make a requst to a bad address and ultimately fails returning 502
+	skipUnlessEnabled(t)
 	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+"bad address", "-pages-domain=gitlab.io")
 	defer teardown()
 
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 986dcfac5..8c8353f4a 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -55,6 +55,7 @@ func (a *Artifact) MakeRequest(w http.ResponseWriter, rawURL string) bool {
 		return true
 	}
 
+	w.WriteHeader(resp.StatusCode)
 	w.Header().Set("Content-Type", resp.Header.Get("Content-Type"))
 	w.Header().Set("Cache-Control", "max-age=600")
 	w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10))
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index 36b3c048a..76f3bcfee 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -27,6 +27,7 @@ func TestMakeRequest(t *testing.T) {
 	assert.Equal(t, result.Header().Get("Content-Length"), "90")
 	assert.Equal(t, result.Header().Get("Cache-Control"), "max-age=600")
 	assert.Equal(t, string(result.Body.Bytes()), "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
+	assert.Equal(t, http.StatusOK, result.Code)
 }
 
 func TestArtifactMakeRequest_bad_request(t *testing.T) {
-- 
GitLab


From 79d1e256f71c4a904ebf921a19c778238d344e40 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Tue, 5 Sep 2017 08:40:03 -0700
Subject: [PATCH 22/39] small updates to acceptance_test.go. Additional updates
 within internal/artifact/artifact.go to set response code after setting all
 headers. #78

---
 acceptance_test.go            | 32 +++++++++++++++++++++++---------
 internal/artifact/artifact.go |  2 +-
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index 936639458..eeb04e842 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -219,14 +219,14 @@ func TestProxyRequest(t *testing.T) {
 		fmt.Fprint(w, content)
 	}))
 	defer testServer.Close()
-	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL, "-pages-domain=gitlab.io")
+	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL)
 	defer teardown()
 
-	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab.io", "index.html")
+	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab-example.com", "index.html")
 	require.NoError(t, err)
-	defer resp.Body.Close()
 	body, err := ioutil.ReadAll(resp.Body)
 	require.NoError(t, err)
+	defer resp.Body.Close()
 	assert.Equal(t, string(body), content)
 	assert.Equal(t, http.StatusOK, resp.StatusCode)
 	assert.Equal(t, contentLength, resp.ContentLength)
@@ -234,24 +234,38 @@ func TestProxyRequest(t *testing.T) {
 }
 
 func TestProxyRequest_404(t *testing.T) {
-	// falls through artifact logic in the event that it is unable to proxy the request.
-	// In this case returning a 404 as the pages domain does not match artifact~1~2.gitlab.io
+	// In the event the server responds with a 404 reponse we want to ensure
+	// that said response code is also proxied in our response.
 	skipUnlessEnabled(t)
-	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+"http://www.example.com", "-pages-domain=")
+	content := "<!DOCTYPE html><html><head><title>404 Not Found</title></head><body></body></html>"
+	contentLength := int64(len(content))
+	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/html; charset=utf-8")
+		w.WriteHeader(http.StatusNotFound)
+		fmt.Fprint(w, content)
+	}))
+	defer testServer.Close()
+	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL)
 	defer teardown()
 
-	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab.io", "index.html")
+	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab-example.com", "index.html")
+	require.NoError(t, err)
+	body, err := ioutil.ReadAll(resp.Body)
 	require.NoError(t, err)
+	defer resp.Body.Close()
+	assert.Equal(t, string(body), content)
 	assert.Equal(t, http.StatusNotFound, resp.StatusCode)
+	assert.Equal(t, contentLength, resp.ContentLength)
+	assert.Equal(t, "max-age=600", resp.Header.Get("Cache-Control"))
 }
 
 func TestProxyRequest_502(t *testing.T) {
 	// This test attempts to make a requst to a bad address and ultimately fails returning 502
 	skipUnlessEnabled(t)
-	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+"bad address", "-pages-domain=gitlab.io")
+	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+"bad address")
 	defer teardown()
 
-	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab.io", "index.html")
+	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab-example.com", "index.html")
 	require.NoError(t, err)
 	assert.Equal(t, http.StatusBadGateway, resp.StatusCode)
 }
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 8c8353f4a..30abbe187 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -55,10 +55,10 @@ func (a *Artifact) MakeRequest(w http.ResponseWriter, rawURL string) bool {
 		return true
 	}
 
-	w.WriteHeader(resp.StatusCode)
 	w.Header().Set("Content-Type", resp.Header.Get("Content-Type"))
 	w.Header().Set("Cache-Control", "max-age=600")
 	w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10))
+	w.WriteHeader(resp.StatusCode)
 	io.Copy(w, resp.Body)
 	return true
 }
-- 
GitLab


From b2c00a9d60bbfc7e5b973621cdd71c955efca2e6 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Tue, 5 Sep 2017 19:31:58 -0700
Subject: [PATCH 23/39] small changes to acceptance tests #78

---
 acceptance_test.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/acceptance_test.go b/acceptance_test.go
index eeb04e842..bedfc6bde 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -267,5 +267,6 @@ func TestProxyRequest_502(t *testing.T) {
 
 	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab-example.com", "index.html")
 	require.NoError(t, err)
+	defer resp.Body.Close()
 	assert.Equal(t, http.StatusBadGateway, resp.StatusCode)
 }
-- 
GitLab


From 43efb77adfa252912d347f71153cdb002ef7fdee Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Tue, 5 Sep 2017 21:10:12 -0700
Subject: [PATCH 24/39] Creation of new method of called TryMakeRequest on the
 Artifact type, along with a buildURL method, along with some corresponding
 test. Used this newly created TryMakeRequest to refactor the app.go file. #78

---
 app.go                             |  13 +--
 app_test.go                        |  54 -----------
 internal/artifact/artifact.go      |  45 ++++++++-
 internal/artifact/artifact_test.go | 141 +++++++++++++++++++++--------
 4 files changed, 149 insertions(+), 104 deletions(-)
 delete mode 100644 app_test.go

diff --git a/app.go b/app.go
index 4e6c03944..68448e532 100644
--- a/app.go
+++ b/app.go
@@ -2,7 +2,6 @@ package main
 
 import (
 	"crypto/tls"
-	"fmt"
 	"log"
 	"net"
 	"net/http"
@@ -15,7 +14,6 @@ import (
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/rs/cors"
 
-	"gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
 	"gitlab.com/gitlab-org/gitlab-pages/metrics"
 )
@@ -54,10 +52,6 @@ func (a *theApp) ServeTLS(ch *tls.ClientHelloInfo) (*tls.Certificate, error) {
 	return nil, nil
 }
 
-func (a *theApp) shouldProxyViaArtifact(host string) bool {
-	return (a.Artifact != nil) && (a.Artifact.Server != nil) && (a.Artifact.Server.String() != "") && a.Artifact.Pattern.MatchString(host)
-}
-
 func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https bool) {
 	w := newLoggingResponseWriter(ww)
 	defer w.Log(r)
@@ -89,11 +83,8 @@ func (a *theApp) serveContent(ww http.ResponseWriter, r *http.Request, https boo
 
 	// In the event a host is prefixed with the artifact prefix an artifact
 	// value is created, and an attempt to proxy the request is made
-	if a.shouldProxyViaArtifact(host) {
-		urlString := fmt.Sprintf("%v%v%v", a.Artifact.Server.String(), artifact.URLBody(host), r.URL.Path)
-		if a.Artifact.MakeRequest(&w, urlString) {
-			return
-		}
+	if a.Artifact.TryMakeRequest(host, &w, r) {
+		return
 	}
 
 	domain := a.domain(host)
diff --git a/app_test.go b/app_test.go
deleted file mode 100644
index c48414b16..000000000
--- a/app_test.go
+++ /dev/null
@@ -1,54 +0,0 @@
-package main
-
-import (
-	"net/url"
-	"regexp"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
-)
-
-func TestShouldProxyViaArtifact(t *testing.T) {
-	cases := []struct {
-		ArtifactServer string
-		Host           string
-		PagesDomain    string
-		Expected       bool
-		Description    string
-	}{
-		{
-			"https://gitlab.com/api/v4",
-			"artifact~1~2.gitlab.io",
-			"gitlab.io",
-			true,
-			"basic case",
-		},
-		{
-			"",
-			"artifact~1~2.gitlab.io",
-			"gitlab.io",
-			false,
-			"Artifact server is not provided",
-		},
-		{
-			"https://gitlab.com/api/v4",
-			"artifact~1~2.gitlab.io",
-			"someothersite.io",
-			false,
-			"host does not match the expected pattern",
-		},
-	}
-
-	for _, c := range cases {
-		URL, err := url.Parse(c.ArtifactServer)
-		assert.NoError(t, err)
-		art := &artifact.Artifact{
-			Server:  URL,
-			Pattern: regexp.MustCompile(c.PagesDomain),
-		}
-
-		app := &theApp{appConfig: appConfig{Domain: c.PagesDomain, Artifact: art}}
-		assert.Equal(t, c.Expected, app.shouldProxyViaArtifact(c.Host), c.Description)
-	}
-}
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 30abbe187..6b51a7d01 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -63,20 +63,59 @@ func (a *Artifact) MakeRequest(w http.ResponseWriter, rawURL string) bool {
 	return true
 }
 
+func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Request) bool {
+	if a == nil || a.Server == nil || a.Server.String() == "" || !a.Pattern.MatchString(host) {
+		return false
+	}
+
+	reqURL, ok := a.buildURL(host, r.URL.Path)
+	if !ok {
+		return false
+	}
+
+	resp, err := a.Client.Get(reqURL.String())
+	if err != nil {
+		httperrors.Serve502(w)
+		return true
+	}
+
+	w.Header().Set("Content-Type", resp.Header.Get("Content-Type"))
+	w.Header().Set("Cache-Control", "max-age=600")
+	w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10))
+	w.WriteHeader(resp.StatusCode)
+	io.Copy(w, resp.Body)
+	return true
+}
+
 // URLBody returns a string that represents the body of a url an Artifact{}
 // will ultimately proxy a request to. An expected host argument would
 // be close to "artifact~1~2". 1 and 2 represent Id's that need to be extracted
 // and interpolated into the const baseURL.
 func URLBody(host string) string {
 	ids := hostPattern.FindAllStringSubmatch(host, -1)
-	strippedIds := make([]string, 2)
 
-	if len(ids) > 0 {
-		strippedIds[0], strippedIds[1] = ids[0][1], ids[0][2]
+	if len(ids) < 1 {
+		return ""
 	}
+
+	strippedIds := make([]string, 2)
+	strippedIds[0], strippedIds[1] = ids[0][1], ids[0][2]
 	return fmt.Sprintf(baseURL, strippedIds[0], strippedIds[1])
 }
 
+func (a *Artifact) buildURL(host, path string) (*url.URL, bool) {
+	body := URLBody(host)
+	if body == "" {
+		return nil, false
+	}
+
+	u, err := url.Parse(fmt.Sprintf("%s%s%s", a.Server.String(), body, path))
+	if err != nil {
+		return nil, false
+	}
+	return u, true
+}
+
 // hostPatternGen returns a pointer to a regexp.Regexp that is made up of
 // the constant hostPattern and the argument which represents the pages domain.
 // This is used to ensure that the requested page meets not only the hostPattern
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index 76f3bcfee..debd6cedf 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -1,47 +1,69 @@
 package artifact
 
 import (
-	"fmt"
-	"net/http"
-	"net/http/httptest"
+	"net/url"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 )
 
-func TestMakeRequest(t *testing.T) {
-	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Set("Content-Type", "text/html; charset=utf-8")
-		fmt.Fprint(w, "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
-	}))
-	defer testServer.Close()
+// func TestMakeRequest(t *testing.T) {
+// 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+// 		w.Header().Set("Content-Type", "text/html; charset=utf-8")
+// 		fmt.Fprint(w, "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
+// 	}))
+// 	defer testServer.Close()
 
-	result := httptest.NewRecorder()
-	art := &Artifact{
-		Client: &http.Client{Timeout: time.Second * time.Duration(1)},
-	}
-	art.MakeRequest(result, testServer.URL)
+// 	result := httptest.NewRecorder()
+// 	art := &Artifact{
+// 		Client: &http.Client{Timeout: time.Second * time.Duration(1)},
+// 	}
+// 	art.MakeRequest(result, testServer.URL)
 
-	assert.Equal(t, result.Header().Get("Content-Type"), "text/html; charset=utf-8")
-	assert.Equal(t, result.Header().Get("Content-Length"), "90")
-	assert.Equal(t, result.Header().Get("Cache-Control"), "max-age=600")
-	assert.Equal(t, string(result.Body.Bytes()), "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
-	assert.Equal(t, http.StatusOK, result.Code)
-}
+// 	assert.Equal(t, result.Header().Get("Content-Type"), "text/html; charset=utf-8")
+// 	assert.Equal(t, result.Header().Get("Content-Length"), "90")
+// 	assert.Equal(t, result.Header().Get("Cache-Control"), "max-age=600")
+// 	assert.Equal(t, string(result.Body.Bytes()), "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
+// 	assert.Equal(t, http.StatusOK, result.Code)
+// }
 
-func TestArtifactMakeRequest_bad_request(t *testing.T) {
-	testServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
-	defer testServer.Close()
+// func TestArtifactMakeRequest_bad_request(t *testing.T) {
+// 	testServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+// 	defer testServer.Close()
 
-	result := httptest.NewRecorder()
-	art := &Artifact{
-		Client: &http.Client{Timeout: time.Second * time.Duration(1)},
-	}
-	art.MakeRequest(result, testServer.URL)
+// 	result := httptest.NewRecorder()
+// 	art := &Artifact{
+// 		Client: &http.Client{Timeout: time.Second * time.Duration(1)},
+// 	}
+// 	art.MakeRequest(result, testServer.URL)
 
-	assert.Equal(t, http.StatusBadGateway, result.Code)
-}
+// 	assert.Equal(t, http.StatusBadGateway, result.Code)
+// }
+
+// func TestTryMakeRequest(t *testing.T) {
+// 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+// 		w.Header().Set("Content-Type", "text/html; charset=utf-8")
+// 		fmt.Fprint(w, "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
+// 	}))
+// 	defer testServer.Close()
+
+// 	requestURL, err := url.Parse()
+// 	server, err := url.Parse(testServer.URL)
+// 	assert.NoError(t, err)
+// 	a := &Artifact{
+// 		Server:  server,
+// 		Client:  &http.Client{Timeout: time.Second * time.Duration(1)},
+// 		Pattern: regexp.MustCompile("host"),
+// 	}
+// 	result := httptest.NewRecorder()
+// 	a.TryMakeRequest("host", result, testServer.URL)
+
+// 	assert.Equal(t, result.Header().Get("Content-Type"), "text/html; charset=utf-8")
+// 	assert.Equal(t, result.Header().Get("Content-Length"), "90")
+// 	assert.Equal(t, result.Header().Get("Cache-Control"), "max-age=600")
+// 	assert.Equal(t, string(result.Body.Bytes()), "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
+// 	assert.Equal(t, http.StatusOK, result.Code)
+// }
 
 func TestURLBody(t *testing.T) {
 	cases := []struct {
@@ -66,22 +88,22 @@ func TestURLBody(t *testing.T) {
 		},
 		{
 			"artifact~one~two",
-			"/projects//jobs//artifacts",
+			"",
 			"letters rather than numbers",
 		},
 		{
 			"artifact~One111~tWo222",
-			"/projects//jobs//artifacts",
+			"",
 			"Mixture of alphanumeric",
 		},
 		{
 			"artifact~!@#$%~%$#@!",
-			"/projects//jobs//artifacts",
+			"",
 			"special characters",
 		},
 		{
 			"artifact~~",
-			"/projects//jobs//artifacts",
+			"",
 			"no ids",
 		},
 		{
@@ -91,7 +113,7 @@ func TestURLBody(t *testing.T) {
 		},
 		{
 			"",
-			"/projects//jobs//artifacts",
+			"",
 			"empty host",
 		},
 		{
@@ -107,6 +129,53 @@ func TestURLBody(t *testing.T) {
 	}
 }
 
+func TestBuildURL(t *testing.T) {
+	cases := []struct {
+		RawServer   string
+		Host        string
+		Path        string
+		Expected    string
+		Ok          bool
+		Description string
+	}{
+		{
+			"https://gitlab.com/api/v4",
+			"artifact~1~2.gitlab.io",
+			"/path/to/file.txt",
+			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/path/to/file.txt",
+			true,
+			"basic case",
+		},
+		{
+			"https://gitlab.com/api/v4",
+			"artifact~100000~200000.gitlab.io",
+			"/file.txt",
+			"https://gitlab.com/api/v4/projects/100000/jobs/200000/artifacts/file.txt",
+			true,
+			"expanded case",
+		},
+		{
+			"",
+			"artifact~A~B.gitlab.io",
+			"",
+			"",
+			false,
+			"un-parseable Host",
+		},
+	}
+
+	for _, c := range cases {
+		server, err := url.Parse(c.RawServer)
+		assert.NoError(t, err)
+		a := &Artifact{Server: server}
+		u, ok := a.buildURL(c.Host, c.Path)
+		assert.Equal(t, c.Ok, ok, c.Description)
+		if c.Ok {
+			assert.Equal(t, c.Expected, u.String(), c.Description)
+		}
+	}
+}
+
 func TestMatchHostGen(t *testing.T) {
 	cases := []struct {
 		URLHost     string
-- 
GitLab


From f8523e4ccde0d5b44010ca7ef95d104d9386b009 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Tue, 5 Sep 2017 22:02:00 -0700
Subject: [PATCH 25/39] update buildURL function and adjust commenting
 throughout the artifacts package. Adjust the artifact_test.go package to test
 the TryMakeRequest method. #78

---
 internal/artifact/artifact.go      | 59 +++++++-------------
 internal/artifact/artifact_test.go | 88 +++++++++++-------------------
 2 files changed, 50 insertions(+), 97 deletions(-)

diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 6b51a7d01..211ba2227 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -39,30 +39,9 @@ func New(u *url.URL, timeout int, pagesDomain string) *Artifact {
 
 }
 
-// MakeRequest given an http.ResponseWriter and a corresponding url as a string
-// will make a request to the argument url. It does this using the http.Client
-// embeded in the calling Artifact pointer. Returning a bool that represents if
-// the http.ResponseWriter is written to in any capacity.
-func (a *Artifact) MakeRequest(w http.ResponseWriter, rawURL string) bool {
-	u, err := url.Parse(rawURL)
-	if err != nil {
-		return false
-	}
-
-	resp, err := a.Client.Get(u.String())
-	if err != nil {
-		httperrors.Serve502(w)
-		return true
-	}
-
-	w.Header().Set("Content-Type", resp.Header.Get("Content-Type"))
-	w.Header().Set("Cache-Control", "max-age=600")
-	w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10))
-	w.WriteHeader(resp.StatusCode)
-	io.Copy(w, resp.Body)
-	return true
-}
-
+// TryMakeRequest will attempt to proxy a request and write it to the argument
+// http.ResponseWriter, ultimately returning a bool that indicates if the
+// http.ResponseWriter has been written to in any capacity.
 func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Request) bool {
 	if a == nil || a.Server == nil || a.Server.String() == "" || !a.Pattern.MatchString(host) {
 		return false
@@ -87,11 +66,24 @@ func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Re
 	return true
 }
 
-// URLBody returns a string that represents the body of a url an Artifact{}
+func (a *Artifact) buildURL(host, path string) (*url.URL, bool) {
+	body := urlBody(host)
+	if body == "" {
+		return nil, false
+	}
+
+	u, err := url.Parse(fmt.Sprintf("%s%s%s", a.Server.String(), body, path))
+	if err != nil {
+		return nil, false
+	}
+	return u, true
+}
+
+// urlBody returns a string that represents the body of a url an Artifact{}
 // will ultimately proxy a request to. An expected host argument would
 // be close to "artifact~1~2". 1 and 2 represent Id's that need to be extracted
 // and interpolated into the const baseURL.
-func URLBody(host string) string {
+func urlBody(host string) string {
 	ids := hostPattern.FindAllStringSubmatch(host, -1)
 
 	if len(ids) < 1 {
@@ -103,23 +95,10 @@ func URLBody(host string) string {
 	return fmt.Sprintf(baseURL, strippedIds[0], strippedIds[1])
 }
 
-func (a *Artifact) buildURL(host, path string) (*url.URL, bool) {
-	body := URLBody(host)
-	if body == "" {
-		return nil, false
-	}
-
-	u, err := url.Parse(fmt.Sprintf("%s%s%s", a.Server.String(), body, path))
-	if err != nil {
-		return nil, false
-	}
-	return u, true
-}
-
 // hostPatternGen returns a pointer to a regexp.Regexp that is made up of
 // the constant hostPattern and the argument which represents the pages domain.
 // This is used to ensure that the requested page meets not only the hostPattern
 // requirements, but is suffixed with the proper pagesDomain.
 func hostPatternGen(pagesDomain string) *regexp.Regexp {
-	return regexp.MustCompile(fmt.Sprintf(`%s\.%s\z`, hostPattern, pagesDomain))
+	return regexp.MustCompile(fmt.Sprintf(`%s\.%s\z`, hostPattern, regexp.QuoteMeta(pagesDomain)))
 }
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index debd6cedf..7fbee670b 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -1,69 +1,43 @@
 package artifact
 
 import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"net/url"
+	"regexp"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
 
-// func TestMakeRequest(t *testing.T) {
-// 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-// 		w.Header().Set("Content-Type", "text/html; charset=utf-8")
-// 		fmt.Fprint(w, "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
-// 	}))
-// 	defer testServer.Close()
-
-// 	result := httptest.NewRecorder()
-// 	art := &Artifact{
-// 		Client: &http.Client{Timeout: time.Second * time.Duration(1)},
-// 	}
-// 	art.MakeRequest(result, testServer.URL)
-
-// 	assert.Equal(t, result.Header().Get("Content-Type"), "text/html; charset=utf-8")
-// 	assert.Equal(t, result.Header().Get("Content-Length"), "90")
-// 	assert.Equal(t, result.Header().Get("Cache-Control"), "max-age=600")
-// 	assert.Equal(t, string(result.Body.Bytes()), "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
-// 	assert.Equal(t, http.StatusOK, result.Code)
-// }
-
-// func TestArtifactMakeRequest_bad_request(t *testing.T) {
-// 	testServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
-// 	defer testServer.Close()
-
-// 	result := httptest.NewRecorder()
-// 	art := &Artifact{
-// 		Client: &http.Client{Timeout: time.Second * time.Duration(1)},
-// 	}
-// 	art.MakeRequest(result, testServer.URL)
-
-// 	assert.Equal(t, http.StatusBadGateway, result.Code)
-// }
-
-// func TestTryMakeRequest(t *testing.T) {
-// 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-// 		w.Header().Set("Content-Type", "text/html; charset=utf-8")
-// 		fmt.Fprint(w, "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
-// 	}))
-// 	defer testServer.Close()
-
-// 	requestURL, err := url.Parse()
-// 	server, err := url.Parse(testServer.URL)
-// 	assert.NoError(t, err)
-// 	a := &Artifact{
-// 		Server:  server,
-// 		Client:  &http.Client{Timeout: time.Second * time.Duration(1)},
-// 		Pattern: regexp.MustCompile("host"),
-// 	}
-// 	result := httptest.NewRecorder()
-// 	a.TryMakeRequest("host", result, testServer.URL)
+func TestTryMakeRequest(t *testing.T) {
+	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/html; charset=utf-8")
+		fmt.Fprint(w, "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
+	}))
+	defer testServer.Close()
+
+	result := httptest.NewRecorder()
+	reqURL, err := url.Parse("/index.html")
+	assert.NoError(t, err)
+	r := &http.Request{URL: reqURL}
+	serverUrl, err := url.Parse(testServer.URL)
+	assert.NoError(t, err)
+	art := &Artifact{
+		Server:  serverUrl,
+		Client:  &http.Client{Timeout: time.Second * time.Duration(1)},
+		Pattern: regexp.MustCompile("artifact~1~2"),
+	}
 
-// 	assert.Equal(t, result.Header().Get("Content-Type"), "text/html; charset=utf-8")
-// 	assert.Equal(t, result.Header().Get("Content-Length"), "90")
-// 	assert.Equal(t, result.Header().Get("Cache-Control"), "max-age=600")
-// 	assert.Equal(t, string(result.Body.Bytes()), "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
-// 	assert.Equal(t, http.StatusOK, result.Code)
-// }
+	assert.True(t, art.TryMakeRequest("artifact~1~2", result, r))
+	assert.Equal(t, "text/html; charset=utf-8", result.Header().Get("Content-Type"))
+	assert.Equal(t, "90", result.Header().Get("Content-Length"))
+	assert.Equal(t, "max-age=600", result.Header().Get("Cache-Control"))
+	assert.Equal(t, string(result.Body.Bytes()), "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
+	assert.Equal(t, http.StatusOK, result.Code)
+}
 
 func TestURLBody(t *testing.T) {
 	cases := []struct {
@@ -124,7 +98,7 @@ func TestURLBody(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		actual := URLBody(c.URLHost)
+		actual := urlBody(c.URLHost)
 		assert.Equal(t, c.Expected, actual, c.Description)
 	}
 }
-- 
GitLab


From f9cb5547882fd856f7558856615fa02b0e988d39 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Tue, 5 Sep 2017 22:12:46 -0700
Subject: [PATCH 26/39] change the nesting of the Artifact struct from being an
 attribute of appConfig to being an attribute of theApp. Change the runApp
 function accordingly. #78

---
 app.go        | 24 ++++++++++++++++++++++--
 app_config.go |  4 +---
 main.go       | 20 +-------------------
 3 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/app.go b/app.go
index 68448e532..d4883b6c3 100644
--- a/app.go
+++ b/app.go
@@ -5,6 +5,7 @@ import (
 	"log"
 	"net"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 	"sync"
@@ -14,6 +15,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/rs/cors"
 
+	"gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
 	"gitlab.com/gitlab-org/gitlab-pages/metrics"
 )
@@ -27,8 +29,9 @@ var (
 
 type theApp struct {
 	appConfig
-	domains domains
-	lock    sync.RWMutex
+	domains  domains
+	lock     sync.RWMutex
+	Artifact *artifact.Artifact
 }
 
 func (a *theApp) domain(host string) *domain {
@@ -181,5 +184,22 @@ func (a *theApp) Run() {
 
 func runApp(config appConfig) {
 	a := theApp{appConfig: config}
+
+	if config.ArtifactsServer != "" {
+		u, err := url.Parse(config.ArtifactsServer)
+		if err != nil {
+			log.Fatal(err)
+		}
+		// url.Parse ensures that the Scheme arttribute is always lower case.
+		if u.Scheme != "" && u.Scheme != "http" && u.Scheme != "https" {
+			log.Fatal(errArtifactSchemaUnsupported)
+		}
+
+		if *artifactsServerTimeout < 1 {
+			log.Fatal(errArtifactsServerTimeoutValue)
+		}
+
+		a.Artifact = artifact.New(u, *artifactsServerTimeout, *pagesDomain)
+	}
 	a.Run()
 }
diff --git a/app_config.go b/app_config.go
index caa0220a2..8ba983cc9 100644
--- a/app_config.go
+++ b/app_config.go
@@ -1,10 +1,8 @@
 package main
 
-import "gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
-
 type appConfig struct {
 	Domain          string
-	Artifact        *artifact.Artifact
+	ArtifactsServer string
 	RootCertificate []byte
 	RootKey         []byte
 
diff --git a/main.go b/main.go
index 233f6c266..fbb5c2e4b 100644
--- a/main.go
+++ b/main.go
@@ -4,11 +4,8 @@ import (
 	"errors"
 	"flag"
 	"log"
-	"net/url"
 	"os"
 	"strings"
-
-	"gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
 )
 
 // VERSION stores the information about the semantic version of application
@@ -53,22 +50,7 @@ func configFromFlags() appConfig {
 		config.RootKey = readFile(*pagesRootKey)
 	}
 
-	if *artifactsServer != "" {
-		u, err := url.Parse(*artifactsServer)
-		if err != nil {
-			log.Fatal(err)
-		}
-		// url.Parse ensures that the Scheme arttribute is always lower case.
-		if u.Scheme != "" && u.Scheme != "http" && u.Scheme != "https" {
-			log.Fatal(errArtifactSchemaUnsupported)
-		}
-
-		if *artifactsServerTimeout < 1 {
-			log.Fatal(errArtifactsServerTimeoutValue)
-		}
-
-		config.Artifact = artifact.New(u, *artifactsServerTimeout, *pagesDomain)
-	}
+	config.ArtifactsServer = *artifactsServer
 	return config
 }
 
-- 
GitLab


From d17cfd7ee03cc6302401292ef1762d00ee10d8fd Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Tue, 5 Sep 2017 22:16:35 -0700
Subject: [PATCH 27/39] unexported the attributes within the internal/artifact
 struct. #78

---
 internal/artifact/artifact.go      | 18 +++++++++---------
 internal/artifact/artifact_test.go |  8 ++++----
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 211ba2227..62e89fd16 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -23,18 +23,18 @@ var (
 // Artifact is a struct that is made up of a url.URL, http.Client, and
 // regexp.Regexp that is used to proxy requests where applicable.
 type Artifact struct {
-	Server  *url.URL
-	Client  *http.Client
-	Pattern *regexp.Regexp
+	server  *url.URL
+	client  *http.Client
+	pattern *regexp.Regexp
 }
 
 // New when provided the arguments defined herein, returns a pointer to an
 // Artifact that is used to proxy requests.
 func New(u *url.URL, timeout int, pagesDomain string) *Artifact {
 	return &Artifact{
-		Server:  u,
-		Client:  &http.Client{Timeout: time.Second * time.Duration(timeout)},
-		Pattern: hostPatternGen(pagesDomain),
+		server:  u,
+		client:  &http.Client{Timeout: time.Second * time.Duration(timeout)},
+		pattern: hostPatternGen(pagesDomain),
 	}
 
 }
@@ -43,7 +43,7 @@ func New(u *url.URL, timeout int, pagesDomain string) *Artifact {
 // http.ResponseWriter, ultimately returning a bool that indicates if the
 // http.ResponseWriter has been written to in any capacity.
 func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Request) bool {
-	if a == nil || a.Server == nil || a.Server.String() == "" || !a.Pattern.MatchString(host) {
+	if a == nil || a.server == nil || a.server.String() == "" || !a.pattern.MatchString(host) {
 		return false
 	}
 
@@ -52,7 +52,7 @@ func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Re
 		return false
 	}
 
-	resp, err := a.Client.Get(reqURL.String())
+	resp, err := a.client.Get(reqURL.String())
 	if err != nil {
 		httperrors.Serve502(w)
 		return true
@@ -72,7 +72,7 @@ func (a *Artifact) buildURL(host, path string) (*url.URL, bool) {
 		return nil, false
 	}
 
-	u, err := url.Parse(fmt.Sprintf("%s%s%s", a.Server.String(), body, path))
+	u, err := url.Parse(fmt.Sprintf("%s%s%s", a.server.String(), body, path))
 	if err != nil {
 		return nil, false
 	}
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index 7fbee670b..32e78a028 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -26,9 +26,9 @@ func TestTryMakeRequest(t *testing.T) {
 	serverUrl, err := url.Parse(testServer.URL)
 	assert.NoError(t, err)
 	art := &Artifact{
-		Server:  serverUrl,
-		Client:  &http.Client{Timeout: time.Second * time.Duration(1)},
-		Pattern: regexp.MustCompile("artifact~1~2"),
+		server:  serverUrl,
+		client:  &http.Client{Timeout: time.Second * time.Duration(1)},
+		pattern: regexp.MustCompile("artifact~1~2"),
 	}
 
 	assert.True(t, art.TryMakeRequest("artifact~1~2", result, r))
@@ -141,7 +141,7 @@ func TestBuildURL(t *testing.T) {
 	for _, c := range cases {
 		server, err := url.Parse(c.RawServer)
 		assert.NoError(t, err)
-		a := &Artifact{Server: server}
+		a := &Artifact{server: server}
 		u, ok := a.buildURL(c.Host, c.Path)
 		assert.Equal(t, c.Ok, ok, c.Description)
 		if c.Ok {
-- 
GitLab


From 77f58d083dc413febd279d8a71e382ba499087dc Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Tue, 5 Sep 2017 22:23:53 -0700
Subject: [PATCH 28/39] add test to acceptance_test.go that ensures a 502 is
 returned in the event of a timeout per the timeout settings #78

---
 acceptance_test.go | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/acceptance_test.go b/acceptance_test.go
index bedfc6bde..191203a60 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -8,6 +8,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -270,3 +271,20 @@ func TestProxyRequest_502(t *testing.T) {
 	defer resp.Body.Close()
 	assert.Equal(t, http.StatusBadGateway, resp.StatusCode)
 }
+
+func TestProxyRequest_timeout(t *testing.T) {
+	skipUnlessEnabled(t)
+	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(2 * time.Second)
+		w.Header().Set("Content-Type", "text/html; charset=utf-8")
+		fmt.Fprint(w, "content")
+	}))
+	defer testServer.Close()
+	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL, "-artifacts-server-timeout=1")
+	defer teardown()
+
+	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab-example.com", "index.html")
+	require.NoError(t, err)
+	defer resp.Body.Close()
+	assert.Equal(t, http.StatusBadGateway, resp.StatusCode)
+}
-- 
GitLab


From e4d784a5b97e0bbcc5c9fdfc2c11d8b35700ad97 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Tue, 5 Sep 2017 22:47:08 -0700
Subject: [PATCH 29/39] removal of check of blank url.Scheme within the runApp
 function. Adjust the acceptance_tests.go as well to remove un-needed test.
 #78

---
 acceptance_test.go | 12 ------------
 app.go             |  2 +-
 2 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index 191203a60..60a7cdadd 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -261,18 +261,6 @@ func TestProxyRequest_404(t *testing.T) {
 }
 
 func TestProxyRequest_502(t *testing.T) {
-	// This test attempts to make a requst to a bad address and ultimately fails returning 502
-	skipUnlessEnabled(t)
-	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+"bad address")
-	defer teardown()
-
-	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab-example.com", "index.html")
-	require.NoError(t, err)
-	defer resp.Body.Close()
-	assert.Equal(t, http.StatusBadGateway, resp.StatusCode)
-}
-
-func TestProxyRequest_timeout(t *testing.T) {
 	skipUnlessEnabled(t)
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		time.Sleep(2 * time.Second)
diff --git a/app.go b/app.go
index d4883b6c3..cd6e36638 100644
--- a/app.go
+++ b/app.go
@@ -191,7 +191,7 @@ func runApp(config appConfig) {
 			log.Fatal(err)
 		}
 		// url.Parse ensures that the Scheme arttribute is always lower case.
-		if u.Scheme != "" && u.Scheme != "http" && u.Scheme != "https" {
+		if u.Scheme != "http" && u.Scheme != "https" {
 			log.Fatal(errArtifactSchemaUnsupported)
 		}
 
-- 
GitLab


From de17088da27907836382a506771f244751aacbb9 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Tue, 5 Sep 2017 23:52:13 -0700
Subject: [PATCH 30/39] refactoring of acceptance tests to run the
 TestProxyRequest tests as table based / sub tests. Add commenting to
 artifact.go

---
 acceptance_test.go                 | 127 ++++++++++++++++-------------
 internal/artifact/artifact.go      |   3 +
 internal/artifact/artifact_test.go |   4 +-
 3 files changed, 76 insertions(+), 58 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index 60a7cdadd..ca83334fa 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -216,63 +216,78 @@ func TestProxyRequest(t *testing.T) {
 	content := "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>"
 	contentLength := int64(len(content))
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Set("Content-Type", "text/html; charset=utf-8")
-		fmt.Fprint(w, content)
-	}))
-	defer testServer.Close()
-	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL)
-	defer teardown()
-
-	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab-example.com", "index.html")
-	require.NoError(t, err)
-	body, err := ioutil.ReadAll(resp.Body)
-	require.NoError(t, err)
-	defer resp.Body.Close()
-	assert.Equal(t, string(body), content)
-	assert.Equal(t, http.StatusOK, resp.StatusCode)
-	assert.Equal(t, contentLength, resp.ContentLength)
-	assert.Equal(t, "max-age=600", resp.Header.Get("Cache-Control"))
-}
-
-func TestProxyRequest_404(t *testing.T) {
-	// In the event the server responds with a 404 reponse we want to ensure
-	// that said response code is also proxied in our response.
-	skipUnlessEnabled(t)
-	content := "<!DOCTYPE html><html><head><title>404 Not Found</title></head><body></body></html>"
-	contentLength := int64(len(content))
-	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Set("Content-Type", "text/html; charset=utf-8")
-		w.WriteHeader(http.StatusNotFound)
-		fmt.Fprint(w, content)
-	}))
-	defer testServer.Close()
-	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL)
-	defer teardown()
-
-	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab-example.com", "index.html")
-	require.NoError(t, err)
-	body, err := ioutil.ReadAll(resp.Body)
-	require.NoError(t, err)
-	defer resp.Body.Close()
-	assert.Equal(t, string(body), content)
-	assert.Equal(t, http.StatusNotFound, resp.StatusCode)
-	assert.Equal(t, contentLength, resp.ContentLength)
-	assert.Equal(t, "max-age=600", resp.Header.Get("Cache-Control"))
-}
-
-func TestProxyRequest_502(t *testing.T) {
-	skipUnlessEnabled(t)
-	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		time.Sleep(2 * time.Second)
-		w.Header().Set("Content-Type", "text/html; charset=utf-8")
-		fmt.Fprint(w, "content")
+		switch r.URL.Path {
+		case "/projects/1/jobs/2/artifacts/delayed_200.html":
+			time.Sleep(2 * time.Second)
+			fallthrough
+		case "/projects/1/jobs/2/artifacts/200.html":
+			w.Header().Set("Content-Type", "text/html; charset=utf-8")
+			fmt.Fprint(w, content)
+		default:
+			w.Header().Set("Content-Type", "text/html; charset=utf-8")
+			w.WriteHeader(404)
+			fmt.Fprint(w, content)
+		}
 	}))
 	defer testServer.Close()
-	teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL, "-artifacts-server-timeout=1")
-	defer teardown()
+	cases := []struct {
+		Path         string
+		Status       int
+		BinaryOption string
+		Content      string
+		Length       int64
+		CacheControl string
+		ContentType  string
+		Description  string
+	}{
+		{
+			"200.html",
+			http.StatusOK,
+			"",
+			content,
+			contentLength,
+			"max-age=600",
+			"text/html; charset=utf-8",
+			"basic proxied request",
+		},
+		{
+			"delayed_200.html",
+			http.StatusBadGateway,
+			"-artifacts-server-timeout=1",
+			"",
+			0,
+			"",
+			"text/html; charset=utf-8",
+			"502 error while attempting to proxy",
+		},
+		{
+			"404.html",
+			http.StatusNotFound,
+			"",
+			content,
+			contentLength,
+			"max-age=600",
+			"text/html; charset=utf-8",
+			"Proxying 404 from server",
+		},
+	}
 
-	resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab-example.com", "index.html")
-	require.NoError(t, err)
-	defer resp.Body.Close()
-	assert.Equal(t, http.StatusBadGateway, resp.StatusCode)
+	for _, c := range cases {
+		t.Run(fmt.Sprintf("Proxy Request Test: %s", c.Description), func(t *testing.T) {
+			teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL, c.BinaryOption)
+			defer teardown()
+			resp, err := GetPageFromListener(t, httpListener, "artifact~1~2.gitlab-example.com", c.Path)
+			require.NoError(t, err)
+			defer resp.Body.Close()
+			assert.Equal(t, c.Status, resp.StatusCode)
+			assert.Equal(t, c.ContentType, resp.Header.Get("Content-Type"))
+			if c.Status != http.StatusBadGateway {
+				body, err := ioutil.ReadAll(resp.Body)
+				require.NoError(t, err)
+				assert.Equal(t, c.Content, string(body))
+				assert.Equal(t, c.Length, resp.ContentLength)
+				assert.Equal(t, c.CacheControl, resp.Header.Get("Cache-Control"))
+			}
+		})
+	}
 }
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 62e89fd16..6ca5c55e0 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -66,6 +66,9 @@ func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Re
 	return true
 }
 
+// buildURL returns a pointer to a url.URL for where the request should be
+// proxied to. The returned bool will indicate if there is some sort of issue
+// with the url while it is being generated.
 func (a *Artifact) buildURL(host, path string) (*url.URL, bool) {
 	body := urlBody(host)
 	if body == "" {
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index 32e78a028..1ad56b306 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -23,10 +23,10 @@ func TestTryMakeRequest(t *testing.T) {
 	reqURL, err := url.Parse("/index.html")
 	assert.NoError(t, err)
 	r := &http.Request{URL: reqURL}
-	serverUrl, err := url.Parse(testServer.URL)
+	serverURL, err := url.Parse(testServer.URL)
 	assert.NoError(t, err)
 	art := &Artifact{
-		server:  serverUrl,
+		server:  serverURL,
 		client:  &http.Client{Timeout: time.Second * time.Duration(1)},
 		pattern: regexp.MustCompile("artifact~1~2"),
 	}
-- 
GitLab


From bb81efcdf983c19a68c3a4eba7b32a125ca07b1d Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Wed, 6 Sep 2017 18:34:46 -0700
Subject: [PATCH 31/39] consolidate the functionality of 2 methods within
 internal/artifact/artifact.go. Needed to make various adjustments to the
 tests accordingly. #78

---
 internal/artifact/artifact.go      | 30 ++++-------
 internal/artifact/artifact_test.go | 83 ++++++------------------------
 2 files changed, 25 insertions(+), 88 deletions(-)

diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 6ca5c55e0..f54f9f93f 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -13,7 +13,8 @@ import (
 )
 
 const (
-	baseURL = "/projects/%s/jobs/%s/artifacts"
+	baseURL             = "/projects/%s/jobs/%s/artifacts"
+	hostPatternTemplate = `(?i)\Aartifact~(\d+)~(\d+)\.%s\z`
 )
 
 var (
@@ -43,7 +44,7 @@ func New(u *url.URL, timeout int, pagesDomain string) *Artifact {
 // http.ResponseWriter, ultimately returning a bool that indicates if the
 // http.ResponseWriter has been written to in any capacity.
 func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Request) bool {
-	if a == nil || a.server == nil || a.server.String() == "" || !a.pattern.MatchString(host) {
+	if a == nil || a.server == nil || a.server.String() == "" {
 		return false
 	}
 
@@ -70,11 +71,14 @@ func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Re
 // proxied to. The returned bool will indicate if there is some sort of issue
 // with the url while it is being generated.
 func (a *Artifact) buildURL(host, path string) (*url.URL, bool) {
-	body := urlBody(host)
-	if body == "" {
+	ids := a.pattern.FindAllStringSubmatch(host, -1)
+	if len(ids) < 1 {
 		return nil, false
 	}
 
+	strippedIds := make([]string, 2)
+	strippedIds[0], strippedIds[1] = ids[0][1], ids[0][2]
+	body := fmt.Sprintf(baseURL, strippedIds[0], strippedIds[1])
 	u, err := url.Parse(fmt.Sprintf("%s%s%s", a.server.String(), body, path))
 	if err != nil {
 		return nil, false
@@ -82,26 +86,10 @@ func (a *Artifact) buildURL(host, path string) (*url.URL, bool) {
 	return u, true
 }
 
-// urlBody returns a string that represents the body of a url an Artifact{}
-// will ultimately proxy a request to. An expected host argument would
-// be close to "artifact~1~2". 1 and 2 represent Id's that need to be extracted
-// and interpolated into the const baseURL.
-func urlBody(host string) string {
-	ids := hostPattern.FindAllStringSubmatch(host, -1)
-
-	if len(ids) < 1 {
-		return ""
-	}
-
-	strippedIds := make([]string, 2)
-	strippedIds[0], strippedIds[1] = ids[0][1], ids[0][2]
-	return fmt.Sprintf(baseURL, strippedIds[0], strippedIds[1])
-}
-
 // hostPatternGen returns a pointer to a regexp.Regexp that is made up of
 // the constant hostPattern and the argument which represents the pages domain.
 // This is used to ensure that the requested page meets not only the hostPattern
 // requirements, but is suffixed with the proper pagesDomain.
 func hostPatternGen(pagesDomain string) *regexp.Regexp {
-	return regexp.MustCompile(fmt.Sprintf(`%s\.%s\z`, hostPattern, regexp.QuoteMeta(pagesDomain)))
+	return regexp.MustCompile(fmt.Sprintf(hostPatternTemplate, regexp.QuoteMeta(pagesDomain)))
 }
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index 1ad56b306..941dd090d 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -28,10 +28,10 @@ func TestTryMakeRequest(t *testing.T) {
 	art := &Artifact{
 		server:  serverURL,
 		client:  &http.Client{Timeout: time.Second * time.Duration(1)},
-		pattern: regexp.MustCompile("artifact~1~2"),
+		pattern: regexp.MustCompile(fmt.Sprintf(hostPatternTemplate, "gitlab-example.io")),
 	}
 
-	assert.True(t, art.TryMakeRequest("artifact~1~2", result, r))
+	assert.True(t, art.TryMakeRequest("artifact~1~2.gitlab-example.io", result, r))
 	assert.Equal(t, "text/html; charset=utf-8", result.Header().Get("Content-Type"))
 	assert.Equal(t, "90", result.Header().Get("Content-Length"))
 	assert.Equal(t, "max-age=600", result.Header().Get("Cache-Control"))
@@ -39,76 +39,13 @@ func TestTryMakeRequest(t *testing.T) {
 	assert.Equal(t, http.StatusOK, result.Code)
 }
 
-func TestURLBody(t *testing.T) {
-	cases := []struct {
-		URLHost     string
-		Expected    string
-		Description string
-	}{
-		{
-			"artifact~1~2",
-			"/projects/1/jobs/2/artifacts",
-			"basic case",
-		},
-		{
-			"artifact~10000~20000",
-			"/projects/10000/jobs/20000/artifacts",
-			"expanded case",
-		},
-		{
-			"artifact~86753095555~55550935768",
-			"/projects/86753095555/jobs/55550935768/artifacts",
-			"large number case",
-		},
-		{
-			"artifact~one~two",
-			"",
-			"letters rather than numbers",
-		},
-		{
-			"artifact~One111~tWo222",
-			"",
-			"Mixture of alphanumeric",
-		},
-		{
-			"artifact~!@#$%~%$#@!",
-			"",
-			"special characters",
-		},
-		{
-			"artifact~~",
-			"",
-			"no ids",
-		},
-		{
-			"ArTifAcT~1~2",
-			"/projects/1/jobs/2/artifacts",
-			"case insensitive",
-		},
-		{
-			"",
-			"",
-			"empty host",
-		},
-		{
-			"artifact~1~2.gitlab.io",
-			"/projects/1/jobs/2/artifacts",
-			"basic with host suffix",
-		},
-	}
-
-	for _, c := range cases {
-		actual := urlBody(c.URLHost)
-		assert.Equal(t, c.Expected, actual, c.Description)
-	}
-}
-
 func TestBuildURL(t *testing.T) {
 	cases := []struct {
 		RawServer   string
 		Host        string
 		Path        string
 		Expected    string
+		PagesDomain string
 		Ok          bool
 		Description string
 	}{
@@ -117,6 +54,7 @@ func TestBuildURL(t *testing.T) {
 			"artifact~1~2.gitlab.io",
 			"/path/to/file.txt",
 			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/path/to/file.txt",
+			"gitlab.io",
 			true,
 			"basic case",
 		},
@@ -125,14 +63,25 @@ func TestBuildURL(t *testing.T) {
 			"artifact~100000~200000.gitlab.io",
 			"/file.txt",
 			"https://gitlab.com/api/v4/projects/100000/jobs/200000/artifacts/file.txt",
+			"gitlab.io",
 			true,
 			"expanded case",
 		},
+		{
+			"https://gitlab.com/api/v4",
+			"artifact~A~B.gitlab.io",
+			"/index.html",
+			"",
+			"example.com",
+			false,
+			"non matching domain and request",
+		},
 		{
 			"",
 			"artifact~A~B.gitlab.io",
 			"",
 			"",
+			"",
 			false,
 			"un-parseable Host",
 		},
@@ -141,7 +90,7 @@ func TestBuildURL(t *testing.T) {
 	for _, c := range cases {
 		server, err := url.Parse(c.RawServer)
 		assert.NoError(t, err)
-		a := &Artifact{server: server}
+		a := &Artifact{server: server, pattern: regexp.MustCompile(fmt.Sprintf(hostPatternTemplate, c.PagesDomain))}
 		u, ok := a.buildURL(c.Host, c.Path)
 		assert.Equal(t, c.Ok, ok, c.Description)
 		if c.Ok {
-- 
GitLab


From ee9be29e6e35e6bed94f1b2fdadbf13ae25228ac Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Wed, 6 Sep 2017 21:31:12 -0700
Subject: [PATCH 32/39] adjust appConfig within main in order to have
 ArtifactsServer be a url.URL pointer rather than a string. Also add
 ArtifictsServerTimeout as well. Transfer validation logic back within
 configFromFlags function. #78

---
 app.go                        | 18 ++----------------
 app_config.go                 | 11 +++++++----
 internal/artifact/artifact.go |  8 ++------
 main.go                       | 23 ++++++++++++++++++++++-
 4 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/app.go b/app.go
index cd6e36638..1c48f947d 100644
--- a/app.go
+++ b/app.go
@@ -5,7 +5,6 @@ import (
 	"log"
 	"net"
 	"net/http"
-	"net/url"
 	"strconv"
 	"strings"
 	"sync"
@@ -185,21 +184,8 @@ func (a *theApp) Run() {
 func runApp(config appConfig) {
 	a := theApp{appConfig: config}
 
-	if config.ArtifactsServer != "" {
-		u, err := url.Parse(config.ArtifactsServer)
-		if err != nil {
-			log.Fatal(err)
-		}
-		// url.Parse ensures that the Scheme arttribute is always lower case.
-		if u.Scheme != "http" && u.Scheme != "https" {
-			log.Fatal(errArtifactSchemaUnsupported)
-		}
-
-		if *artifactsServerTimeout < 1 {
-			log.Fatal(errArtifactsServerTimeoutValue)
-		}
-
-		a.Artifact = artifact.New(u, *artifactsServerTimeout, *pagesDomain)
+	if config.ArtifactsServer != nil {
+		a.Artifact = artifact.New(config.ArtifactsServer, config.ArtifactsServerTimeout, config.Domain)
 	}
 	a.Run()
 }
diff --git a/app_config.go b/app_config.go
index 8ba983cc9..033a08215 100644
--- a/app_config.go
+++ b/app_config.go
@@ -1,10 +1,13 @@
 package main
 
+import "net/url"
+
 type appConfig struct {
-	Domain          string
-	ArtifactsServer string
-	RootCertificate []byte
-	RootKey         []byte
+	Domain                 string
+	ArtifactsServer        *url.URL
+	ArtifactsServerTimeout int
+	RootCertificate        []byte
+	RootKey                []byte
 
 	ListenHTTP    []uintptr
 	ListenHTTPS   []uintptr
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index f54f9f93f..a037886c2 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -17,10 +17,6 @@ const (
 	hostPatternTemplate = `(?i)\Aartifact~(\d+)~(\d+)\.%s\z`
 )
 
-var (
-	hostPattern = regexp.MustCompile(`(?i)\Aartifact~(\d+)~(\d+)`)
-)
-
 // Artifact is a struct that is made up of a url.URL, http.Client, and
 // regexp.Regexp that is used to proxy requests where applicable.
 type Artifact struct {
@@ -87,8 +83,8 @@ func (a *Artifact) buildURL(host, path string) (*url.URL, bool) {
 }
 
 // hostPatternGen returns a pointer to a regexp.Regexp that is made up of
-// the constant hostPattern and the argument which represents the pages domain.
-// This is used to ensure that the requested page meets not only the hostPattern
+// the constant hostPatternTemplate and the argument which represents the pages domain.
+// This is used to ensure that the requested page meets not only the hostPatternTemplate
 // requirements, but is suffixed with the proper pagesDomain.
 func hostPatternGen(pagesDomain string) *regexp.Regexp {
 	return regexp.MustCompile(fmt.Sprintf(hostPatternTemplate, regexp.QuoteMeta(pagesDomain)))
diff --git a/main.go b/main.go
index fbb5c2e4b..e843f53a9 100644
--- a/main.go
+++ b/main.go
@@ -4,6 +4,7 @@ import (
 	"errors"
 	"flag"
 	"log"
+	"net/url"
 	"os"
 	"strings"
 )
@@ -50,7 +51,27 @@ func configFromFlags() appConfig {
 		config.RootKey = readFile(*pagesRootKey)
 	}
 
-	config.ArtifactsServer = *artifactsServer
+	if *artifactsServerTimeout < 1 {
+		log.Fatal(errArtifactsServerTimeoutValue)
+	}
+
+	if *artifactsServer != "" {
+		u, err := url.Parse(*artifactsServer)
+		if err != nil {
+			log.Fatal(err)
+		}
+		// url.Parse ensures that the Scheme arttribute is always lower case.
+		if u.Scheme != "http" && u.Scheme != "https" {
+			log.Fatal(errArtifactSchemaUnsupported)
+		}
+
+		if *artifactsServerTimeout < 1 {
+			log.Fatal(errArtifactsServerTimeoutValue)
+		}
+
+		config.ArtifactsServerTimeout = *artifactsServerTimeout
+		config.ArtifactsServer = u
+	}
 	return config
 }
 
-- 
GitLab


From a458688ae093c81787b4ea91f994c6e3c94f49fe Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Thu, 7 Sep 2017 18:08:09 -0700
Subject: [PATCH 33/39] update message for the artifacts-server command line
 flag #78

---
 main.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/main.go b/main.go
index e843f53a9..44e5fa284 100644
--- a/main.go
+++ b/main.go
@@ -22,7 +22,7 @@ var (
 	useHTTP2               = flag.Bool("use-http2", true, "Enable HTTP2 support")
 	pagesRoot              = flag.String("pages-root", "shared/pages", "The directory where pages are stored")
 	pagesDomain            = flag.String("pages-domain", "gitlab-example.com", "The domain to serve static pages")
-	artifactsServer        = flag.String("artifacts-server", "", "https://gitlab.com/api/v4")
+	artifactsServer        = flag.String("artifacts-server", "", "API URL to proxy artifact requests to, e.g.: 'https://gitlab.com/api/v4'")
 	artifactsServerTimeout = flag.Int("artifacts-server-timeout", 10, "Timeout (in seconds) for a proxied request to the artifacts server")
 	pagesStatus            = flag.String("pages-status", "", "The url path for a status page, e.g., /@status")
 	metricsAddress         = flag.String("metrics-address", "", "The address to listen on for metrics requests")
-- 
GitLab


From f719f24db6a2ae69c4e15bdf78268f8648dc2d0c Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Thu, 7 Sep 2017 18:18:24 -0700
Subject: [PATCH 34/39] refactor the artifact type and corresponsing functions
 and methods within that package in order to change the server attribute to be
 a string and not url.URL. Made a corresponding to app_config to chang the
 ArtifactsServer url.URL attribute to be a sring, along with changes to app.go
 and main.go to compensate for said change. #78

---
 app.go                             | 2 +-
 app_config.go                      | 4 +---
 internal/artifact/artifact.go      | 8 ++++----
 internal/artifact/artifact_test.go | 8 ++------
 main.go                            | 2 +-
 5 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/app.go b/app.go
index 1c48f947d..3714582ae 100644
--- a/app.go
+++ b/app.go
@@ -184,7 +184,7 @@ func (a *theApp) Run() {
 func runApp(config appConfig) {
 	a := theApp{appConfig: config}
 
-	if config.ArtifactsServer != nil {
+	if config.ArtifactsServer != "" {
 		a.Artifact = artifact.New(config.ArtifactsServer, config.ArtifactsServerTimeout, config.Domain)
 	}
 	a.Run()
diff --git a/app_config.go b/app_config.go
index 033a08215..dd47ed017 100644
--- a/app_config.go
+++ b/app_config.go
@@ -1,10 +1,8 @@
 package main
 
-import "net/url"
-
 type appConfig struct {
 	Domain                 string
-	ArtifactsServer        *url.URL
+	ArtifactsServer        string
 	ArtifactsServerTimeout int
 	RootCertificate        []byte
 	RootKey                []byte
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index a037886c2..55067ec61 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -20,14 +20,14 @@ const (
 // Artifact is a struct that is made up of a url.URL, http.Client, and
 // regexp.Regexp that is used to proxy requests where applicable.
 type Artifact struct {
-	server  *url.URL
+	server  string
 	client  *http.Client
 	pattern *regexp.Regexp
 }
 
 // New when provided the arguments defined herein, returns a pointer to an
 // Artifact that is used to proxy requests.
-func New(u *url.URL, timeout int, pagesDomain string) *Artifact {
+func New(u string, timeout int, pagesDomain string) *Artifact {
 	return &Artifact{
 		server:  u,
 		client:  &http.Client{Timeout: time.Second * time.Duration(timeout)},
@@ -40,7 +40,7 @@ func New(u *url.URL, timeout int, pagesDomain string) *Artifact {
 // http.ResponseWriter, ultimately returning a bool that indicates if the
 // http.ResponseWriter has been written to in any capacity.
 func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Request) bool {
-	if a == nil || a.server == nil || a.server.String() == "" {
+	if a == nil || a.server == "" {
 		return false
 	}
 
@@ -75,7 +75,7 @@ func (a *Artifact) buildURL(host, path string) (*url.URL, bool) {
 	strippedIds := make([]string, 2)
 	strippedIds[0], strippedIds[1] = ids[0][1], ids[0][2]
 	body := fmt.Sprintf(baseURL, strippedIds[0], strippedIds[1])
-	u, err := url.Parse(fmt.Sprintf("%s%s%s", a.server.String(), body, path))
+	u, err := url.Parse(fmt.Sprintf("%s%s%s", a.server, body, path))
 	if err != nil {
 		return nil, false
 	}
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index 941dd090d..dd1f14844 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -23,10 +23,8 @@ func TestTryMakeRequest(t *testing.T) {
 	reqURL, err := url.Parse("/index.html")
 	assert.NoError(t, err)
 	r := &http.Request{URL: reqURL}
-	serverURL, err := url.Parse(testServer.URL)
-	assert.NoError(t, err)
 	art := &Artifact{
-		server:  serverURL,
+		server:  testServer.URL,
 		client:  &http.Client{Timeout: time.Second * time.Duration(1)},
 		pattern: regexp.MustCompile(fmt.Sprintf(hostPatternTemplate, "gitlab-example.io")),
 	}
@@ -88,9 +86,7 @@ func TestBuildURL(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		server, err := url.Parse(c.RawServer)
-		assert.NoError(t, err)
-		a := &Artifact{server: server, pattern: regexp.MustCompile(fmt.Sprintf(hostPatternTemplate, c.PagesDomain))}
+		a := &Artifact{server: c.RawServer, pattern: regexp.MustCompile(fmt.Sprintf(hostPatternTemplate, c.PagesDomain))}
 		u, ok := a.buildURL(c.Host, c.Path)
 		assert.Equal(t, c.Ok, ok, c.Description)
 		if c.Ok {
diff --git a/main.go b/main.go
index 44e5fa284..1cfac4090 100644
--- a/main.go
+++ b/main.go
@@ -70,7 +70,7 @@ func configFromFlags() appConfig {
 		}
 
 		config.ArtifactsServerTimeout = *artifactsServerTimeout
-		config.ArtifactsServer = u
+		config.ArtifactsServer = *artifactsServer
 	}
 	return config
 }
-- 
GitLab


From 507703f70a053fdb867d9ed131ae569480f3fdaa Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Thu, 7 Sep 2017 19:03:30 -0700
Subject: [PATCH 35/39] change to artifact TryMakeRequest function to limit
 caching within the 2xx response code series. Adjust tests in order to reflect
 this #78

---
 internal/artifact/artifact.go      | 10 ++--
 internal/artifact/artifact_test.go | 86 ++++++++++++++++++++++++------
 2 files changed, 77 insertions(+), 19 deletions(-)

diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 55067ec61..790f08ca0 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -27,9 +27,9 @@ type Artifact struct {
 
 // New when provided the arguments defined herein, returns a pointer to an
 // Artifact that is used to proxy requests.
-func New(u string, timeout int, pagesDomain string) *Artifact {
+func New(s string, timeout int, pagesDomain string) *Artifact {
 	return &Artifact{
-		server:  u,
+		server:  s,
 		client:  &http.Client{Timeout: time.Second * time.Duration(timeout)},
 		pattern: hostPatternGen(pagesDomain),
 	}
@@ -55,8 +55,12 @@ func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Re
 		return true
 	}
 
+	// we only cache responses within the 2xx series response codes
+	if 200 >= resp.StatusCode || resp.StatusCode <= 299 {
+		w.Header().Set("Cache-Control", "max-age=3600")
+	}
+
 	w.Header().Set("Content-Type", resp.Header.Get("Content-Type"))
-	w.Header().Set("Cache-Control", "max-age=600")
 	w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10))
 	w.WriteHeader(resp.StatusCode)
 	io.Copy(w, resp.Body)
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index dd1f14844..28796b563 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -13,28 +13,82 @@ import (
 )
 
 func TestTryMakeRequest(t *testing.T) {
+	content := "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>"
+	contentType := "text/html; charset=utf-8"
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Set("Content-Type", "text/html; charset=utf-8")
-		fmt.Fprint(w, "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
+		w.Header().Set("Content-Type", contentType)
+		switch r.URL.Path {
+		case "/projects/1/jobs/2/artifacts/200.html":
+			w.WriteHeader(http.StatusOK)
+		case "/projects/1/jobs/2/artifacts/max-caching.html":
+			w.WriteHeader(http.StatusIMUsed)
+		case "/projects/1/jobs/2/artifacts/non-caching.html":
+			w.WriteHeader(http.StatusTeapot)
+		case "/projects/1/jobs/2/artifacts/500.html":
+			w.WriteHeader(http.StatusInternalServerError)
+		case "/projects/1/jobs/2/artifacts/404.html":
+			w.WriteHeader(http.StatusNotFound)
+		}
+		fmt.Fprint(w, content)
 	}))
 	defer testServer.Close()
 
-	result := httptest.NewRecorder()
-	reqURL, err := url.Parse("/index.html")
-	assert.NoError(t, err)
-	r := &http.Request{URL: reqURL}
-	art := &Artifact{
-		server:  testServer.URL,
-		client:  &http.Client{Timeout: time.Second * time.Duration(1)},
-		pattern: regexp.MustCompile(fmt.Sprintf(hostPatternTemplate, "gitlab-example.io")),
+	cases := []struct {
+		Path         string
+		Status       int
+		Content      string
+		Length       string
+		CacheControl string
+		ContentType  string
+		Description  string
+	}{
+		{
+			"/200.html",
+			http.StatusOK,
+			content,
+			"90",
+			"max-age=3600",
+			"text/html; charset=utf-8",
+			"basic successful request",
+		},
+		{
+			"/max-caching.html",
+			http.StatusIMUsed,
+			content,
+			"90",
+			"max-age=3600",
+			"text/html; charset=utf-8",
+			"max caching request",
+		},
+		{
+			"/non-caching.html",
+			http.StatusTeapot,
+			content,
+			"90",
+			"",
+			"text/html; charset=utf-8",
+			"no caching request",
+		},
 	}
 
-	assert.True(t, art.TryMakeRequest("artifact~1~2.gitlab-example.io", result, r))
-	assert.Equal(t, "text/html; charset=utf-8", result.Header().Get("Content-Type"))
-	assert.Equal(t, "90", result.Header().Get("Content-Length"))
-	assert.Equal(t, "max-age=600", result.Header().Get("Cache-Control"))
-	assert.Equal(t, string(result.Body.Bytes()), "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>")
-	assert.Equal(t, http.StatusOK, result.Code)
+	for _, c := range cases {
+		result := httptest.NewRecorder()
+		reqURL, err := url.Parse(c.Path)
+		assert.NoError(t, err)
+		r := &http.Request{URL: reqURL}
+		art := &Artifact{
+			server:  testServer.URL,
+			client:  &http.Client{Timeout: time.Second * time.Duration(1)},
+			pattern: regexp.MustCompile(fmt.Sprintf(hostPatternTemplate, "gitlab-example.io")),
+		}
+
+		assert.True(t, art.TryMakeRequest("artifact~1~2.gitlab-example.io", result, r))
+		assert.Equal(t, c.ContentType, result.Header().Get("Content-Type"))
+		assert.Equal(t, c.Length, result.Header().Get("Content-Length"))
+		assert.Equal(t, c.CacheControl, result.Header().Get("Cache-Control"))
+		assert.Equal(t, c.Content, string(result.Body.Bytes()))
+		assert.Equal(t, c.Status, result.Code)
+	}
 }
 
 func TestBuildURL(t *testing.T) {
-- 
GitLab


From 6d679ead7a9d52d4bce32b80a1e44ba18d5e5b3c Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Thu, 7 Sep 2017 20:22:00 -0700
Subject: [PATCH 36/39] additional revision and refinement to a feature that
 only sets the cache-control on an artifact proxy if it is within certain
 status codes #78

---
 acceptance_test.go            | 4 ++--
 internal/artifact/artifact.go | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index ca83334fa..1920d1a94 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -246,7 +246,7 @@ func TestProxyRequest(t *testing.T) {
 			"",
 			content,
 			contentLength,
-			"max-age=600",
+			"max-age=3600",
 			"text/html; charset=utf-8",
 			"basic proxied request",
 		},
@@ -266,7 +266,7 @@ func TestProxyRequest(t *testing.T) {
 			"",
 			content,
 			contentLength,
-			"max-age=600",
+			"",
 			"text/html; charset=utf-8",
 			"Proxying 404 from server",
 		},
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 790f08ca0..503d8a300 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -15,6 +15,8 @@ import (
 const (
 	baseURL             = "/projects/%s/jobs/%s/artifacts"
 	hostPatternTemplate = `(?i)\Aartifact~(\d+)~(\d+)\.%s\z`
+	minStatusCode       = 200
+	maxStatusCode       = 299
 )
 
 // Artifact is a struct that is made up of a url.URL, http.Client, and
@@ -56,7 +58,7 @@ func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Re
 	}
 
 	// we only cache responses within the 2xx series response codes
-	if 200 >= resp.StatusCode || resp.StatusCode <= 299 {
+	if (resp.StatusCode >= minStatusCode) && (resp.StatusCode <= maxStatusCode) {
 		w.Header().Set("Cache-Control", "max-age=3600")
 	}
 
-- 
GitLab


From 45c6f2ffcb06f1b51f10c66b919ccabb017a401e Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Thu, 7 Sep 2017 20:25:12 -0700
Subject: [PATCH 37/39] revision to buildURL function to make it more robust
 #78

---
 internal/artifact/artifact.go | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 503d8a300..e68c23460 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -74,12 +74,11 @@ func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Re
 // with the url while it is being generated.
 func (a *Artifact) buildURL(host, path string) (*url.URL, bool) {
 	ids := a.pattern.FindAllStringSubmatch(host, -1)
-	if len(ids) < 1 {
+	if len(ids) != 1 || len(ids[0]) != 3 {
 		return nil, false
 	}
 
-	strippedIds := make([]string, 2)
-	strippedIds[0], strippedIds[1] = ids[0][1], ids[0][2]
+	strippedIds := ids[0][1:3]
 	body := fmt.Sprintf(baseURL, strippedIds[0], strippedIds[1])
 	u, err := url.Parse(fmt.Sprintf("%s%s%s", a.server, body, path))
 	if err != nil {
-- 
GitLab


From e9ebfb9e37dc5bc4e90993b561840a10ea6a267c Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Thu, 7 Sep 2017 20:52:10 -0700
Subject: [PATCH 38/39] edit internal/artifact to return the gitlab 404 and 500
 pages in the event a 404 or 500 page is proxied respectively, adjust tests
 accordingly #78

---
 acceptance_test.go            | 23 ++++++++++++++++++-----
 internal/artifact/artifact.go | 10 ++++++++++
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index 1920d1a94..3e575bdf1 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -219,13 +219,16 @@ func TestProxyRequest(t *testing.T) {
 		switch r.URL.Path {
 		case "/projects/1/jobs/2/artifacts/delayed_200.html":
 			time.Sleep(2 * time.Second)
-			fallthrough
 		case "/projects/1/jobs/2/artifacts/200.html":
 			w.Header().Set("Content-Type", "text/html; charset=utf-8")
 			fmt.Fprint(w, content)
+		case "/projects/1/jobs/2/artifacts/500.html":
+			w.Header().Set("Content-Type", "text/html; charset=utf-8")
+			w.WriteHeader(http.StatusInternalServerError)
+			fmt.Fprint(w, content)
 		default:
 			w.Header().Set("Content-Type", "text/html; charset=utf-8")
-			w.WriteHeader(404)
+			w.WriteHeader(http.StatusNotFound)
 			fmt.Fprint(w, content)
 		}
 	}))
@@ -264,12 +267,22 @@ func TestProxyRequest(t *testing.T) {
 			"404.html",
 			http.StatusNotFound,
 			"",
-			content,
-			contentLength,
+			"",
+			0,
 			"",
 			"text/html; charset=utf-8",
 			"Proxying 404 from server",
 		},
+		{
+			"500.html",
+			http.StatusInternalServerError,
+			"",
+			"",
+			0,
+			"",
+			"text/html; charset=utf-8",
+			"Proxying 500 from server",
+		},
 	}
 
 	for _, c := range cases {
@@ -281,7 +294,7 @@ func TestProxyRequest(t *testing.T) {
 			defer resp.Body.Close()
 			assert.Equal(t, c.Status, resp.StatusCode)
 			assert.Equal(t, c.ContentType, resp.Header.Get("Content-Type"))
-			if c.Status != http.StatusBadGateway {
+			if !((c.Status == http.StatusBadGateway) || (c.Status == http.StatusNotFound) || (c.Status == http.StatusInternalServerError)) {
 				body, err := ioutil.ReadAll(resp.Body)
 				require.NoError(t, err)
 				assert.Equal(t, c.Content, string(body))
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index e68c23460..1ce198999 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -57,6 +57,16 @@ func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Re
 		return true
 	}
 
+	if resp.StatusCode == http.StatusNotFound {
+		httperrors.Serve404(w)
+		return true
+	}
+
+	if resp.StatusCode == http.StatusInternalServerError {
+		httperrors.Serve500(w)
+		return true
+	}
+
 	// we only cache responses within the 2xx series response codes
 	if (resp.StatusCode >= minStatusCode) && (resp.StatusCode <= maxStatusCode) {
 		w.Header().Set("Cache-Control", "max-age=3600")
-- 
GitLab


From 47ecb98973f23872885740291928bab468e00778 Mon Sep 17 00:00:00 2001
From: "J. Shuster" <joshuagregoryshuster@gmail.com>
Date: Thu, 7 Sep 2017 21:11:47 -0700
Subject: [PATCH 39/39] update to internal/artifact package in order to make
 the generation of a url within the buildURL method more robust. #78

---
 internal/artifact/artifact.go      | 16 ++++++-
 internal/artifact/artifact_test.go | 72 ++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index 1ce198999..bcb525ac5 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -7,6 +7,7 @@ import (
 	"net/url"
 	"regexp"
 	"strconv"
+	"strings"
 	"time"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
@@ -90,7 +91,20 @@ func (a *Artifact) buildURL(host, path string) (*url.URL, bool) {
 
 	strippedIds := ids[0][1:3]
 	body := fmt.Sprintf(baseURL, strippedIds[0], strippedIds[1])
-	u, err := url.Parse(fmt.Sprintf("%s%s%s", a.server, body, path))
+	ourPath := a.server
+	if strings.HasSuffix(ourPath, "/") {
+		ourPath = ourPath[0:len(ourPath)-1] + body
+	} else {
+		ourPath = ourPath + body
+	}
+
+	if len(path) == 0 || strings.HasPrefix(path, "/") {
+		ourPath = ourPath + path
+	} else {
+		ourPath = ourPath + "/" + path
+	}
+
+	u, err := url.Parse(ourPath)
 	if err != nil {
 		return nil, false
 	}
diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go
index 28796b563..ad2285f65 100644
--- a/internal/artifact/artifact_test.go
+++ b/internal/artifact/artifact_test.go
@@ -110,6 +110,69 @@ func TestBuildURL(t *testing.T) {
 			true,
 			"basic case",
 		},
+		{
+			"https://gitlab.com/api/v4/",
+			"artifact~1~2.gitlab.io",
+			"/path/to/file.txt",
+			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/path/to/file.txt",
+			"gitlab.io",
+			true,
+			"basic case 2",
+		},
+		{
+			"https://gitlab.com/api/v4",
+			"artifact~1~2.gitlab.io",
+			"path/to/file.txt",
+			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/path/to/file.txt",
+			"gitlab.io",
+			true,
+			"basic case 3",
+		},
+		{
+			"https://gitlab.com/api/v4/",
+			"artifact~1~2.gitlab.io",
+			"path/to/file.txt",
+			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/path/to/file.txt",
+			"gitlab.io",
+			true,
+			"basic case 4",
+		},
+		{
+			"https://gitlab.com/api/v4",
+			"artifact~1~2.gitlab.io",
+			"",
+			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts",
+			"gitlab.io",
+			true,
+			"basic case 5",
+		},
+		{
+			"https://gitlab.com/api/v4/",
+			"artifact~1~2.gitlab.io",
+			"",
+			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts",
+			"gitlab.io",
+			true,
+			"basic case 6",
+		},
+		{
+			"https://gitlab.com/api/v4",
+			"artifact~1~2.gitlab.io",
+			"/",
+			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/",
+			"gitlab.io",
+			true,
+			"basic case 7",
+		},
+		{
+			"https://gitlab.com/api/v4/",
+			"artifact~1~2.gitlab.io",
+			"/",
+			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/",
+			"gitlab.io",
+			true,
+			"basic case 8",
+		},
 		{
 			"https://gitlab.com/api/v4",
 			"artifact~100000~200000.gitlab.io",
@@ -119,6 +182,15 @@ func TestBuildURL(t *testing.T) {
 			true,
 			"expanded case",
 		},
+		{
+			"https://gitlab.com/api/v4/",
+			"artifact~1~2.gitlab.io",
+			"/file.txt",
+			"https://gitlab.com/api/v4/projects/1/jobs/2/artifacts/file.txt",
+			"gitlab.io",
+			true,
+			"server with tailing slash",
+		},
 		{
 			"https://gitlab.com/api/v4",
 			"artifact~A~B.gitlab.io",
-- 
GitLab