diff --git a/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/Utils/rpc_client.rb b/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/Utils/rpc_client.rb index be755d5a0..963ed336f 100644 --- a/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/Utils/rpc_client.rb +++ b/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/Utils/rpc_client.rb @@ -32,16 +32,26 @@ def initialize(api_url, auth_secret) # rubocop:disable Metrics/ParameterLists def call_rpc(endpoint, caller: nil, method: :get, payload: nil, symbolize_keys: false, if_none_match: nil) + resp = call_rpc_raw(endpoint, caller: caller, method: method, payload: payload, + symbolize_keys: symbolize_keys, if_none_match: if_none_match) + return resp if resp == NotModified + + resp.body + end + + def call_rpc_raw(endpoint, caller: nil, method: :get, payload: nil, symbolize_keys: false, if_none_match: nil) response = make_request(endpoint, caller: caller, method: method, payload: payload, symbolize_keys: symbolize_keys, if_none_match: if_none_match) - handle_response(response) - end + return NotModified if response.status == HTTP_NOT_MODIFIED + + raise_appropriate_error(response) unless response.success? + response + end # rubocop:enable Metrics/ParameterLists def fetch_schema(endpoint, if_none_match: nil) - response = make_request(endpoint, method: :get, symbolize_keys: true, if_none_match: if_none_match) - handle_response(response) + call_rpc(endpoint, method: :get, symbolize_keys: true, if_none_match: if_none_match) end private @@ -111,13 +121,6 @@ def generate_signature(timestamp) OpenSSL::HMAC.hexdigest('SHA256', @auth_secret, timestamp) end - def handle_response(response) - return response.body if response.success? - return NotModified if response.status == HTTP_NOT_MODIFIED - - raise_appropriate_error(response) - end - def raise_appropriate_error(response) error_body = parse_error_body(response) status = response.status diff --git a/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb b/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb index 46a17c196..acf0bbb86 100644 --- a/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb +++ b/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb @@ -1,4 +1,7 @@ require 'base64' +require 'cgi' +require 'json' +require 'stringio' module ForestAdminDatasourceRpc class Collection < ForestAdminDatasourceToolkit::Collection @@ -127,7 +130,12 @@ def execute(caller, name, data, filter = nil) "Forwarding '#{@name}' action #{name} call to the Rpc agent on #{url}." ) - @client.call_rpc(url, caller: caller, method: :post, payload: params, symbolize_keys: true) + response = @client.call_rpc_raw(url, caller: caller, method: :post, payload: params, + symbolize_keys: true) + + return build_file_result(response) if response.headers['x-forest-action-type'] == 'File' + + response.body end def get_form(caller, name, data = nil, filter = nil, metas = nil) @@ -176,5 +184,20 @@ def encode_form_data(data) end end end + + def build_file_result(response) + response_headers_header = response.headers['x-forest-action-response-headers'] + file_name_header = response.headers['x-forest-action-file-name'] + + result = { + type: 'File', + mime_type: response.headers['content-type'], + name: CGI.unescape(file_name_header.to_s), + stream: response.body.to_s + } + result[:response_headers] = JSON.parse(response_headers_header) if response_headers_header + + result + end end end diff --git a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb index 0d307ec67..fff27c825 100644 --- a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb +++ b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb @@ -7,17 +7,20 @@ module ForestAdminDatasourceRpc include ForestAdminDatasourceToolkit::Components::Query::ConditionTree describe Collection do + let(:raw_response) do + instance_double(Faraday::Response, body: {}, headers: {}, status: 200, success?: true) + end + let(:rpc_client) { instance_double(Utils::RpcClient, call_rpc: {}, call_rpc_raw: raw_response) } + let(:datasource) { Datasource.new({ uri: 'http://localhost' }, introspection) } + let(:collection) { datasource.get_collection('Product') } + let(:caller) { build_caller } + before do logger = instance_double(Logger, log: nil) allow(ForestAdminAgent::Facades::Container).to receive_messages(logger: logger, cache: 'secret') allow(Utils::RpcClient).to receive(:new).and_return(rpc_client) end - let(:rpc_client) { instance_double(Utils::RpcClient, call_rpc: {}) } - let(:datasource) { Datasource.new({ uri: 'http://localhost' }, introspection) } - let(:collection) { datasource.get_collection('Product') } - let(:caller) { build_caller } - include_context 'with introspection' context 'when initialized' do @@ -251,7 +254,7 @@ module ForestAdminDatasourceRpc collection.execute(caller, 'my_action', data) - expect(rpc_client).to have_received(:call_rpc) do |url, options| + expect(rpc_client).to have_received(:call_rpc_raw) do |url, options| expect(url).to eq('/forest/rpc/Product/action-execute') expect(options[:caller]).to eq(caller) expect(options[:method]).to eq(:post) @@ -270,15 +273,15 @@ module ForestAdminDatasourceRpc end end - it 'asks the RPC client to symbolize response keys so ActionResult.parse sees :type' do + it 'calls call_rpc_raw with symbolized keys so the response headers stay reachable' do collection.execute(caller, 'my_action', {}) - expect(rpc_client).to have_received(:call_rpc) do |_url, options| + expect(rpc_client).to have_received(:call_rpc_raw) do |_url, options| expect(options[:symbolize_keys]).to be(true) end end - it 'returns the action result as-is so :type and other keys reach ActionResult.parse' do + it 'returns the parsed body so :type and other keys reach ActionResult.parse' do success_result = { type: 'Success', message: 'ok', @@ -286,13 +289,65 @@ module ForestAdminDatasourceRpc html: nil, response_headers: {} } - allow(rpc_client).to receive(:call_rpc).and_return(success_result) + allow(raw_response).to receive(:body).and_return(success_result) result = collection.execute(caller, 'my_action', {}) expect(result).to eq(success_result) expect(result[:type]).to eq('Success') end + + context 'when the server replies with X-Forest-Action-Type=File' do + let(:file_body) { 'binary-payload' } + let(:raw_response) do + instance_double( + Faraday::Response, + body: file_body, + headers: { + 'content-type' => 'application/pdf', + 'x-forest-action-type' => 'File', + 'x-forest-action-file-name' => CGI.escape('report final.pdf'), + 'x-forest-action-response-headers' => { 'set-cookie' => 'token=xyz' }.to_json + }, + status: 200, + success?: true + ) + end + + it 'rebuilds a File action result from the response headers and body' do + result = collection.execute(caller, 'download', {}) + + expect(result[:type]).to eq('File') + expect(result[:mime_type]).to eq('application/pdf') + expect(result[:name]).to eq('report final.pdf') + expect(result[:response_headers]).to eq({ 'set-cookie' => 'token=xyz' }) + expect(result[:stream]).to eq(file_body) + end + + context 'when response_headers header is absent' do + let(:raw_response) do + instance_double( + Faraday::Response, + body: 'hi', + headers: { + 'content-type' => 'text/plain', + 'x-forest-action-type' => 'File', + 'x-forest-action-file-name' => 'note.txt' + }, + status: 200, + success?: true + ) + end + + it 'omits response_headers from the rebuilt result' do + result = collection.execute(caller, 'download', {}) + + expect(result[:type]).to eq('File') + expect(result[:name]).to eq('note.txt') + expect(result).not_to have_key(:response_headers) + end + end + end end context 'when call get_form' do diff --git a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb index 5ff24a78d..1cf32e4b6 100644 --- a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb +++ b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb @@ -99,6 +99,28 @@ module Utils end end + describe '#call_rpc_raw' do + let(:response_headers) { { 'x-forest-action-type' => 'File' } } + let(:response) do + instance_double(Faraday::Response, status: 200, body: {}, success?: true, headers: response_headers) + end + let(:faraday_connection) { instance_double(Faraday::Connection, send: response) } + let(:timestamp) { '2025-04-01T14:07:02Z' } + + before do + allow(Faraday::Connection).to receive(:new).and_return(faraday_connection) + allow(Time).to receive(:now).and_return(instance_double(Time, utc: instance_double(Time, iso8601: timestamp))) + end + + it 'returns the Faraday::Response object so callers can read headers' do + result = rpc_client.call_rpc_raw('/rpc/test', method: :post) + + expect(result).to be(response) + expect(result.body).to eq({}) + expect(result.headers).to eq(response_headers) + end + end + describe '#fetch_schema' do let(:response_headers) { {} } let(:response) { instance_double(Faraday::Response, status: 200, body: { collections: [] }, success?: true, headers: response_headers) } diff --git a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/action_execute.rb b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/action_execute.rb index 30d9235e1..6c4fd7b7f 100644 --- a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/action_execute.rb +++ b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/action_execute.rb @@ -1,3 +1,5 @@ +require 'cgi' +require 'json' require 'jsonapi-serializers' module ForestAdminRpcAgent @@ -18,7 +20,31 @@ def handle_request(args) data = args[:params]['data'] action = args[:params]['action'] - collection.execute(args[:caller], action, data, filter) + result = collection.execute(args[:caller], action, data, filter) + + return build_file_response(result) if file_result?(result) + + result + end + + private + + def file_result?(result) + result.is_a?(Hash) && result[:type] == 'File' + end + + def build_file_response(result) + encoded_name = CGI.escape(result[:name].to_s) + headers = { + 'Content-Type' => result[:mime_type], + 'Content-Disposition' => %(attachment; filename="#{encoded_name}"), + 'X-Forest-Action-Type' => 'File', + 'X-Forest-Action-File-Name' => encoded_name + } + headers['X-Forest-Action-Response-Headers'] = result[:response_headers].to_json if result[:response_headers] + + # raw: true skips JSON encoding — file streams are arbitrary binary bytes. + { status: 200, headers: headers, content: result[:stream], raw: true } end end end diff --git a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/base_route.rb b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/base_route.rb index c0ab2c20a..01da82071 100644 --- a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/base_route.rb +++ b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/base_route.rb @@ -26,7 +26,13 @@ def register_sinatra(app) status result[:status] # Set custom headers if provided result[:headers]&.each { |key, value| headers[key] = value } - result[:content] ? serialize_response(result[:content]) : '' + if result[:content].nil? + '' + elsif result[:raw] + result[:content].to_s + else + serialize_response(result[:content]) + end else serialize_response(result) end @@ -68,7 +74,13 @@ def build_rails_response(result) if result.is_a?(Hash) && result.key?(:status) response_headers = { 'Content-Type' => 'application/json' } response_headers.merge!(result[:headers]) if result[:headers] - body = result[:content] ? serialize_response(result[:content]) : '' + body = if result[:content].nil? + '' + elsif result[:raw] + result[:content].to_s + else + serialize_response(result[:content]) + end [result[:status], response_headers, [body]] else [200, { 'Content-Type' => 'application/json' }, [serialize_response(result)]] diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/action_execute_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/action_execute_spec.rb index 16d87e941..4f0305540 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/action_execute_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/action_execute_spec.rb @@ -94,6 +94,54 @@ module Routes .to raise_error(ForestAdminAgent::Http::Exceptions::NotFoundError) end end + + context 'when the action returns a File result' do + let(:binary_payload) { String.new("%PDF-1.4\n%\xE2\xE3\xCF\xD3\n", encoding: 'ASCII-8BIT') } + let(:file_result) do + { + type: 'File', + name: 'report final.pdf', + mime_type: 'application/pdf', + stream: binary_payload, + response_headers: { 'set-cookie' => 'token=xyz' } + } + end + + before do + allow(@datasource.get_collection('users')) + .to receive(:execute).and_return(file_result) + end + + it 'returns a raw HTTP response so the binary content is not JSON-encoded' do + response = route.handle_request(args) + + expect(response).to include(status: 200, raw: true) + expect(response[:content]).to eq(binary_payload) + expect(response[:content].encoding.name).to eq('ASCII-8BIT') + end + + it 'sets File-specific headers including the encoded filename' do + response = route.handle_request(args) + headers = response[:headers] + + expect(headers['Content-Type']).to eq('application/pdf') + expect(headers['X-Forest-Action-Type']).to eq('File') + expect(headers['X-Forest-Action-File-Name']).to eq(CGI.escape('report final.pdf')) + expect(headers['Content-Disposition']).to include(CGI.escape('report final.pdf')) + expect(headers['X-Forest-Action-Response-Headers']).to eq({ 'set-cookie' => 'token=xyz' }.to_json) + end + + context 'when the action does not provide response_headers' do + let(:file_result) do + { type: 'File', name: 'note.txt', mime_type: 'text/plain', stream: 'hi' } + end + + it 'omits the X-Forest-Action-Response-Headers header' do + response = route.handle_request(args) + expect(response[:headers]).not_to have_key('X-Forest-Action-Response-Headers') + end + end + end end end end diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/base_route_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/base_route_spec.rb index 01a2f4cc4..c4e8bca3d 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/base_route_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/base_route_spec.rb @@ -19,6 +19,20 @@ def handle_request(_params) end end + # Test route that returns a binary payload — emulates an action File result. + # The "%\xE2\xE3\xCF\xD3" prefix forces non-UTF8 bytes (the PDF binary marker), + # which is exactly what used to crash serialize_response. + class BinaryTestRoute < BaseRoute + def handle_request(_args) + { + status: 200, + headers: { 'Content-Type' => 'application/pdf' }, + content: String.new("%PDF-1.4\n%\xE2\xE3\xCF\xD3\nbinary-body-bytes", encoding: 'ASCII-8BIT'), + raw: true + } + end + end + describe BaseRoute do subject(:route) { TestRoute.new('/test', 'get', 'test_route') } @@ -136,6 +150,43 @@ def handle_request(_params) ) end end + + # Integration: exercises the handler proc that register_rails passes + # to router.match. This is the only place where binary action results + # actually hit Rack — unit tests that stub collection.execute return a + # Ruby Hash that never goes through serialize_response. + context 'when the handler returns a raw binary response (File action)' do + subject(:binary_route) { BinaryTestRoute.new('/binary', 'post', 'binary_test') } + let(:expected_bytes) do + String.new("%PDF-1.4\n%\xE2\xE3\xCF\xD3\nbinary-body-bytes", encoding: 'ASCII-8BIT') + end + let(:request) do + instance_double( + ActionDispatch::Request, + path_parameters: {}, query_parameters: {}, request_parameters: {}, env: {} + ) + end + + it 'passes the binary body to Rack untouched (no JSON encoding)' do + captured_handler = nil + allow(rails_router).to receive(:match) { |_, opts| captured_handler = opts[:to] } + binary_route.send(:register_rails, rails_router) + + env = { + 'REQUEST_METHOD' => 'POST', + 'PATH_INFO' => '/binary', + 'QUERY_STRING' => '', + 'rack.input' => StringIO.new + } + status, response_headers, body = captured_handler.call(env) + + expect(status).to eq(200) + expect(response_headers['Content-Type']).to eq('application/pdf') + expect(body).to be_an(Array) + expect(body.first).to eq(expected_bytes) + expect(body.first.encoding.name).to eq('ASCII-8BIT') + end + end end describe '#build_rails_response' do @@ -166,6 +217,23 @@ def handle_request(_params) expect(response[1]).to eq({ 'Content-Type' => 'application/json' }) expect(response[2]).to eq(['']) end + + it 'passes raw binary content through without JSON-encoding it (e.g. File action result)' do + binary = String.new("%PDF-1.4\n%\xE2\xE3\xCF\xD3\n", encoding: 'ASCII-8BIT') + result = { + status: 200, + headers: { 'Content-Type' => 'application/pdf' }, + content: binary, + raw: true + } + + response = route.send(:build_rails_response, result) + + expect(response[0]).to eq(200) + expect(response[1]['Content-Type']).to eq('application/pdf') + expect(response[2]).to eq([binary]) + expect(response[2].first.encoding.name).to eq('ASCII-8BIT') + end end context 'when result is not a Hash with :status key' do