[ARVADOS] updated: 05a38f1bd00d572cbfd67d7276f5bcae6bb24805
git at public.curoverse.com
git at public.curoverse.com
Mon Nov 9 03:29:10 EST 2015
Summary of changes:
.../controllers/collections_controller_test.rb | 40 +++++++++++++---------
apps/workbench/test/integration/download_test.rb | 22 +++++++-----
services/keep-web/handler.go | 3 ++
3 files changed, 41 insertions(+), 24 deletions(-)
discards b956a83421693e0dae56546d63e10dab6eb9cc3a (commit)
discards cf5734e08070805662bfcebab4bc8d3f90a3ae7e (commit)
via 05a38f1bd00d572cbfd67d7276f5bcae6bb24805 (commit)
via a4ed403c77477b7cc869c5fdd801ac839e941b9c (commit)
via 9d2a99db8f9b1259d94f650c550d2101e5d915d5 (commit)
This update added new revisions after undoing existing revisions. That is
to say, the old revision is not a strict subset of the new revision. This
situation occurs when you --force push a change and generate a repository
containing something like this:
* -- * -- B -- O -- O -- O (b956a83421693e0dae56546d63e10dab6eb9cc3a)
\
N -- N -- N (05a38f1bd00d572cbfd67d7276f5bcae6bb24805)
When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 05a38f1bd00d572cbfd67d7276f5bcae6bb24805
Author: Tom Clegg <tom at curoverse.com>
Date: Mon Nov 9 03:28:50 2015 -0500
5824: Add anonymous-404 and download-by-pdh tests.
diff --git a/apps/workbench/test/controllers/collections_controller_test.rb b/apps/workbench/test/controllers/collections_controller_test.rb
index b4e7dd3..7b0cbb5 100644
--- a/apps/workbench/test/controllers/collections_controller_test.rb
+++ b/apps/workbench/test/controllers/collections_controller_test.rb
@@ -10,6 +10,15 @@ class CollectionsControllerTest < ActionController::TestCase
NONEXISTENT_COLLECTION = "ffffffffffffffffffffffffffffffff+0"
+ def config_anonymous enable
+ Rails.configuration.anonymous_user_token =
+ if enable
+ api_fixture('api_client_authorizations')['anonymous']['api_token']
+ else
+ false
+ end
+ end
+
def stub_file_content
# For the duration of the current test case, stub file download
# content with a randomized (but recognizable) string. Return the
@@ -167,8 +176,7 @@ class CollectionsControllerTest < ActionController::TestCase
end
test 'anonymous download' do
- Rails.configuration.anonymous_user_token =
- api_fixture('api_client_authorizations')['anonymous']['api_token']
+ config_anonymous true
expect_content = stub_file_content
get :show_file, {
uuid: api_fixture('collections')['user_agreement_in_anonymously_accessible_project']['uuid'],
@@ -205,15 +213,14 @@ class CollectionsControllerTest < ActionController::TestCase
"using a reader token set the session's API token")
end
- [false, api_fixture('api_client_authorizations')['anonymous']['api_token']].
- each do |anon_conf|
- test "download a file using a reader token with insufficient scope (anon_conf=#{!!anon_conf})" do
- Rails.configuration.anonymous_user_token = anon_conf
+ [false, true].each do |anon|
+ test "download a file using a reader token with insufficient scope, anon #{anon}" do
+ config_anonymous anon
params = collection_params(:foo_file, 'foo')
params[:reader_token] =
api_fixture('api_client_authorizations')['active_noscope']['api_token']
get(:show_file, params)
- if anon_conf
+ if anon
# Some files can be shown without a valid token, but not this one.
assert_response 404
else
@@ -463,8 +470,7 @@ class CollectionsControllerTest < ActionController::TestCase
end
test "anonymous user accesses collection in shared project" do
- Rails.configuration.anonymous_user_token =
- api_fixture('api_client_authorizations')['anonymous']['api_token']
+ config_anonymous true
collection = api_fixture('collections')['public_text_file']
get(:show, {id: collection['uuid']})
@@ -541,8 +547,7 @@ class CollectionsControllerTest < ActionController::TestCase
test "Redirect to keep_web_url via #{id_type} with no token" do
setup_for_keep_web
- Rails.configuration.anonymous_user_token =
- api_fixture('api_client_authorizations')['anonymous']['api_token']
+ config_anonymous true
id = api_fixture('collections')['public_text_file'][id_type]
get :show_file, {uuid: id, file: "Hello World.txt"}
assert_response :redirect
@@ -559,10 +564,13 @@ class CollectionsControllerTest < ActionController::TestCase
end
end
- test "No redirect to keep_web_url if collection not found" do
- setup_for_keep_web
- id = api_fixture('collections')['w_a_z_file']['uuid']
- get :show_file, {uuid: id, file: "w a z"}, session_for(:spectator)
- assert_response 404
+ [false, true].each do |anon|
+ test "No redirect to keep_web_url if collection not found, anon #{anon}" do
+ setup_for_keep_web
+ config_anonymous anon
+ id = api_fixture('collections')['w_a_z_file']['uuid']
+ get :show_file, {uuid: id, file: "w a z"}, session_for(:spectator)
+ assert_response 404
+ end
end
end
diff --git a/apps/workbench/test/integration/download_test.rb b/apps/workbench/test/integration/download_test.rb
index 9e4fd56..cf4246c 100644
--- a/apps/workbench/test/integration/download_test.rb
+++ b/apps/workbench/test/integration/download_test.rb
@@ -19,21 +19,27 @@ class DownloadTest < ActionDispatch::IntegrationTest
end
end
- test "download from keep-web with a reader token" do
- uuid = api_fixture('collections')['foo_file']['uuid']
- token = api_fixture('api_client_authorizations')['active_all_collections']['api_token']
- visit "/collections/download/#{uuid}/#{token}/"
- within "#collection_files" do
- click_link "foo"
+ ['uuid', 'portable_data_hash'].each do |id_type|
+ test "download from keep-web by #{id_type} using a reader token" do
+ uuid_or_pdh = api_fixture('collections')['foo_file'][id_type]
+ token = api_fixture('api_client_authorizations')['active_all_collections']['api_token']
+ visit "/collections/download/#{uuid_or_pdh}/#{token}/"
+ within "#collection_files" do
+ click_link "foo"
+ end
+ wait_for_download 'foo', 'foo'
end
+ end
+
+ def wait_for_download filename, expect_data
data = nil
tries = 0
while tries < 20
sleep 0.1
tries += 1
- data = File.read(DownloadHelper.path.join 'foo') rescue nil
+ data = File.read(DownloadHelper.path.join filename) rescue nil
end
- assert_equal 'foo', data
+ assert_equal expect_data, data
end
# TODO(TC): test "view pages hosted by keep-web, using session
commit a4ed403c77477b7cc869c5fdd801ac839e941b9c
Author: Tom Clegg <tom at curoverse.com>
Date: Sun Nov 8 15:52:29 2015 -0500
5824: Propagate non-token parts of query string (notably ?attachment=disposition) when redirecting.
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 962c5e1..847329f 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -183,7 +183,17 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
Path: "/",
HttpOnly: true,
})
- redir := (&url.URL{Host: r.Host, Path: r.URL.Path}).String()
+
+ // Propagate query parameters (except api_token) from
+ // the original request.
+ redirQuery := r.URL.Query()
+ redirQuery.Del("api_token")
+
+ redir := (&url.URL{
+ Host: r.Host,
+ Path: r.URL.Path,
+ RawQuery: redirQuery.Encode(),
+ }).String()
w.Header().Add("Location", redir)
statusCode, statusText = http.StatusSeeOther, redir
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index b4758e0..94a1cf7 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -32,9 +32,11 @@ func (s *IntegrationSuite) TestVhost404(c *check.C) {
arvadostest.NonexistentCollection + ".example.com/t=" + arvadostest.ActiveToken + "/theperthcountyconspiracy",
} {
resp := httptest.NewRecorder()
+ u := mustParseURL(testURL)
req := &http.Request{
- Method: "GET",
- URL: mustParseURL(testURL),
+ Method: "GET",
+ URL: u,
+ RequestURI: u.RequestURI(),
}
(&handler{}).ServeHTTP(resp, req)
c.Check(resp.Code, check.Equals, http.StatusNotFound)
@@ -120,10 +122,11 @@ func doVhostRequestsWithHostPath(c *check.C, authz authorizer, hostPath string)
} {
u := mustParseURL("http://" + hostPath)
req := &http.Request{
- Method: "GET",
- Host: u.Host,
- URL: u,
- Header: http.Header{},
+ Method: "GET",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{},
}
failCode := authz(req, tok)
resp := doReq(req)
@@ -157,10 +160,11 @@ func doReq(req *http.Request) *httptest.ResponseRecorder {
cookies := (&http.Response{Header: resp.Header()}).Cookies()
u, _ := req.URL.Parse(resp.Header().Get("Location"))
req = &http.Request{
- Method: "GET",
- Host: u.Host,
- URL: u,
- Header: http.Header{},
+ Method: "GET",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{},
}
for _, c := range cookies {
req.AddCookie(c)
@@ -228,6 +232,21 @@ func (s *IntegrationSuite) TestVhostRedirectQueryTokenSingleOriginError(c *check
)
}
+// If client requests an attachment by putting ?disposition=attachment
+// in the query string, and gets redirected, the redirect target
+// should respond with an attachment.
+func (s *IntegrationSuite) TestVhostRedirectQueryTokenRequestAttachment(c *check.C) {
+ resp := s.testVhostRedirectTokenToCookie(c, "GET",
+ arvadostest.FooCollection+".example.com/foo",
+ "?disposition=attachment&api_token="+arvadostest.ActiveToken,
+ "",
+ "",
+ http.StatusOK,
+ "foo",
+ )
+ c.Check(strings.Split(resp.Header().Get("Content-Disposition"), ";")[0], check.Equals, "attachment")
+}
+
func (s *IntegrationSuite) TestVhostRedirectQueryTokenTrustAllContent(c *check.C) {
defer func(orig bool) {
trustAllContent = orig
@@ -316,12 +335,13 @@ func (s *IntegrationSuite) TestAnonymousTokenError(c *check.C) {
}
func (s *IntegrationSuite) TestRange(c *check.C) {
- u, _ := url.Parse("http://example.com/c="+arvadostest.HelloWorldCollection+"/Hello%20world.txt")
+ u, _ := url.Parse("http://example.com/c=" + arvadostest.HelloWorldCollection + "/Hello%20world.txt")
req := &http.Request{
- Method: "GET",
- Host: u.Host,
- URL: u,
- Header: http.Header{"Range": {"bytes=0-4"}},
+ Method: "GET",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{"Range": {"bytes=0-4"}},
}
resp := httptest.NewRecorder()
(&handler{}).ServeHTTP(resp, req)
@@ -359,11 +379,12 @@ func (s *IntegrationSuite) TestRange(c *check.C) {
func (s *IntegrationSuite) testVhostRedirectTokenToCookie(c *check.C, method, hostPath, queryString, contentType, reqBody string, expectStatus int, expectRespBody string) *httptest.ResponseRecorder {
u, _ := url.Parse(`http://` + hostPath + queryString)
req := &http.Request{
- Method: method,
- Host: u.Host,
- URL: u,
- Header: http.Header{"Content-Type": {contentType}},
- Body: ioutil.NopCloser(strings.NewReader(reqBody)),
+ Method: method,
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{"Content-Type": {contentType}},
+ Body: ioutil.NopCloser(strings.NewReader(reqBody)),
}
resp := httptest.NewRecorder()
@@ -376,15 +397,16 @@ func (s *IntegrationSuite) testVhostRedirectTokenToCookie(c *check.C, method, ho
if resp.Code != http.StatusSeeOther {
return resp
}
- c.Check(resp.Body.String(), check.Matches, `.*href="//`+regexp.QuoteMeta(html.EscapeString(hostPath))+`".*`)
+ c.Check(resp.Body.String(), check.Matches, `.*href="//`+regexp.QuoteMeta(html.EscapeString(hostPath))+`(\?[^"]*)?".*`)
cookies := (&http.Response{Header: resp.Header()}).Cookies()
u, _ = u.Parse(resp.Header().Get("Location"))
req = &http.Request{
- Method: "GET",
- Host: u.Host,
- URL: u,
- Header: http.Header{},
+ Method: "GET",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{},
}
for _, c := range cookies {
req.AddCookie(c)
commit 9d2a99db8f9b1259d94f650c550d2101e5d915d5
Author: Tom Clegg <tom at curoverse.com>
Date: Sun Nov 8 06:39:05 2015 -0500
5824: Support partial content with Range header (only if start==0).
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index e8678fa..962c5e1 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -9,6 +9,7 @@ import (
"net/http"
"net/url"
"os"
+ "regexp"
"strconv"
"strings"
@@ -293,8 +294,10 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
}
defer rdr.Close()
- // One or both of these can be -1 if not found:
basenamePos := strings.LastIndex(filename, "/")
+ if basenamePos < 0 {
+ basenamePos = 0
+ }
extPos := strings.LastIndex(filename, ".")
if extPos > basenamePos {
// Now extPos is safely >= 0.
@@ -306,8 +309,41 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Length", fmt.Sprintf("%d", rdr.Len()))
}
+ applyContentDispositionHdr(w, r, filename[basenamePos:], attachment)
+ rangeRdr, statusCode := applyRangeHdr(w, r, rdr)
+
+ w.WriteHeader(statusCode)
+ _, err = io.Copy(w, rangeRdr)
+ if err != nil {
+ statusCode, statusText = http.StatusBadGateway, err.Error()
+ }
+}
+
+var rangeRe = regexp.MustCompile(`^bytes=0-([0-9]*)$`)
+
+func applyRangeHdr(w http.ResponseWriter, r *http.Request, rdr keepclient.ReadCloserWithLen) (io.Reader, int) {
+ w.Header().Set("Accept-Ranges", "bytes")
+ hdr := r.Header.Get("Range")
+ fields := rangeRe.FindStringSubmatch(hdr)
+ if fields == nil {
+ return rdr, http.StatusOK
+ }
+ rangeEnd, err := strconv.ParseInt(fields[1], 10, 64)
+ if err != nil {
+ // Empty or too big for int64 == send entire content
+ return rdr, http.StatusOK
+ }
+ if uint64(rangeEnd) >= rdr.Len() {
+ return rdr, http.StatusOK
+ }
+ w.Header().Set("Content-Length", fmt.Sprintf("%d", rangeEnd+1))
+ w.Header().Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", 0, rangeEnd, rdr.Len()))
+ return &io.LimitedReader{R: rdr, N: rangeEnd + 1}, http.StatusPartialContent
+}
+
+func applyContentDispositionHdr(w http.ResponseWriter, r *http.Request, filename string, isAttachment bool) {
disposition := "inline"
- if attachment {
+ if isAttachment {
disposition = "attachment"
}
if strings.ContainsRune(r.RequestURI, '?') {
@@ -316,18 +352,9 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
// "filename.txt?disposition=attachment".
//
// TODO(TC): Follow advice at RFC 6266 appendix D
- if basenamePos < 0 {
- basenamePos = 0
- }
- disposition += "; filename=" + strconv.QuoteToASCII(filename[basenamePos:])
+ disposition += "; filename=" + strconv.QuoteToASCII(filename)
}
if disposition != "inline" {
w.Header().Set("Content-Disposition", disposition)
}
-
- w.WriteHeader(http.StatusOK)
- _, err = io.Copy(w, rdr)
- if err != nil {
- statusCode, statusText = http.StatusBadGateway, err.Error()
- }
}
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 392de94..b4758e0 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -315,6 +315,47 @@ func (s *IntegrationSuite) TestAnonymousTokenError(c *check.C) {
)
}
+func (s *IntegrationSuite) TestRange(c *check.C) {
+ u, _ := url.Parse("http://example.com/c="+arvadostest.HelloWorldCollection+"/Hello%20world.txt")
+ req := &http.Request{
+ Method: "GET",
+ Host: u.Host,
+ URL: u,
+ Header: http.Header{"Range": {"bytes=0-4"}},
+ }
+ resp := httptest.NewRecorder()
+ (&handler{}).ServeHTTP(resp, req)
+ c.Check(resp.Code, check.Equals, http.StatusPartialContent)
+ c.Check(resp.Body.String(), check.Equals, "Hello")
+ c.Check(resp.Header().Get("Content-Length"), check.Equals, "5")
+ c.Check(resp.Header().Get("Content-Range"), check.Equals, "bytes 0-4/12")
+
+ req.Header.Set("Range", "bytes=0-")
+ resp = httptest.NewRecorder()
+ (&handler{}).ServeHTTP(resp, req)
+ // 200 and 206 are both correct:
+ c.Check(resp.Code, check.Equals, http.StatusOK)
+ c.Check(resp.Body.String(), check.Equals, "Hello world\n")
+ c.Check(resp.Header().Get("Content-Length"), check.Equals, "12")
+
+ // Unsupported ranges are ignored
+ for _, hdr := range []string{
+ "bytes=5-5", // non-zero start byte
+ "bytes=-5", // last 5 bytes
+ "cubits=0-5", // unsupported unit
+ "bytes=0-340282366920938463463374607431768211456", // 2^128
+ } {
+ req.Header.Set("Range", hdr)
+ resp = httptest.NewRecorder()
+ (&handler{}).ServeHTTP(resp, req)
+ c.Check(resp.Code, check.Equals, http.StatusOK)
+ c.Check(resp.Body.String(), check.Equals, "Hello world\n")
+ c.Check(resp.Header().Get("Content-Length"), check.Equals, "12")
+ c.Check(resp.Header().Get("Content-Range"), check.Equals, "")
+ c.Check(resp.Header().Get("Accept-Ranges"), check.Equals, "bytes")
+ }
+}
+
func (s *IntegrationSuite) testVhostRedirectTokenToCookie(c *check.C, method, hostPath, queryString, contentType, reqBody string, expectStatus int, expectRespBody string) *httptest.ResponseRecorder {
u, _ := url.Parse(`http://` + hostPath + queryString)
req := &http.Request{
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list