12167: Merge branch 'master'

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --combined services/keep-web/handler.go
index 02e7cb185834b3cccb8a7642ea81509bfecb8dfd,0813ba7add172e2af932a120016a0636d86d2fdc..517ec1a2a26e96967ad50bec925a65b1f6149f6a
--- 1/services/keep-web/handler.go
--- 2/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@@ -10,6 -10,7 +10,6 @@@ import 
        "html"
        "html/template"
        "io"
 -      "log"
        "net/http"
        "net/url"
        "os"
@@@ -25,7 -26,6 +25,7 @@@
        "git.curoverse.com/arvados.git/sdk/go/health"
        "git.curoverse.com/arvados.git/sdk/go/httpserver"
        "git.curoverse.com/arvados.git/sdk/go/keepclient"
 +      log "github.com/Sirupsen/logrus"
        "golang.org/x/net/webdav"
  )
  
@@@ -191,12 -191,13 +191,12 @@@ func (h *handler) ServeHTTP(wOrig http.
                } else if w.WroteStatus() == 0 {
                        w.WriteHeader(statusCode)
                } else if w.WroteStatus() != statusCode {
 -                      httpserver.Log(r.RemoteAddr, "WARNING",
 +                      log.WithField("RequestID", r.Header.Get("X-Request-Id")).Warn(
                                fmt.Sprintf("Our status changed from %d to %d after we sent headers", w.WroteStatus(), statusCode))
                }
                if statusText == "" {
                        statusText = http.StatusText(statusCode)
                }
 -              httpserver.Log(remoteAddr, statusCode, statusText, w.WroteBodyBytes(), r.Method, r.Host, r.URL.Path, r.URL.RawQuery)
        }()
  
        if strings.HasPrefix(r.URL.Path, "/_health/") && r.Method == "GET" {
@@@ -313,6 -314,14 +313,14 @@@
                return
        }
  
+       if useSiteFS {
+               if tokens == nil {
+                       tokens = auth.NewCredentialsFromHTTPRequest(r).Tokens
+               }
+               h.serveSiteFS(w, r, tokens, credentialsOK, attachment)
+               return
+       }
+ 
        targetPath := pathParts[stripParts:]
        if tokens == nil && len(targetPath) > 0 && strings.HasPrefix(targetPath[0], "t=") {
                // http://ID.example/t=TOKEN/PATH...
@@@ -334,11 -343,6 +342,6 @@@
                tokens = append(reqTokens, h.Config.AnonymousTokens...)
        }
  
-       if useSiteFS {
-               h.serveSiteFS(w, r, tokens, credentialsOK, attachment)
-               return
-       }
- 
        if len(targetPath) > 0 && targetPath[0] == "_" {
                // If a collection has a directory called "t=foo" or
                // "_", it can be served at
@@@ -416,7 -420,6 +419,7 @@@
                statusCode, statusText = http.StatusInternalServerError, err.Error()
                return
        }
 +      kc.RequestID = r.Header.Get("X-Request-Id")
  
        var basename string
        if len(targetPath) > 0 {
@@@ -424,11 -427,11 +427,11 @@@
        }
        applyContentDispositionHdr(w, r, basename, attachment)
  
 -      client := &arvados.Client{
 +      client := (&arvados.Client{
                APIHost:   arv.ApiServer,
                AuthToken: arv.ApiToken,
                Insecure:  arv.ApiInsecure,
 -      }
 +      }).WithRequestID(r.Header.Get("X-Request-Id"))
  
        fs, err := collection.FileSystem(client, kc)
        if err != nil {
@@@ -529,12 -532,11 +532,12 @@@ func (h *handler) serveSiteFS(w http.Re
                http.Error(w, err.Error(), http.StatusInternalServerError)
                return
        }
 -      client := &arvados.Client{
 +      kc.RequestID = r.Header.Get("X-Request-Id")
 +      client := (&arvados.Client{
                APIHost:   arv.ApiServer,
                AuthToken: arv.ApiToken,
                Insecure:  arv.ApiInsecure,
 -      }
 +      }).WithRequestID(r.Header.Get("X-Request-Id"))
        fs := client.SiteFileSystem(kc)
        f, err := fs.Open(r.URL.Path)
        if os.IsNotExist(err) {
@@@ -616,9 -618,9 +619,9 @@@ the entire directory tree with wget, tr
  <UL>
  {{range .Files}}
  {{if .IsDir }}
-   <LI>{{" " | printf "%15s  " | nbsp}}<A href="{{.Name}}/">{{.Name}}/</A></LI>
+   <LI>{{" " | printf "%15s  " | nbsp}}<A href="{{print "./" .Name}}/">{{.Name}}/</A></LI>
  {{else}}
-   <LI>{{.Size | printf "%15d  " | nbsp}}<A href="{{.Name}}">{{.Name}}</A></LI>
+   <LI>{{.Size | printf "%15d  " | nbsp}}<A href="{{print "./" .Name}}">{{.Name}}</A></LI>
  {{end}}
  {{end}}
  </UL>

diff --combined services/keep-web/handler_test.go
index 03bd8f171e9748aead6c3537542c4d99f48fbc6b,06401f4825e29595bbefa3b9db368c1576ad2e56..f86f81bfa15e5a1c20fed2f68a796f029ae3a966
--- 1/services/keep-web/handler_test.go
--- 2/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@@ -12,10 -12,12 +12,12 @@@ import 
        "net/http"
        "net/http/httptest"
        "net/url"
+       "os"
        "path/filepath"
        "regexp"
        "strings"
  
+       "git.curoverse.com/arvados.git/sdk/go/arvados"
        "git.curoverse.com/arvados.git/sdk/go/arvadostest"
        "git.curoverse.com/arvados.git/sdk/go/auth"
        check "gopkg.in/check.v1"
@@@ -430,6 -432,38 +432,38 @@@ func (s *IntegrationSuite) TestAnonymou
        )
  }
  
