[ARVADOS] updated: 1e427ad73a413642bfe35d7865f8b03b8564c1da

[git at public.curoverse.com]

Summary of changes:
 .../app/controllers/collections_controller.rb      | 26 ++++++++++++++++------
 .../controllers/collections_controller_test.rb     | 25 ++++++++++++++++-----
 2 files changed, 38 insertions(+), 13 deletions(-)

       via  1e427ad73a413642bfe35d7865f8b03b8564c1da (commit)
      from  05a38f1bd00d572cbfd67d7276f5bcae6bb24805 (commit)

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 1e427ad73a413642bfe35d7865f8b03b8564c1da
Author: Tom Clegg <tom at curoverse.com>
Date:   Mon Nov 9 15:00:14 2015 -0500

    5824: Preserve query in keep_web_url template. Warn when redirecting preview to a single-origin keep_web_url.

diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index cadef38..2228a07 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -148,7 +148,7 @@ class CollectionsController < ApplicationController
         # to read the collection.
         opts[:query_token] = usable_token
       end
-      opts[:disposition] = params[:disposition]
+      opts[:disposition] = params[:disposition] if params[:disposition]
       return redirect_to keep_web_url(params[:uuid], params[:file], opts)
     end
 
@@ -325,7 +325,8 @@ class CollectionsController < ApplicationController
   end
 
   def keep_web_url(uuid_or_pdh, file, opts)
-    fmt = {uuid_or_pdh: uuid_or_pdh.sub('+', '-')}
+    munged_id = uuid_or_pdh.sub('+', '-')
+    fmt = {uuid_or_pdh: munged_id}
     uri = URI.parse(Rails.configuration.keep_web_url % fmt)
     uri.path += '/' unless uri.path.end_with? '/'
     if opts[:path_token]
@@ -334,15 +335,26 @@ class CollectionsController < ApplicationController
     uri.path += '_/'
     uri.path += CGI::escape(file)
 
-    query_params = []
+    query = CGI::parse(uri.query || '')
     { query_token: 'api_token',
       disposition: 'disposition' }.each do |opt, param|
-      if opts[opt]
-        query_params << param + '=' + CGI::escape(opts[opt])
+      if opts.include? opt
+        query[param] = opts[opt]
       end
     end
-    unless query_params.empty?
-      uri.query = query_params.join '&'
+    unless query.empty?
+      uri.query = query.to_query
+    end
+
+    if query.include? 'api_token' and
+        query['disposition'] != 'attachment' and
+        not uri.host.start_with?(munged_id + "--") and
+        not uri.host.start_with?(munged_id + ".")
+      # keep-web refuses query tokens ("?api_token=X") unless it sees
+      # the collection ID in the hostname, or is running in
+      # attachment-only mode.
+      logger.warn("Single-origin keep_web_url can't serve inline content, " \
+                  "but redirecting anyway: #{uri.to_s}")
     end
 
     uri.to_s
diff --git a/apps/workbench/test/controllers/collections_controller_test.rb b/apps/workbench/test/controllers/collections_controller_test.rb
index 7b0cbb5..c6db9de 100644
--- a/apps/workbench/test/controllers/collections_controller_test.rb
+++ b/apps/workbench/test/controllers/collections_controller_test.rb
@@ -521,7 +521,7 @@ class CollectionsControllerTest < ActionController::TestCase
     assert_not_includes @response.body, '<a href="#Upload"'
   end
 
-  def setup_for_keep_web cfg='https://%{uuid_or_pdh}.dl.zzzzz.example'
+  def setup_for_keep_web cfg='https://%{uuid_or_pdh}.collections.zzzzz.example'
     Rails.configuration.keep_web_url = cfg
     @controller.expects(:file_enumerator).never
   end
@@ -533,7 +533,7 @@ class CollectionsControllerTest < ActionController::TestCase
       id = api_fixture('collections')['w_a_z_file'][id_type]
       get :show_file, {uuid: id, file: "w a z"}, session_for(:active)
       assert_response :redirect
-      assert_equal "https://#{id.sub '+', '-'}.dl.zzzzz.example/_/w+a+z?api_token=#{tok}", @response.redirect_url
+      assert_equal "https://#{id.sub '+', '-'}.collections.zzzzz.example/_/w+a+z?api_token=#{tok}", @response.redirect_url
     end
 
     test "Redirect to keep_web_url via #{id_type} with reader token" do
@@ -542,7 +542,7 @@ class CollectionsControllerTest < ActionController::TestCase
       id = api_fixture('collections')['w_a_z_file'][id_type]
       get :show_file, {uuid: id, file: "w a z", reader_token: tok}, session_for(:expired)
       assert_response :redirect
-      assert_equal "https://#{id.sub '+', '-'}.dl.zzzzz.example/t=#{tok}/_/w+a+z", @response.redirect_url
+      assert_equal "https://#{id.sub '+', '-'}.collections.zzzzz.example/t=#{tok}/_/w+a+z", @response.redirect_url
     end
 
     test "Redirect to keep_web_url via #{id_type} with no token" do
@@ -551,16 +551,29 @@ class CollectionsControllerTest < ActionController::TestCase
       id = api_fixture('collections')['public_text_file'][id_type]
       get :show_file, {uuid: id, file: "Hello World.txt"}
       assert_response :redirect
-      assert_equal "https://#{id.sub '+', '-'}.dl.zzzzz.example/_/Hello+World.txt", @response.redirect_url
+      assert_equal "https://#{id.sub '+', '-'}.collections.zzzzz.example/_/Hello+World.txt", @response.redirect_url
+    end
+
+    test "Redirect to keep_web_url via #{id_type} with disposition param" do
+      setup_for_keep_web
+      config_anonymous true
+      id = api_fixture('collections')['public_text_file'][id_type]
+      get :show_file, {
+        uuid: id,
+        file: "Hello World.txt",
+        disposition: 'attachment',
+      }
+      assert_response :redirect
+      assert_equal "https://#{id.sub '+', '-'}.collections.zzzzz.example/_/Hello+World.txt?disposition=attachment", @response.redirect_url
     end
 
     test "Redirect to keep_web_url via #{id_type} using -attachment-only-host mode" do
-      setup_for_keep_web 'https://dl.zzzzz.example/c=%{uuid_or_pdh}'
+      setup_for_keep_web 'https://collections.zzzzz.example/c=%{uuid_or_pdh}'
       tok = api_fixture('api_client_authorizations')['active']['api_token']
       id = api_fixture('collections')['w_a_z_file'][id_type]
       get :show_file, {uuid: id, file: "w a z"}, session_for(:active)
       assert_response :redirect
-      assert_equal "https://dl.zzzzz.example/c=#{id.sub '+', '-'}/_/w+a+z?api_token=#{tok}", @response.redirect_url
+      assert_equal "https://collections.zzzzz.example/c=#{id.sub '+', '-'}/_/w+a+z?api_token=#{tok}", @response.redirect_url
     end
   end
 

-----------------------------------------------------------------------


hooks/post-receive
--