Fix support for spaces in parameters (#49)

* Add failing test get_user_rooms where id contains spaces

* Fix escaping of parameters containing spaces

* Add pending changes to CHANGELOG

* Release 1.9.1

CHANGELOG.md

@@ -6,6 +6,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased](/~https://github.com/pusher/chatkit-server-ruby/compare/1.7.1...HEAD)]
 
+## [1.9.1](/~https://github.com/pusher/chatkit-server-ruby/compare/1.9.0...1.9.1)
+
+### Fixes
+
+- Fix support for parameters which allow spaces.
+  Methods whose parameters contain spaces and were interpolated into URI path
+  segments were incorrectly encoded. For example, user ids containing spaces
+  when passed to get_user_rooms had spaces encoded as '+' (suitable for
+  query strings) not '%20'.
+
 ## [1.9.0](/~https://github.com/pusher/chatkit-server-ruby/compare/1.7.1...1.8.0)
 
 ### Additions

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    pusher-chatkit-server (1.9.0)
+    pusher-chatkit-server (1.9.1)
       pusher-platform (~> 0.11.2)
 
 GEM

chatkit.gemspec

@@ -1,6 +1,6 @@
 Gem::Specification.new do |s|
   s.name        = 'pusher-chatkit-server'
-  s.version     = '1.9.0'
+  s.version     = '1.9.1'
   s.licenses    = ['MIT']
   s.summary     = 'Pusher Chatkit Ruby SDK'
   s.authors     = ['Pusher']

lib/chatkit/client.rb

@@ -88,6 +88,13 @@ def generate_su_token(options = {})
       generate_access_token({ su: true }.merge(options))
     end
 
+    # This helper should be used to encode parameters that appear in path segments.
+    # CGI::escape should NOT be used as it treats the string as if it appears in a query string.
+    # E.G. We want "user name" to be encoded as "user%20name" rather than "user+name"
+    def url_encode_path_segment(s)
+      ERB::Util.url_encode(s)
+    end
+
     # User API
 
     def create_user(options)
@@ -145,7 +152,7 @@ def update_user(options)
 
       api_request({
         method: "PUT",
-        path: "/users/#{CGI::escape options[:id]}",
+        path: "/users/#{url_encode_path_segment options[:id]}",
         body: payload,
         jwt: generate_su_token({ user_id: options[:id] })[:token]
       })
@@ -158,7 +165,7 @@ def async_delete_user(options)
 
       scheduler_request({
         method: "PUT",
-        path: "/users/#{CGI::escape options[:id]}",
+        path: "/users/#{url_encode_path_segment options[:id]}",
         jwt: generate_su_token[:token]
       })
     end
@@ -170,7 +177,7 @@ def get_delete_status(options)
 
       scheduler_request({
         method: "GET",
-        path: "/status/#{CGI::escape options[:id]}",
+        path: "/status/#{url_encode_path_segment options[:id]}",
         jwt: generate_su_token[:token]
       })
     end
@@ -182,7 +189,7 @@ def get_user(options)
 
       api_request({
         method: "GET",
-        path: "/users/#{CGI::escape options[:id]}",
+        path: "/users/#{url_encode_path_segment options[:id]}",
         jwt: generate_su_token[:token]
       })
     end
@@ -267,7 +274,7 @@ def update_room(options)
 
       api_request({
         method: "PUT",
-        path: "/rooms/#{CGI::escape options[:id]}",
+        path: "/rooms/#{url_encode_path_segment options[:id]}",
         body: payload,
         jwt: generate_su_token[:token]
       })
@@ -280,7 +287,7 @@ def async_delete_room(options)
 
       scheduler_request({
         method: "PUT",
-        path: "/rooms/#{CGI::escape options[:id]}",
+        path: "/rooms/#{url_encode_path_segment options[:id]}",
         jwt: generate_su_token[:token]
       })
     end
@@ -292,7 +299,7 @@ def get_room(options)
 
       api_request({
         method: "GET",
-        path: "/rooms/#{CGI::escape options[:id]}",
+        path: "/rooms/#{url_encode_path_segment options[:id]}",
         jwt: generate_su_token[:token]
       })
     end
@@ -337,7 +344,7 @@ def add_users_to_room(options)
 
       api_request({
         method: "PUT",
-        path: "/rooms/#{CGI::escape options[:room_id]}/users/add",
+        path: "/rooms/#{url_encode_path_segment options[:room_id]}/users/add",
         body: { user_ids: options[:user_ids] },
         jwt: generate_su_token[:token]
       })
@@ -354,7 +361,7 @@ def remove_users_from_room(options)
 
       api_request({
         method: "PUT",
-        path: "/rooms/#{CGI::escape options[:room_id]}/users/remove",
+        path: "/rooms/#{url_encode_path_segment options[:room_id]}/users/remove",
         body: { user_ids: options[:user_ids] },
         jwt: generate_su_token[:token]
       })
@@ -370,7 +377,7 @@ def fetch_multipart_message(options)
 
       api_request({
         method: "GET",
-        path: "/rooms/#{CGI::escape options[:room_id]}/messages/#{options[:message_id]}",
+        path: "/rooms/#{url_encode_path_segment options[:room_id]}/messages/#{options[:message_id]}",
         jwt: generate_su_token[:token]
       })
     end
@@ -392,7 +399,7 @@ def fetch_multipart_messages(options)
 
       api_request({
         method: "GET",
-        path: "/rooms/#{CGI::escape options[:room_id]}/messages",
+        path: "/rooms/#{url_encode_path_segment options[:room_id]}/messages",
         query: query_params,
         jwt: generate_su_token[:token]
       })
@@ -410,7 +417,7 @@ def get_room_messages(options)
 
       api_v2_request({
         method: "GET",
-        path: "/rooms/#{CGI::escape options[:room_id]}/messages",
+        path: "/rooms/#{url_encode_path_segment options[:room_id]}/messages",
         query: query_params,
         jwt: generate_su_token[:token]
       })
@@ -467,7 +474,7 @@ def send_multipart_message(options)
 
       api_request({
         method: "POST",
-        path: "/rooms/#{CGI::escape options[:room_id]}/messages",
+        path: "/rooms/#{url_encode_path_segment options[:room_id]}/messages",
         body: {parts: request_parts},
         jwt: token
       })
@@ -509,7 +516,7 @@ def send_message(options)
 
       api_v2_request({
         method: "POST",
-        path: "/rooms/#{CGI::escape options[:room_id]}/messages",
+        path: "/rooms/#{url_encode_path_segment options[:room_id]}/messages",
         body: payload,
         jwt: generate_su_token({ user_id: options[:sender_id] })[:token]
       })
@@ -591,7 +598,7 @@ def edit_multipart_message(room_id, message_id, options)
 
       api_request({
         method: "PUT",
-        path: "/rooms/#{CGI::escape room_id}/messages/#{message_id}",
+        path: "/rooms/#{url_encode_path_segment room_id}/messages/#{message_id}",
         body: {parts: request_parts},
         jwt: token
       })
@@ -645,7 +652,7 @@ def edit_message(room_id, message_id, options)
 
       api_v2_request({
         method: "PUT",
-        path: "/rooms/#{CGI::escape room_id}/messages/#{message_id}",
+        path: "/rooms/#{url_encode_path_segment room_id}/messages/#{message_id}",
         body: payload,
         jwt: generate_su_token({ user_id: options[:sender_id] })[:token]
       })
@@ -700,7 +707,7 @@ def get_user_roles(options)
 
       authorizer_request({
         method: "GET",
-        path: "/users/#{CGI::escape options[:user_id]}/roles",
+        path: "/users/#{url_encode_path_segment options[:user_id]}/roles",
         jwt: generate_su_token[:token]
       })
     end
@@ -750,7 +757,7 @@ def get_read_cursor(options)
 
       cursors_request({
         method: "GET",
-        path: "/cursors/0/rooms/#{CGI::escape options[:room_id]}/users/#{CGI::escape options[:user_id]}",
+        path: "/cursors/0/rooms/#{url_encode_path_segment options[:room_id]}/users/#{url_encode_path_segment options[:user_id]}",
         jwt: generate_su_token[:token]
       })
     end
@@ -770,7 +777,7 @@ def set_read_cursor(options)
 
       cursors_request({
         method: "PUT",
-        path: "/cursors/0/rooms/#{CGI::escape options[:room_id]}/users/#{CGI::escape options[:user_id]}",
+        path: "/cursors/0/rooms/#{url_encode_path_segment options[:room_id]}/users/#{url_encode_path_segment options[:user_id]}",
         body: { position: options[:position] },
         jwt: generate_su_token[:token]
       })
@@ -783,7 +790,7 @@ def get_user_read_cursors(options)
 
       cursors_request({
         method: "GET",
-        path: "/cursors/0/users/#{CGI::escape options[:user_id]}",
+        path: "/cursors/0/users/#{url_encode_path_segment options[:user_id]}",
         jwt: generate_su_token[:token]
       })
     end
@@ -795,7 +802,7 @@ def get_room_read_cursors(options)
 
       cursors_request({
         method: "GET",
-        path: "/cursors/0/rooms/#{CGI::escape options[:room_id]}",
+        path: "/cursors/0/rooms/#{url_encode_path_segment options[:room_id]}",
         jwt: generate_su_token[:token]
       })
     end
@@ -852,7 +859,7 @@ def get_rooms_for_user(options)
 
       request_options = {
         method: "GET",
-        path: "/users/#{CGI::escape options[:id]}/rooms",
+        path: "/users/#{url_encode_path_segment options[:id]}/rooms",
         jwt: generate_su_token[:token]
       }
 
@@ -891,7 +898,7 @@ def delete_role(options)
 
       authorizer_request({
         method: "DELETE",
-        path: "/roles/#{CGI::escape options[:name]}/scope/#{options[:scope]}",
+        path: "/roles/#{url_encode_path_segment options[:name]}/scope/#{options[:scope]}",
         jwt: generate_su_token[:token]
       })
     end
@@ -913,7 +920,7 @@ def assign_role_to_user(options)
 
       authorizer_request({
         method: "PUT",
-        path: "/users/#{CGI::escape options[:user_id]}/roles",
+        path: "/users/#{url_encode_path_segment options[:user_id]}/roles",
         body: body,
         jwt: generate_su_token[:token]
       })
@@ -926,7 +933,7 @@ def remove_role_for_user(options)
 
       request_options = {
         method: "DELETE",
-        path: "/users/#{CGI::escape options[:user_id]}/roles",
+        path: "/users/#{url_encode_path_segment options[:user_id]}/roles",
         jwt: generate_su_token[:token]
       }
 
@@ -944,7 +951,7 @@ def get_permissions_for_role(options)
 
       authorizer_request({
         method: "GET",
-        path: "/roles/#{CGI::escape options[:name]}/scope/#{options[:scope]}/permissions",
+        path: "/roles/#{url_encode_path_segment options[:name]}/scope/#{options[:scope]}/permissions",
         jwt: generate_su_token[:token]
       })
     end
@@ -967,7 +974,7 @@ def update_permissions_for_role(options)
 
       authorizer_request({
         method: "PUT",
-        path: "/roles/#{CGI::escape options[:name]}/scope/#{options[:scope]}/permissions",
+        path: "/roles/#{url_encode_path_segment options[:name]}/scope/#{options[:scope]}/permissions",
         body: body,
         jwt: generate_su_token[:token]
       })
@@ -992,7 +999,7 @@ def upload_attachment(token, room_id, part)
 
       attachment_response = api_request({
         method: "POST",
-        path: "/rooms/#{CGI::escape room_id}/attachments",
+        path: "/rooms/#{url_encode_path_segment room_id}/attachments",
         body: attachment_req,
         jwt: token
       })

spec/client_spec.rb

@@ -701,6 +701,23 @@
         expect(get_user_rooms_res[:body][0][:member_user_ids]).to eq [user_id]
       end
 
+      it "an id is provided which contains spaces" do
+        user_id = SecureRandom.uuid + " with spaces"
+        create_res = @chatkit.create_user({ id: user_id, name: 'Ham' })
+        expect(create_res[:status]).to eq 201
+
+        room_res = @chatkit.create_room({ creator_id: user_id, name: 'my room' })
+        expect(room_res[:status]).to eq 201
+
+        get_user_rooms_res = @chatkit.get_user_rooms({ id: user_id })
+        expect(get_user_rooms_res[:status]).to eq 200
+        expect(get_user_rooms_res[:body].count).to eq 1
+        expect(get_user_rooms_res[:body][0][:id]).to eq room_res[:body][:id]
+        expect(get_user_rooms_res[:body][0][:name]).to eq 'my room'
+        expect(get_user_rooms_res[:body][0][:private]).to be false
+        expect(get_user_rooms_res[:body][0][:member_user_ids]).to eq [user_id]
+      end
+
       it "an id is provided and only return the correct rooms" do
         user_id = SecureRandom.uuid
         user_id2 = SecureRandom.uuid