+ func (s *IntegrationSuite) TestSpecialCharsInPath(c *check.C) {
+       s.testServer.Config.AttachmentOnlyHost = "download.example.com"
+ 
+       client := s.testServer.Config.Client
+       client.AuthToken = arvadostest.ActiveToken
+       fs, err := (&arvados.Collection{}).FileSystem(&client, nil)
+       c.Assert(err, check.IsNil)
+       f, err := fs.OpenFile("https:\\\"odd' path chars", os.O_CREATE, 0777)
+       c.Assert(err, check.IsNil)
+       f.Close()
+       mtxt, err := fs.MarshalManifest(".")
+       c.Assert(err, check.IsNil)
+       coll := arvados.Collection{ManifestText: mtxt}
+       err = client.RequestAndDecode(&coll, "POST", "arvados/v1/collections", client.UpdateBody(coll), nil)
+       c.Assert(err, check.IsNil)
+ 
+       u, _ := url.Parse("http://download.example.com/c=" + coll.UUID + "/")
+       req := &http.Request{
+               Method:     "GET",
+               Host:       u.Host,
+               URL:        u,
+               RequestURI: u.RequestURI(),
+               Header: http.Header{
+                       "Authorization": {"Bearer " + client.AuthToken},
+               },
+       }
+       resp := httptest.NewRecorder()
+       s.testServer.Handler.ServeHTTP(resp, req)
+       c.Check(resp.Code, check.Equals, http.StatusOK)
+       c.Check(resp.Body.String(), check.Matches, `(?ms).*href="./https:%5c%22odd%27%20path%20chars"\S+https:\\"odd' path chars.*`)
+ }
+ 
  // XHRs can't follow redirect-with-cookie so they rely on method=POST
  // and disposition=attachment (telling us it's acceptable to respond
  // with content instead of a redirect) and an Origin header that gets
@@@ -632,7 -666,7 +666,7 @@@ func (s *IntegrationSuite) TestDirector
                        Host:       u.Host,
                        URL:        u,
                        RequestURI: u.RequestURI(),
 -                      Header:     trial.header,
 +                      Header:     copyHeader(trial.header),
                }
                s.testServer.Handler.ServeHTTP(resp, req)
                var cookies []*http.Cookie
@@@ -643,7 -677,7 +677,7 @@@
                                Host:       u.Host,
                                URL:        u,
                                RequestURI: u.RequestURI(),
 -                              Header:     trial.header,
 +                              Header:     copyHeader(trial.header),
                        }
                        cookies = append(cookies, (&http.Response{Header: resp.Header()}).Cookies()...)
                        for _, c := range cookies {
@@@ -660,7 -694,7 +694,7 @@@
                } else {
                        c.Check(resp.Code, check.Equals, http.StatusOK)
                        for _, e := range trial.expect {
-                               c.Check(resp.Body.String(), check.Matches, `(?ms).*href="`+e+`".*`)
+                               c.Check(resp.Body.String(), check.Matches, `(?ms).*href="./`+e+`".*`)
                        }
                        c.Check(resp.Body.String(), check.Matches, `(?ms).*--cut-dirs=`+fmt.Sprintf("%d", trial.cutDirs)+` .*`)
                }
@@@ -671,7 -705,7 +705,7 @@@
                        Host:       u.Host,
                        URL:        u,
                        RequestURI: u.RequestURI(),
 -                      Header:     trial.header,
 +                      Header:     copyHeader(trial.header),
                        Body:       ioutil.NopCloser(&bytes.Buffer{}),
                }
                resp = httptest.NewRecorder()
@@@ -687,7 -721,7 +721,7 @@@
                        Host:       u.Host,
                        URL:        u,
                        RequestURI: u.RequestURI(),
 -                      Header:     trial.header,
 +                      Header:     copyHeader(trial.header),
                        Body:       ioutil.NopCloser(&bytes.Buffer{}),
                }
                resp = httptest.NewRecorder()
@@@ -723,11 -757,3 +757,11 @@@ func (s *IntegrationSuite) TestHealthCh
        c.Check(resp.Code, check.Equals, http.StatusOK)
        c.Check(resp.Body.String(), check.Matches, `{"health":"OK"}\n`)
  }
 +
 +func copyHeader(h http.Header) http.Header {
 +      hc := http.Header{}
 +      for k, v := range h {
 +              hc[k] = append([]string(nil), v...)
 +      }
 +      return hc
 +}