URL-encode param values in GET query strings (#41)

* URL-encode param values in GET query strings

Commit 00b6b67 (tag: v0.1.12)
"Fix request helper for get requests to build URI based on how API expects it"
attempted to solve some unclearly-indentified problem with encoding
the query string in GET-based API calls.  The commit leaves param values
unencoded/as-is instead of URL-encoding them.

Unfortunately, that causes a regression in that param values containing
spaces lead to the error "EOFError: end of file reached".

I'm not sure what regressions we will cause by _reverting_ 00b6b67,
but _that_ implementation is clearly broken.

* Remove support for the bogus im.counters?username= param

The `/api/v1/im.counters` endpoint does not support such a parameter.

* Do not pass `nil` param values to the RC API endpoints

* Add support for the optional im.counters?userId= param

Refactor all im.counters tests

* Add specs for RequestHelper#create_request

lib/rocket_chat/messages/im.rb

@@ -78,16 +78,16 @@ def list_everyone(offset: nil, count: nil, sort: nil, fields: nil, query: nil)
       #
       # im.counters REST API
       # @param [String] room_id Rocket.Chat roomId
-      # @param [String] username Rocket.Chat username
+      # @param [String] user_id Rocket.Chat userId [optional]
       # @return [RocketChat::ImSummary]
       # @raise [HTTPError, StatusError]
       #
-      def counters(room_id:, username: nil)
+      def counters(room_id:, user_id: nil)
         response = session.request_json(
           '/api/v1/im.counters',
           body: {
             roomId: room_id,
-            username: username
+            userId: user_id
           }
         )
         RocketChat::ImSummary.new response

lib/rocket_chat/request_helper.rb

@@ -104,14 +104,14 @@ def create_http(options)
 
     def create_request(path, options)
       headers = get_headers(options)
-      body = options[:body]
+      body = options[:body]&.reject { |_key, value| value.nil? }
 
       if options[:method] == :post
         req = Net::HTTP::Post.new(path, headers)
         add_body(req, body) if body
       else
         uri = path
-        uri += "?#{body.map { |k, v| "#{k}=#{v}" }.join('&')}" if body
+        uri += "?#{URI.encode_www_form(body)}" if body
         req = Net::HTTP::Get.new(uri, headers)
       end
 

spec/rocket_chat/messages/im_spec.rb

@@ -313,97 +313,95 @@
 
   describe '#counters' do
     before do
-      stub_unauthed_request :get, '/api/v1/im.counters?roomId=rocket.cat&username='
+      stub_unauthed_request :get, "/api/v1/im.counters?roomId=#{ROOM_ID}"
 
-      stub_authed_request(:get, '/api/v1/im.counters?roomId=rocket.cat&username=')
+      stub_authed_request(:get, "/api/v1/im.counters?roomId=#{ROOM_ID}")
         .to_return(
           body: {
             joined: true,
             members: 2,
-            unreads: 0,
-            unreadsFrom: '2019-01-04T21:40:11.251Z',
-            msgs: 0,
-            latest: '2019-01-04T21:40:11.251Z',
-            userMentions: 0,
+            unreads: 3,
+            unreadsFrom: '2019-01-01T12:34:56.789Z',
+            msgs: 4,
+            latest: '2019-01-23T01:23:45.678Z',
+            userMentions: 5,
             success: true
           }.to_json,
           status: 200
         )
 
-      stub_authed_request(:get, '/api/v1/im.counters?roomId=rocket.cat&username=user.test')
-        .to_return(
-          body: {
-            joined: true,
-            members: 2,
-            unreads: 1,
-            unreadsFrom: '2019-01-05T20:37:09.130Z',
-            msgs: 0,
-            latest: '2019-01-05T20:37:09.130Z',
-            userMentions: 0,
-            success: true
-          }.to_json,
-          status: 200
-        )
+      stub_authed_request(:get, "/api/v1/im.counters?roomId=#{BOGUS_ROOM_ID}")
+        .to_return(UNAUTHORIZED)
 
-      stub_authed_request(:get, '/api/v1/im.counters?roomId=1234&username=')
+      stub_authed_request(:get, '/api/v1/im.counters?roomId=')
         .to_return(
           body: {
             success: false,
-            error: '[invalid-channel]',
-            errorType: 'invalid-channel'
-          }.to_json,
-          status: 400
-        )
-
-      stub_authed_request(:get, '/api/v1/im.counters?roomId=&username=')
-        .to_return(
-          body: {
-            success: false,
-            error: 'Body param "roomId" or "username" is required [error-room-param-not-provided]',
+            error: 'Query param "roomId" is required [error-room-param-not-provided]',
             errorType: 'error-room-param-not-provided'
           }.to_json,
           status: 400
         )
+
+      stub_authed_request(:get, "/api/v1/im.counters?roomId=#{ROOM_ID}&userId=#{USER_ID}")
+        .to_return(UNAUTHORIZED)
     end
 
-    context 'with an invalid session token' do
-      let(:token) { RocketChat::Token.new(authToken: nil, userId: nil) }
+    context 'when called with valid room_id' do
+      it 'returns counters for that DM room' do # rubocop:disable RSpec/MultipleExpectations
+        counters = session.im.counters room_id: ROOM_ID
+        expect(counters).to be_a RocketChat::ImSummary
+        expect(counters.joined).to be true
+        expect(counters.members).to eq 2
+        expect(counters.unreads).to eq 3
+        expect(counters.unreads_from).to eq '2019-01-01T12:34:56.789Z'
+        expect(counters.msgs).to eq 4
+        expect(counters.latest).to eq '2019-01-23T01:23:45.678Z'
+        expect(counters.user_mentions).to eq 5
+      end
+    end
 
-      it 'raises a status error' do
+    context 'when called with bogus room_id' do
+      it 'raises StatusError' do
         expect do
-          session.im.counters room_id: 'rocket.cat'
-        end.to raise_error RocketChat::StatusError, 'You must be logged in to do this.'
+          session.im.counters room_id: BOGUS_ROOM_ID
+        end.to raise_error RocketChat::StatusError, UNAUTHORIZED_MESSAGE
       end
     end
 
-    context 'with a valid session' do
-      it 'get quantity of messages' do
-        im = session.im.counters room_id: 'rocket.cat'
-        expect(im).to be_a RocketChat::ImSummary
-        expect(im.members).to eq 2
-        expect(im.unreads).to eq 0
-        expect(im.msgs).to eq 0
-        expect(im.user_mentions).to eq 0
+    context 'when called with blank room_id' do
+      it 'raises StatusError' do
+        expect do
+          session.im.counters room_id: ''
+        end.to raise_error RocketChat::StatusError, 'Query param "roomId" is required [error-room-param-not-provided]'
       end
+    end
 
-      it 'get quantity of messages specifying the username' do
-        im = session.im.counters room_id: 'rocket.cat', username: 'user.test'
-        expect(im.joined).to be true
-        expect(im.unreads_from).to eq '2019-01-05T20:37:09.130Z'
-        expect(im.latest).to eq '2019-01-05T20:37:09.130Z'
-        expect(im.success).to be true
+    context 'when called without room_id' do
+      it 'raises ArgumentError' do
+        expect do
+          session.im.counters
+        end.to raise_error ArgumentError
       end
+    end
 
-      it 'does not send valid attributes' do
-        expect do
-          session.im.counters room_id: '1234'
-        end.to raise_error RocketChat::StatusError
+    context 'when called with user_id' do
+      context 'when you do not have view-room-administration permission' do
+        it 'raises error' do
+          expect do
+            session.im.counters room_id: ROOM_ID, user_id: USER_ID
+          end.to raise_error RocketChat::StatusError, UNAUTHORIZED_MESSAGE
+        end
       end
+    end
+
+    context 'when called with an invalid session token' do
+      let(:token) { RocketChat::Token.new(authToken: 'bogus-token', userId: USER_ID) }
 
-      it 'does not send any attributes' do
+      it 'raises error' do
         expect do
-          session.im.counters room_id: '', username: ''
-        end.to raise_error RocketChat::StatusError
+          session.im.counters room_id: ROOM_ID
+        end.to raise_error RocketChat::StatusError, UNAUTHORIZED_MESSAGE
       end
     end
   end

spec/rocket_chat/request_helper_spec.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe RocketChat::RequestHelper do
+  describe '#create_request' do
+    let(:including_class) { Class.new { include RocketChat::RequestHelper } }
+    let(:instance) { including_class.new }
+
+    describe 'URI-encoding' do
+      subject(:req) { instance.send(:create_request, '/api/endpoint', body: request_params) }
+
+      context 'when encoding multiple, simple parameters' do
+        let(:request_params) { { foo: 1, bar: 'string' } }
+
+        it 'URI-encodes each of them into the query string' do
+          expect(req.path).to end_with('?foo=1&bar=string')
+        end
+      end
+
+      context 'when encoding parameters containing spaces, ampersands, and other specials' do
+        let(:request_params) { { foo: 'This & That', bar: '+ Plusses, too, with 98% success!' } }
+
+        it 'URI-encodes them properly into the query string' do
+          expect(req.path).to end_with('?foo=This+%26+That&bar=%2B+Plusses%2C+too%2C+with+98%25+success%21')
+        end
+      end
+
+      context 'when encoding empty strings and nil parameters' do
+        let(:request_params) { { empty: '', null: nil } }
+
+        it 'preserves empty strings but drops nils from the query string' do
+          expect(req.path).to end_with('?empty=')
+        end
+      end
+    end
+  end
+end

spec/shared/room_behaviors.rb

@@ -326,12 +326,12 @@
 
       stub_authed_request(:post, described_class.api_path('rename'))
         .with(
-          body: { roomId: nil, name: 'new_room_name' }.to_json
+          body: { name: 'new_room_name' }.to_json
         ).to_return(not_provided_room_body)
 
       stub_authed_request(:post, described_class.api_path('rename'))
         .with(
-          body: { roomId: nil, name: nil }.to_json
+          body: {}.to_json
         ).to_return(
           body: {
             success: false,

spec/spec_helper.rb

@@ -15,12 +15,18 @@
 AUTH_TOKEN = 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
 USER_ID = 'AAAAAAAAAAAAAAAAA'
 OTHER_USER_ID = 'BBBBBBBBBBBBBBBBB'
+ROOM_ID = 'ValidRoomID'
+BOGUS_ROOM_ID = 'BogusRoomID'
 USERNAME = 'user'
 PASSWORD = 'password'
+
+UNAUTHORIZED_MESSAGE = 'You must be logged in to do this.'
 UNAUTHORIZED_BODY = {
   status: :error,
-  message: 'You must be logged in to do this.'
+  message: UNAUTHORIZED_MESSAGE
 }.to_json
+UNAUTHORIZED_STATUS = 401
+UNAUTHORIZED = { body: UNAUTHORIZED_BODY, status: UNAUTHORIZED_STATUS }.freeze
 
 ### Authenticated request helpers
 
@@ -31,5 +37,5 @@ def stub_authed_request(method, action)
 
 def stub_unauthed_request(method, action)
   stub_request(method, SERVER_URI + action)
-    .to_return(body: UNAUTHORIZED_BODY, status: 401)
+    .to_return(UNAUTHORIZED)
 end