Fix request_signer body serialization and wildcard double-encoding

CHANGELOG.md

@@ -8,6 +8,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 ### Deprecated
 ### Removed
 ### Fixed
+- Fixed `request_signer` receiving the raw body object instead of the serialized string, causing signature mismatches ([#311](https://github.com/opensearch-project/opensearch-ruby/issues/311))
+- Fixed double-encoding of wildcard and special characters in index/path parameters (e.g. `index: "test-*"` was being sent as `test-%252A`) ([#319](https://github.com/opensearch-project/opensearch-ruby/issues/319))
 ### Security
 
 ## [4.0.0]

lib/opensearch/api/utils.rb

@@ -9,7 +9,7 @@
 
 # frozen_string_literal: true
 
-require 'cgi/escape'
+require 'uri'
 
 module OpenSearch
   module API
@@ -38,11 +38,14 @@ def self.validate_query_params!(params, valid_param_names = nil)
       end
 
       # @return the value escaped for URL unless it is a Hash
+      # Uses URI encoding rather than CGI.escape so that characters meaningful in
+      # OpenSearch index patterns (e.g. '*') are preserved and not percent-encoded,
+      # which would cause double-encoding when the HTTP layer encodes the URL.
       def self.normalize_value(value)
         return value.clone if value.is_a? Hash
         value = value.to_s.strip unless value.is_a? Enumerable
         value = value.split(',') if value.is_a? String
-        value.map { |v| CGI.escape(v.to_s) }.join(',')
+        value.map { |v| URI::DEFAULT_PARSER.escape(v.to_s, /[^A-Za-z0-9\-._~*]/) }.join(',')
       end
 
       def self.build_url(*parts)

lib/opensearch/transport/client.rb

@@ -190,8 +190,13 @@ def perform_request(method, path, params = {}, body = nil, headers = {})
         method = @send_get_body_as if method == 'GET' && body
         if @options[:request_signer]
           connection = transport.get_connection
+          # Serialize body to the same string that will be sent on the wire so
+          # the signer computes a signature over the exact bytes transmitted.
+          # The transport uses __convert_to_json (which returns a String as-is
+          # and serializes everything else), so we mirror that here.
+          serialized_body = body.is_a?(String) ? body : (body && transport.serializer.dump(body))
           headers = @options[:request_signer].sign_request(
-            method: method, path: path, params: params, body: body, headers: headers,
+            method: method, path: path, params: params, body: serialized_body, headers: headers,
             host: connection.host[:host],
             port: connection.host[:port],
             url: connection.full_url(path, params),

spec/opensearch/api/utils_spec.rb

@@ -44,18 +44,44 @@
 
     it 'returns a new hash with keys as strings and values normalized except those of NON_URL_ARGS' do
       is_expected.to eq({
-                          'hello' => 'world+2',
+                          'hello' => 'world%202',
                           'hash' => { bar: 'baz' },
                           'number' => '42',
                           'bool' => 'true',
-                          'string' => 'amazing+spider-man,+big+%26+small',
+                          'string' => 'amazing%20spider-man,%20big%20%26%20small',
                           'nil' => '',
                           'array' => '1,2,3',
                           'headers' => [100]
                         })
     end
   end
 
+  describe '#normalize_value' do
+    it 'preserves wildcard * so index patterns are not double-encoded by the HTTP layer' do
+      expect(described_class.normalize_value('test-*')).to eq('test-*')
+    end
+
+    it 'preserves * in comma-separated multi-index patterns' do
+      expect(described_class.normalize_value('logs-*,metrics-*')).to eq('logs-*,metrics-*')
+    end
+
+    it 'percent-encodes spaces as %20 (not + which would be double-encoded)' do
+      expect(described_class.normalize_value('my index')).to eq('my%20index')
+    end
+
+    it 'percent-encodes & and other special chars' do
+      expect(described_class.normalize_value('a&b')).to eq('a%26b')
+    end
+
+    it 'returns plain strings unchanged' do
+      expect(described_class.normalize_value('my-index_name')).to eq('my-index_name')
+    end
+
+    it 'converts an array of values joining with comma' do
+      expect(described_class.normalize_value(['idx-one', 'idx-*'])).to eq('idx-one,idx-*')
+    end
+  end
+
   describe '#validate_query_params!' do
     subject(:validate) { described_class.validate_query_params!(params, Set.new(%w[foo bar])) }
 

spec/opensearch/client/unit/request_signer_spec.rb

@@ -40,9 +40,48 @@ def self.sign_request(args)
       { query: 'string' },
       { request: :body },
       { header1: 'value1',
-        dummy_header1: { request: :body },
+        dummy_header1: '{"request":"body"}',
         dummy_header2: { query: 'string' },
         dummy_header3: 'localhost' }
     )
   end
+
+  context 'when a body is provided' do
+    let(:captured_bodies) { [] }
+    let(:capturing_signer) do
+      bodies = captured_bodies
+      Class.new do
+        define_singleton_method(:sign_request) do |args|
+          bodies << args[:body]
+          args[:headers]
+        end
+      end
+    end
+    let(:capturing_client) do
+      described_class.new(
+        host: 'http://localhost:9200',
+        request_signer: capturing_signer
+      ).tap do |cli|
+        allow(cli.transport.transport).to receive(:perform_request).and_return({ body: 'ok' })
+      end
+    end
+
+    it 'passes a serialized String to sign_request, not the raw Hash' do
+      body_hash = { query: { match_all: {} } }
+      capturing_client.transport.perform_request('POST', '_search', {}, body_hash, {})
+      expect(captured_bodies.last).to be_a(String)
+      expect(captured_bodies.last).to eq(OpenSearch::API.serializer.dump(body_hash))
+    end
+
+    it 'passes nil to sign_request when body is nil' do
+      capturing_client.transport.perform_request('GET', '_cat/health', {}, nil, {})
+      expect(captured_bodies.last).to be_nil
+    end
+
+    it 'passes a pre-serialized String body through unchanged' do
+      raw = '{"already":"serialized"}'
+      capturing_client.transport.perform_request('POST', '_search', {}, raw, {})
+      expect(captured_bodies.last).to eq(raw)
+    end
+  end
 end