From 2882bca4839fe84c5f1b9587d799885c4844d247 Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Sat, 20 Oct 2012 11:32:34 +0200 Subject: [PATCH 1/2] Fix user filtering for directory listings --- lib/remote_storage/riak.rb | 36 ++++++++++++++++++++++++++++++------ spec/directories_spec.rb | 27 ++++++++++++++++++++++++++- spec/spec_helper.rb | 1 + 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/lib/remote_storage/riak.rb b/lib/remote_storage/riak.rb index 75ba9c7..55435f9 100644 --- a/lib/remote_storage/riak.rb +++ b/lib/remote_storage/riak.rb @@ -5,6 +5,8 @@ require "cgi" module RemoteStorage module Riak + ::Riak.url_decoding = true + def client @client ||= ::Riak::Client.new(LiquorCabinet.config['riak'].symbolize_keys) end @@ -134,6 +136,13 @@ module RemoteStorage def directory_entries(user, directory) directory = "/" if directory == "" + + user_keys = data_bucket.get_index("user_id_bin", user) + directory_keys = data_bucket.get_index("directory_bin", directory) + + all_keys = user_keys & directory_keys + return [] if all_keys.empty? + map_query = <<-EOH function(v){ keys = v.key.split(':'); @@ -145,15 +154,26 @@ module RemoteStorage }]; } EOH - objects = ::Riak::MapReduce.new(client). - index("user_data", "user_id_bin", user). - index("user_data", "directory_bin", directory). + + map_reduce = ::Riak::MapReduce.new(client) + all_keys.each do |key| + map_reduce.add("user_data", key) + end + + map_reduce. map(map_query, :keep => true). run end def sub_directories(user, directory) directory = "/" if directory == "" + + user_keys = directory_bucket.get_index("user_id_bin", user) + directory_keys = directory_bucket.get_index("directory_bin", directory) + + all_keys = user_keys & directory_keys + return [] if all_keys.empty? + map_query = <<-EOH function(v){ keys = v.key.split(':'); @@ -165,9 +185,13 @@ module RemoteStorage }]; } EOH - objects = ::Riak::MapReduce.new(client). - index("rs_directories", "user_id_bin", user). - index("rs_directories", "directory_bin", directory). + + map_reduce = ::Riak::MapReduce.new(client) + all_keys.each do |key| + map_reduce.add("rs_directories", key) + end + + map_reduce. map(map_query, :keep => true). run end diff --git a/spec/directories_spec.rb b/spec/directories_spec.rb index 13c1f5d..a1251a8 100644 --- a/spec/directories_spec.rb +++ b/spec/directories_spec.rb @@ -37,12 +37,14 @@ describe "Directories" do it "has a Last-Modifier header set" do get "/jimmy/tasks/" + last_response.status.must_equal 200 last_response.headers["Last-Modified"].wont_be_nil end it "has CORS headers set" do get "/jimmy/tasks/" + last_response.status.must_equal 200 last_response.headers["Access-Control-Allow-Origin"].must_equal "*" last_response.headers["Access-Control-Allow-Methods"].must_equal "GET, PUT, DELETE" last_response.headers["Access-Control-Allow-Headers"].must_equal "Authorization, Content-Type, Origin" @@ -66,6 +68,30 @@ describe "Directories" do content["home/"].to_s.length.must_be :>=, 10 end + context "for a different user" do + before do + auth = auth_bucket.new("alice:321") + auth.data = [":r", "documents:r", "tasks:rw"] + auth.store + + header "Authorization", "Bearer 321" + + put "/alice/tasks/homework", "write an essay" + end + + it "does not list the directories of jimmy" do + get "/alice/tasks/" + + last_response.status.must_equal 200 + + content = JSON.parse(last_response.body) + content.wont_include "/" + content.wont_include "tasks/" + content.wont_include "home/" + content.must_include "homework" + end + end + context "sub-directories without objects" do it "lists the direct sub-directories" do put "/jimmy/tasks/private/projects/world-domination/start", "write a manifesto" @@ -240,7 +266,6 @@ describe "Directories" do describe "DELETE file" do context "last file in directory" do before do - directory_bucket.delete("jimmy:tasks") put "/jimmy/tasks/home/trash", "take out the trash" end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b22bcee..1e9a6b0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -16,6 +16,7 @@ riak_config = YAML.load(config)[ENV['RACK_ENV']]['riak'].symbolize_keys set :riak_config, riak_config ::Riak.disable_list_keys_warnings = true +::Riak.url_decoding = true def app LiquorCabinet From 88f61bf7fa45f84d776be6600f3e028f2ea5a87d Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Sat, 20 Oct 2012 11:36:01 +0200 Subject: [PATCH 2/2] Fix handling of keys containing colons (fixes #13) --- lib/remote_storage/riak.rb | 5 +++-- spec/directories_spec.rb | 6 +++--- spec/riak_spec.rb | 13 +++++++++++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/remote_storage/riak.rb b/lib/remote_storage/riak.rb index 55435f9..c8d0c13 100644 --- a/lib/remote_storage/riak.rb +++ b/lib/remote_storage/riak.rb @@ -128,7 +128,7 @@ module RemoteStorage directory_entries(user, directory).each do |entry| timestamp = DateTime.rfc2822(entry["last_modified"]).to_i - listing.merge!({ entry["name"] => timestamp }) + listing.merge!({ CGI.unescape(entry["name"]) => timestamp }) end listing @@ -146,7 +146,8 @@ module RemoteStorage map_query = <<-EOH function(v){ keys = v.key.split(':'); - key_name = keys[keys.length-1]; + keys.splice(0, 2); + key_name = keys.join(':'); last_modified_date = v.values[0]['metadata']['X-Riak-Last-Modified']; return [{ name: key_name, diff --git a/spec/directories_spec.rb b/spec/directories_spec.rb index a1251a8..cf3a2da 100644 --- a/spec/directories_spec.rb +++ b/spec/directories_spec.rb @@ -18,7 +18,7 @@ describe "Directories" do before do put "/jimmy/tasks/foo", "do the laundry" - put "/jimmy/tasks/bar", "do the laundry" + put "/jimmy/tasks/http%3A%2F%2F5apps.com", "prettify design" end it "lists the objects with a timestamp of the last modification" do @@ -28,7 +28,7 @@ describe "Directories" do last_response.content_type.must_equal "application/json" content = JSON.parse(last_response.body) - content.must_include "bar" + content.must_include "http://5apps.com" content.must_include "foo" content["foo"].to_s.must_match /\d+/ content["foo"].to_s.length.must_be :>=, 10 @@ -62,7 +62,7 @@ describe "Directories" do content = JSON.parse(last_response.body) content.must_include "foo" - content.must_include "bar" + content.must_include "http://5apps.com" content.must_include "home/" content["home/"].to_s.must_match /\d+/ content["home/"].to_s.length.must_be :>=, 10 diff --git a/spec/riak_spec.rb b/spec/riak_spec.rb index a2d6ccb..0962337 100644 --- a/spec/riak_spec.rb +++ b/spec/riak_spec.rb @@ -174,6 +174,19 @@ describe "App with Riak backend" do end end + context "with escaped key" do + before do + put "/jimmy/documents/http%3A%2F%2F5apps.com", "super website" + end + + it "delivers the data correctly" do + header "Authorization", "Bearer 123" + get "/jimmy/documents/http%3A%2F%2F5apps.com" + + last_response.body.must_equal 'super website' + end + end + context "invalid JSON" do context "empty body" do before do