From 9049576e8587f7ba80c5e3798f67cd22545e6a52 Mon Sep 17 00:00:00 2001 From: Matthew James Date: Fri, 31 Mar 2023 14:37:45 -0600 Subject: [PATCH] Add Ability to retrieve metadata from the server (#1) * Add Request Metadata * Fix Misc Issues * Start Sending Metadata * Add Method to get metadata * Change Way of Getting Metadata * Change Way of Getting Metadata * Small Refactor * Fixed bugs and failing test cases * Removed unneeded variable in test --------- Co-authored-by: Matt James Co-authored-by: chimano --- Dockerfile | 6 +- Gemfile.lock | 40 ++++----- lib/grpc_web/grpc_web_call.rb | 12 +++ lib/grpc_web/server/grpc_request_decoder.rb | 14 +++ lib/grpc_web/server/grpc_request_processor.rb | 41 +++++---- lib/grpc_web/server/grpc_response_encoder.rb | 14 +++ lib/grpc_web/server/rack_handler.rb | 15 +++- .../integration/ruby_server_js_client_spec.rb | 4 +- .../server/grpc_request_processor_spec.rb | 32 +++++-- .../server/message_serialization_spec.rb | 14 ++- .../unit/grpc_web/server/rack_handler_spec.rb | 90 +++++++++++++++++-- 11 files changed, 221 insertions(+), 61 deletions(-) create mode 100644 lib/grpc_web/grpc_web_call.rb create mode 100644 lib/grpc_web/server/grpc_request_decoder.rb create mode 100644 lib/grpc_web/server/grpc_response_encoder.rb diff --git a/Dockerfile b/Dockerfile index 3a69499..63dae2c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -25,7 +25,9 @@ RUN apt-get update && apt-get install -y \ libxss1 \ libxtst6 \ nodejs \ - xdg-utils + xdg-utils \ + libvulkan1 \ + libu2f-udev # Install Chrome RUN wget --quiet https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb \ @@ -46,7 +48,7 @@ WORKDIR /app COPY .ruby-version grpc-web.gemspec Gemfile Gemfile.lock /app/ COPY lib/grpc_web/version.rb /app/lib/grpc_web/ -RUN gem install bundler \ +RUN gem install bundler -v 2.3.26 \ && bundle config --global frozen 1 \ && bundle install -j4 --retry 3 \ # Remove unneeded files (cached *.gem, *.o, *.c) diff --git a/Gemfile.lock b/Gemfile.lock index ac738f5..76e6d99 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -8,22 +8,20 @@ PATH GEM remote: https://rubygems.org/ specs: - addressable (2.7.0) - public_suffix (>= 2.0.2, < 5.0) - apparition (0.1.0) - backports - capybara (~> 3.12, < 4) + addressable (2.8.1) + public_suffix (>= 2.0.2, < 6.0) + apparition (0.6.0) + capybara (~> 3.13, < 4) websocket-driver (>= 0.6.5) ast (2.4.0) - backports (3.15.0) byebug (11.0.1) - capybara (3.15.1) + capybara (3.35.3) addressable mini_mime (>= 0.1.3) nokogiri (~> 1.8) rack (>= 1.6.0) rack-test (>= 0.6.3) - regexp_parser (~> 1.2) + regexp_parser (>= 1.5, < 3.0) xpath (~> 3.2) coderay (1.1.2) crack (0.4.3) @@ -40,10 +38,11 @@ GEM jaro_winkler (1.5.4) json (2.3.0) method_source (0.9.2) - mini_mime (1.0.2) - mini_portile2 (2.4.0) - nokogiri (1.10.8) - mini_portile2 (~> 2.4.0) + mini_mime (1.1.2) + mini_portile2 (2.6.1) + nokogiri (1.12.5) + mini_portile2 (~> 2.6.1) + racc (~> 1.4) parallel (1.19.1) parser (2.7.0.2) ast (~> 2.4.0) @@ -53,15 +52,16 @@ GEM pry-byebug (3.7.0) byebug (~> 11.0) pry (~> 0.10) - public_suffix (4.0.1) - rack (2.0.9) + public_suffix (4.0.7) + racc (1.6.2) + rack (2.2.6.4) rack-cors (1.1.0) rack (>= 2.0.0) - rack-test (1.1.0) - rack (>= 1.0, < 3) + rack-test (2.1.0) + rack (>= 1.3) rainbow (3.0.0) rake (13.0.1) - regexp_parser (1.6.0) + regexp_parser (2.7.0) rspec (3.9.0) rspec-core (~> 3.9.0) rspec-expectations (~> 3.9.0) @@ -96,9 +96,9 @@ GEM addressable (>= 2.3.6) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) - websocket-driver (0.7.1) + websocket-driver (0.7.5) websocket-extensions (>= 0.1.0) - websocket-extensions (0.1.4) + websocket-extensions (0.1.5) xpath (3.2.0) nokogiri (~> 1.8) @@ -118,4 +118,4 @@ DEPENDENCIES webmock BUNDLED WITH - 2.1.4 + 2.3.26 diff --git a/lib/grpc_web/grpc_web_call.rb b/lib/grpc_web/grpc_web_call.rb new file mode 100644 index 0000000..cf19ca7 --- /dev/null +++ b/lib/grpc_web/grpc_web_call.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module GRPCWeb + GRPCWebCall = Struct.new( + :request, + :metadata, + :metadata_received, + :metadata_sent, + :metadata_to_send, + ) +end + diff --git a/lib/grpc_web/server/grpc_request_decoder.rb b/lib/grpc_web/server/grpc_request_decoder.rb new file mode 100644 index 0000000..ca6cd75 --- /dev/null +++ b/lib/grpc_web/server/grpc_request_decoder.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class GRPCWeb::GRPCRequestDecoder + + def self.decode(grpc_request) + text_coder = ::GRPCWeb::TextCoder + framing = ::GRPCWeb::RequestFraming + serialization = ::GRPCWeb::MessageSerialization + + grpc_web_request = text_coder.decode_request(grpc_request) + grpc_web_request = framing.unframe_request(grpc_web_request) + serialization.deserialize_request(grpc_web_request) + end +end diff --git a/lib/grpc_web/server/grpc_request_processor.rb b/lib/grpc_web/server/grpc_request_processor.rb index 0ff16b2..d555801 100644 --- a/lib/grpc_web/server/grpc_request_processor.rb +++ b/lib/grpc_web/server/grpc_request_processor.rb @@ -6,38 +6,45 @@ require 'grpc_web/server/error_callback' require 'grpc_web/server/message_serialization' require 'grpc_web/server/text_coder' +require 'grpc_web/server/grpc_response_encoder' +require 'grpc_web/server/grpc_request_decoder' # Placeholder module GRPCWeb::GRPCRequestProcessor class << self include ::GRPCWeb::ContentTypes - def process(grpc_web_request) - text_coder = ::GRPCWeb::TextCoder - framing = ::GRPCWeb::RequestFraming - serialization = ::GRPCWeb::MessageSerialization - - grpc_web_request = text_coder.decode_request(grpc_web_request) - grpc_web_request = framing.unframe_request(grpc_web_request) - grpc_web_request = serialization.deserialize_request(grpc_web_request) - grpc_web_response = execute_request(grpc_web_request) - grpc_web_response = serialization.serialize_response(grpc_web_response) - grpc_web_response = framing.frame_response(grpc_web_response) - text_coder.encode_response(grpc_web_response) + def process(grpc_call) + encoder = GRPCWeb::GRPCResponseEncoder + + grpc_web_response = execute_request(grpc_call) + encoder.encode(grpc_web_response) end private - def execute_request(request) - service_method = ::GRPC::GenericService.underscore(request.service_method.to_s) + def execute_request(grpc_call) + decoder = GRPCWeb::GRPCRequestDecoder + service_method = ::GRPC::GenericService.underscore(grpc_call.request.service_method.to_s) begin - response = request.service.send(service_method, request.body) + # Check arity to before passing in metadata to make sure that server can handle the request. + # This is to ensure backwards compatibility + if grpc_call.request.service.method(service_method.to_sym).arity == 1 + response = grpc_call.request.service.send( + service_method, + decoder.decode(grpc_call.request).body, + ) + else + response = grpc_call.request.service.send( + service_method, decoder.decode(grpc_call.request).body, grpc_call, + ) + end rescue StandardError => e - ::GRPCWeb.on_error.call(e, request.service, request.service_method) + ::GRPCWeb.on_error.call(e, grpc_call.request.service, grpc_call.request.service_method) response = e # Return exception as body if one is raised end - ::GRPCWeb::GRPCWebResponse.new(response_content_type(request), response) + ::GRPCWeb::GRPCWebResponse.new(response_content_type(grpc_call.request), response) end # Use Accept header value if specified, otherwise use request content type diff --git a/lib/grpc_web/server/grpc_response_encoder.rb b/lib/grpc_web/server/grpc_response_encoder.rb new file mode 100644 index 0000000..0802b36 --- /dev/null +++ b/lib/grpc_web/server/grpc_response_encoder.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class GRPCWeb::GRPCResponseEncoder + + def self.encode(response) + text_coder = ::GRPCWeb::TextCoder + framing = ::GRPCWeb::RequestFraming + serialization = ::GRPCWeb::MessageSerialization + + grpc_web_response = serialization.serialize_response(response) + grpc_web_response = framing.frame_response(grpc_web_response) + text_coder.encode_response(grpc_web_response) + end +end diff --git a/lib/grpc_web/server/rack_handler.rb b/lib/grpc_web/server/rack_handler.rb index 818019a..e270000 100644 --- a/lib/grpc_web/server/rack_handler.rb +++ b/lib/grpc_web/server/rack_handler.rb @@ -5,8 +5,10 @@ require 'rack/request' require 'grpc_web/content_types' require 'grpc_web/grpc_web_request' +require 'grpc_web/grpc_web_call' require 'grpc_web/server/error_callback' require 'grpc_web/server/grpc_request_processor' +require "base64" # Placeholder module GRPCWeb::RackHandler @@ -25,9 +27,20 @@ def call(service, service_method, env) content_type = rack_request.content_type accept = rack_request.get_header(ACCEPT_HEADER) + metadata = Hash[ + *env.select { |k, _v| k.start_with? 'HTTP_' } + .reject { |k, _v| k.eql? ACCEPT_HEADER } + .collect { |k, v| [k.sub(/^HTTP_/, ''), v] } + .collect { |k, v| [k, k.end_with?('_BIN') ? Base64.decode64(v) : v] } + .collect { |k, v| [k.split('_').collect(&:downcase).join('_'), v] } + .sort + .flatten + ] body = rack_request.body.read + request = GRPCWeb::GRPCWebRequest.new(service, service_method, content_type, accept, body) - response = GRPCWeb::GRPCRequestProcessor.process(request) + call = GRPCWeb::GRPCWebCall.new(request, metadata, started: false) + response = GRPCWeb::GRPCRequestProcessor.process(call) [200, { 'Content-Type' => response.content_type }, [response.body]] rescue Google::Protobuf::ParseError => e diff --git a/spec/integration/ruby_server_js_client_spec.rb b/spec/integration/ruby_server_js_client_spec.rb index 7cc669d..277b6ba 100644 --- a/spec/integration/ruby_server_js_client_spec.rb +++ b/spec/integration/ruby_server_js_client_spec.rb @@ -115,7 +115,7 @@ def say_hello(_request, _metadata = nil) it 'makes a request using application/grpc-web-text content type' do expect(GRPCWeb::GRPCRequestProcessor).to \ receive(:process) \ - .with(have_attributes(content_type: 'application/grpc-web-text')) \ + .with(have_attributes(request: have_attributes(content_type: 'application/grpc-web-text'))) \ .and_call_original perform_request end @@ -129,7 +129,7 @@ def say_hello(_request, _metadata = nil) it 'makes a request using application/grpc-web+proto content type' do expect(GRPCWeb::GRPCRequestProcessor).to \ receive(:process) \ - .with(have_attributes(content_type: 'application/grpc-web+proto')) \ + .with(have_attributes(request: have_attributes(content_type: 'application/grpc-web+proto'))) \ .and_call_original perform_request end diff --git a/spec/unit/grpc_web/server/grpc_request_processor_spec.rb b/spec/unit/grpc_web/server/grpc_request_processor_spec.rb index f5cd685..3daab8f 100644 --- a/spec/unit/grpc_web/server/grpc_request_processor_spec.rb +++ b/spec/unit/grpc_web/server/grpc_request_processor_spec.rb @@ -2,9 +2,18 @@ RSpec.describe ::GRPCWeb::GRPCRequestProcessor do describe '#process' do - subject(:process) { described_class.process(initial_request) } + subject(:process) { described_class.process(initial_call) } - let(:initial_request) { instance_double(::GRPCWeb::GRPCWebRequest) } + let(:initial_call) { instance_double(::GRPCWeb::GRPCWebCall, request: initial_request) } + let(:initial_request) do + instance_double( + ::GRPCWeb::GRPCWebRequest, + service_method: service_method, + service: service, + accept: request_accept, + content_type: request_content_type, + ) + end let(:text_coder) { ::GRPCWeb::TextCoder } let(:framing) { ::GRPCWeb::RequestFraming } @@ -57,9 +66,21 @@ process end - it 'executes the request' do - expect(service).to receive(:say_hello).with(deserialized_request.body) - process + context 'when handler only accepts one parameter' do + before { allow(service).to receive(:method).and_return(instance_double(Method, arity: 1)) } + + it 'executes the request' do + expect(service).to receive(:say_hello).with(deserialized_request.body) + process + end + end + + context 'when handler accepts more than one parameter' do + + it 'executes the request with additional metadata' do + expect(service).to receive(:say_hello).with(deserialized_request.body, initial_call) + process + end end describe 'execution response' do @@ -116,6 +137,7 @@ context 'when the service raises an error' do let(:error) { StandardError.new('something went wrong') } + let(:service) { instance_double(TestHelloService) } before { allow(service).to receive(:say_hello).and_raise(error) } diff --git a/spec/unit/grpc_web/server/message_serialization_spec.rb b/spec/unit/grpc_web/server/message_serialization_spec.rb index b85588c..de61068 100644 --- a/spec/unit/grpc_web/server/message_serialization_spec.rb +++ b/spec/unit/grpc_web/server/message_serialization_spec.rb @@ -79,7 +79,7 @@ end let(:body) { HelloRequest.new(name: 'Noa') } - shared_examples_for 'generates a body with a payload frame' do |expected_payload_frame_body:| + shared_examples_for 'generates a body with a payload frame' do |expected_payload_frame_body| it 'generates a body with a payload frame' do expect(serialized_response.body).to be_a(Array) expect(serialized_response.body.find(&:payload?).body).to eq expected_payload_frame_body @@ -93,7 +93,7 @@ end end - shared_examples_for 'generates a body with a header frame' do |expected_header_frame_body:| + shared_examples_for 'generates a body with a header frame' do |expected_header_frame_body| it 'generates a body with a header frame' do expect(serialized_response.body).to be_a(Array) expect(serialized_response.body.find(&:header?).body).to eq expected_header_frame_body @@ -108,7 +108,6 @@ it_behaves_like( 'generates a body with a header frame', - expected_header_frame_body: "grpc-status:2\r\ngrpc-message:StandardError: I've made a huge mistake\r\n"\ "x-grpc-web:1\r\n", ) @@ -121,7 +120,6 @@ it_behaves_like( 'generates a body with a header frame', - expected_header_frame_body: "grpc-status:5\r\ngrpc-message:Where am I?\r\nx-grpc-web:1\r\nuser-role-id:123\r\n", ) end @@ -132,11 +130,11 @@ it_behaves_like( 'generates a body with a payload frame', - expected_payload_frame_body: '{"name":"Noa"}', + '{"name":"Noa"}', ) it_behaves_like( 'generates a body with a header frame', - expected_header_frame_body: "grpc-status:0\r\ngrpc-message:OK\r\nx-grpc-web:1\r\n", + "grpc-status:0\r\ngrpc-message:OK\r\nx-grpc-web:1\r\n", ) it_behaves_like 'serializes an exception' @@ -147,11 +145,11 @@ it_behaves_like( 'generates a body with a payload frame', - expected_payload_frame_body: "\n\x03Noa", + "\n\x03Noa", ) it_behaves_like( 'generates a body with a header frame', - expected_header_frame_body: "grpc-status:0\r\ngrpc-message:OK\r\nx-grpc-web:1\r\n", + "grpc-status:0\r\ngrpc-message:OK\r\nx-grpc-web:1\r\n", ) it_behaves_like 'serializes an exception' diff --git a/spec/unit/grpc_web/server/rack_handler_spec.rb b/spec/unit/grpc_web/server/rack_handler_spec.rb index 65a95ce..07fc5ae 100644 --- a/spec/unit/grpc_web/server/rack_handler_spec.rb +++ b/spec/unit/grpc_web/server/rack_handler_spec.rb @@ -2,6 +2,7 @@ require 'grpc_web/content_types' require 'grpc_web/server/rack_handler' +require 'grpc_web/grpc_web_call' RSpec.describe(::GRPCWeb::RackHandler) do subject(:call) { described_class.call(service, service_method, env) } @@ -14,6 +15,7 @@ let(:request_method) { 'POST' } let(:content_type) { ::GRPCWeb::ContentTypes::DEFAULT_CONTENT_TYPE } let(:accept_header) { '*/*' } + let(:metadata) { {} } let(:request_body) { 'request body' } let(:env) do { @@ -45,15 +47,91 @@ it 'calls the request processor with the modified request' do expect(::GRPCWeb::GRPCRequestProcessor).to receive(:process) - .with(::GRPCWeb::GRPCWebRequest.new( - service, - service_method, - content_type, - accept_header, - request_body, + .with(::GRPCWeb::GRPCWebCall.new( + ::GRPCWeb::GRPCWebRequest.new( + service, + service_method, + content_type, + accept_header, + request_body, + ), + metadata, + started: false, )) call end + + context 'when it contains metadata' do + let(:env) do + { + 'PATH_INFO' => path_info, + 'REQUEST_METHOD' => request_method, + 'CONTENT_TYPE' => content_type, + 'HTTP_ACCEPT' => accept_header, + 'rack.input' => StringIO.new(request_body), + 'HTTP_USER_NAME' => 'test_user', + 'HTTP_PASSWORD' => 'test', + } + end + let(:metadata) do + { + 'password' => 'test', + 'user_name' => 'test_user', + } + end + + it 'passes the metadata to the service handler' do + expect(::GRPCWeb::GRPCRequestProcessor).to receive(:process) + .with(::GRPCWeb::GRPCWebCall.new( + ::GRPCWeb::GRPCWebRequest.new( + service, + service_method, + content_type, + accept_header, + request_body, + ), + metadata, + started: false, + )) + call + end + + context 'when the metadata is in binary' do + let(:env) do + { + 'PATH_INFO' => path_info, + 'REQUEST_METHOD' => request_method, + 'CONTENT_TYPE' => content_type, + 'HTTP_ACCEPT' => accept_header, + 'rack.input' => StringIO.new(request_body), + 'HTTP_DATA_BIN' => Base64.encode64('\x01\x02\x03'.b), + 'HTTP_MY_CERT_BIN' => Base64.encode64('\xD1\xD2'.b), + } + end + let(:metadata) do + { + 'data_bin' => '\x01\x02\x03'.b, + 'my_cert_bin' => '\xD1\xD2'.b, + } + end + + it 'passes the metadata to the service handler decoded' do + expect(::GRPCWeb::GRPCRequestProcessor).to receive(:process) + .with(::GRPCWeb::GRPCWebCall.new( + ::GRPCWeb::GRPCWebRequest.new( + service, + service_method, + content_type, + accept_header, + request_body, + ), + metadata, + started: false, + )) + call + end + end + end end context 'with an invalid request' do