diff --git a/lib/protocol/grpc/header/timeout.rb b/lib/protocol/grpc/header/timeout.rb index 080de7e..f726071 100644 --- a/lib/protocol/grpc/header/timeout.rb +++ b/lib/protocol/grpc/header/timeout.rb @@ -13,24 +13,29 @@ module Header # This header appears only in request headers, not in trailers. class Timeout < String # The wire format for a gRPC timeout value. - FORMAT = /\A(?[1-9]\d{0,7})(?[HMSmun])\z/ + FORMAT = /\A(?\d{1,8})(?[HMSmun])\z/ # Format a timeout duration for the `grpc-timeout` header. # @parameter timeout [Numeric] The timeout duration in seconds. # @returns [String] The formatted timeout. def self.format(timeout) - if timeout >= 3600 - "#{(timeout / 3600).to_i}H" - elsif timeout >= 60 - "#{(timeout / 60).to_i}M" - elsif timeout >= 1 - "#{timeout.to_i}S" - elsif timeout >= 0.001 - "#{(timeout * 1000).to_i}m" - elsif timeout >= 0.000001 - "#{(timeout * 1_000_000).to_i}u" - else - "#{(timeout * 1_000_000_000).to_i}n" + raise ArgumentError, "Timeout must be finite and non-negative!" unless timeout.finite? && timeout >= 0 + raise RangeError, "Timeout exceeds the grpc-timeout wire limit!" if timeout > 99_999_999 * 3600 + return "0n" if timeout.zero? + + nanoseconds = (timeout * 1_000_000_000).ceil + units = {"H" => 3_600_000_000_000, "M" => 60_000_000_000, "S" => 1_000_000_000, "m" => 1_000_000, "u" => 1000, "n" => 1} + + # Prefer an exact representation in the largest possible unit: + units.each do |unit, scale| + amount, remainder = nanoseconds.divmod(scale) + return "#{amount}#{unit}" if remainder.zero? && amount <= 99_999_999 + end + + # Otherwise round up in the finest unit that fits the wire limit: + units.reverse_each do |unit, scale| + amount = (nanoseconds + scale - 1).div(scale) + return "#{amount}#{unit}" if amount <= 99_999_999 end end @@ -69,7 +74,7 @@ def initialize(value) # @raises [ArgumentError] If the timeout value is invalid. def to_seconds unless match = FORMAT.match(self) - raise ArgumentError, "Invalid grpc-timeout: #{self.inspect}" + raise ArgumentError, "Invalid grpc-timeout: #{self.inspect}!" end amount = match[:amount].to_i diff --git a/lib/protocol/grpc/metadata.rb b/lib/protocol/grpc/metadata.rb index 712cc9c..6f96c4f 100644 --- a/lib/protocol/grpc/metadata.rb +++ b/lib/protocol/grpc/metadata.rb @@ -17,7 +17,7 @@ module Metadata # @parameter timeout [Numeric | Nil] Optional timeout in seconds. # @parameter content_type [String] The request content type. # @returns [Protocol::HTTP::Headers] The constructed request headers. - def self.build(metadata: {}, timeout: nil, content_type: "application/grpc+proto") + def self.build(metadata: {}, timeout: nil, content_type: "application/grpc") headers = Protocol::HTTP::Headers.new(policy: Protocol::GRPC::HEADER_POLICY) headers["content-type"] = content_type headers["te"] = "trailers" @@ -52,9 +52,9 @@ def self.extract(headers) # Decode binary headers: if key.end_with?("-bin") if value.is_a?(String) - value = Base64.strict_decode64(value) + value = decode_binary(value) elsif value.is_a?(Array) - value = value.map{|item| Base64.strict_decode64(item)} + value = value.map{|item| decode_binary(item)} end end @@ -64,6 +64,24 @@ def self.extract(headers) metadata end + # Decode a padded or unpadded binary metadata value. + # @parameter value [String] The base64 encoded value. + # @returns [String] The decoded bytes. + # @raises [ArgumentError] If the value has invalid Base64 characters or padding. + def self.decode_binary(value) + # Only supply omitted padding; validate existing padding unchanged: + unless value.end_with?("=") + case value.bytesize % 4 + when 2 + value += "==" + when 3 + value += "=" + end + end + + Base64.strict_decode64(value) + end + # Extract gRPC status from headers. # Returns Status::UNKNOWN if status is not present. # @@ -107,8 +125,9 @@ def self.extract_message(headers) # @parameter headers [Protocol::HTTP::Headers] # @parameter status [Integer] gRPC status code # @parameter message [String | Nil] Optional status message - # @parameter error [Exception | Nil] Optional error object (used to extract backtrace) - def self.assign_status!(headers, status: Status::OK, message: nil, error: nil) + # @parameter error [Exception | Nil] Optional error object used for the message. + # @parameter backtrace [Boolean] Whether to include the error backtrace for debugging. + def self.assign_status!(headers, status: Status::OK, message: nil, error: nil, backtrace: false) headers["grpc-status"] = status if error && message.nil? @@ -121,7 +140,7 @@ def self.assign_status!(headers, status: Status::OK, message: nil, error: nil) end # Add backtrace from error if available - if error && error.backtrace && !error.backtrace.empty? + if backtrace && error && error.backtrace && !error.backtrace.empty? # Assign backtrace array directly - Split header will handle it headers["backtrace"] = error.backtrace end diff --git a/lib/protocol/grpc/status.rb b/lib/protocol/grpc/status.rb index 500403f..5a0fc7a 100644 --- a/lib/protocol/grpc/status.rb +++ b/lib/protocol/grpc/status.rb @@ -7,6 +7,20 @@ module Protocol module GRPC # Provides gRPC status codes and their names. module Status + # Map an HTTP response status when the server did not provide grpc-status. + # @parameter status [Integer] The HTTP status code. + # @returns [Integer] The fallback gRPC status code. + def self.for_http_status(status) + case status + when 400 then INTERNAL + when 401 then UNAUTHENTICATED + when 403 then PERMISSION_DENIED + when 404 then UNIMPLEMENTED + when 429, 502, 503, 504 then UNAVAILABLE + else UNKNOWN + end + end + OK = 0 CANCELLED = 1 UNKNOWN = 2 diff --git a/releases.md b/releases.md index 4187d50..2c170f0 100644 --- a/releases.md +++ b/releases.md @@ -1,5 +1,12 @@ # Releases +## Unreleased + + - Preserve timeout precision within the eight-digit wire limit, rounding up when necessary. + - Accept padded and unpadded binary metadata. + - **Breaking**: Error backtraces are no longer sent to clients by default. Pass `backtrace: true` to `Metadata.assign_status!` to explicitly enable them for debugging. + - Default request metadata to `application/grpc` and provide `Status.for_http_status` for responses without `grpc-status`. + ## v0.15.0 - **Breaking**: Removed `Protocol::GRPC::Status::DESCRIPTIONS`. Use `Protocol::GRPC::Status::NAMES` for canonical gRPC status names. diff --git a/test/protocol/grpc/header/timeout.rb b/test/protocol/grpc/header/timeout.rb index f23b13c..cdc9e23 100644 --- a/test/protocol/grpc/header/timeout.rb +++ b/test/protocol/grpc/header/timeout.rb @@ -13,6 +13,34 @@ end with ".format" do + it "preserves fractional seconds and partial minutes and hours" do + [1.5, 59.9, 90, 3599, 3601, 7199, 0.0000000001].each do |duration| + encoded = subject.format(duration) + decoded = subject.new(encoded).to_seconds + expect(decoded).to be >= duration + expect(decoded - duration).to be < 0.000001 + end + end + + it "rounds up when the finest units exceed eight digits" do + duration = 123456.789123 + encoded = subject.format(duration) + expect(encoded).to be =~ /\A\d{1,8}[HMSmun]\z/ + expect(subject.new(encoded).to_seconds).to be >= duration + expect(subject.new(encoded).to_seconds - duration).to be < 1 + end + + it "represents zero without a negative or invalid wire value" do + expect(subject.new(subject.format(0)).to_seconds).to be == 0 + end + + it "rejects negative, non-finite, and out-of-range durations" do + [-1, Float::INFINITY, Float::NAN].each do |duration| + expect{subject.format(duration)}.to raise_exception(ArgumentError) + end + expect{subject.format(100_000_000 * 3600)}.to raise_exception(RangeError) + end + it "formats seconds" do expect(subject.format(5)).to be == "5S" end @@ -66,15 +94,13 @@ it "raises an argument error for invalid values" do invalid_values = [ "", - "0S", - "01S", "123456789S", "1s", "oneS", ] invalid_values.each do |value| - expect{subject.new(value).to_seconds}.to raise_exception(ArgumentError, message: be == "Invalid grpc-timeout: #{value.inspect}") + expect{subject.new(value).to_seconds}.to raise_exception(ArgumentError, message: be == "Invalid grpc-timeout: #{value.inspect}!") end end end diff --git a/test/protocol/grpc/http_status.rb b/test/protocol/grpc/http_status.rb new file mode 100644 index 0000000..f98b811 --- /dev/null +++ b/test/protocol/grpc/http_status.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# Released under the MIT License. +# Copyright, 2026, by Samuel Williams. + +require "protocol/grpc/status" + +describe Protocol::GRPC::Status do + with ".for_http_status" do + it "maps HTTP errors without a grpc-status" do + {400 => 13, 401 => 16, 403 => 7, 404 => 12, 429 => 14, 502 => 14, 503 => 14, 504 => 14, 200 => 2, 500 => 2}.each do |http, grpc| + expect(subject.for_http_status(http)).to be == grpc + end + end + end +end diff --git a/test/protocol/grpc/metadata.rb b/test/protocol/grpc/metadata.rb index 06ba94d..ee9f0d4 100644 --- a/test/protocol/grpc/metadata.rb +++ b/test/protocol/grpc/metadata.rb @@ -12,7 +12,7 @@ it "builds basic gRPC headers" do headers = subject.build - expect(headers["content-type"].to_s).to be == "application/grpc+proto" + expect(headers["content-type"].to_s).to be == "application/grpc" expect(headers["te"].to_s).to be == "trailers" end @@ -42,7 +42,45 @@ end end + with ".decode_binary" do + it "decodes correctly padded and unpadded standard Base64" do + { + "" => "", + "Zg==" => "f", + "Zg" => "f", + "Zm8=" => "fo", + "Zm8" => "fo", + "Zm9v" => "foo", + "+w==" => "\xFB".b, + "+w" => "\xFB".b, + "/w==" => "\xFF".b, + "/w" => "\xFF".b, + }.each do |encoded, decoded| + expect(subject.decode_binary(encoded)).to be == decoded + end + end + end + with ".extract" do + it "decodes padded and unpadded repeated binary metadata" do + headers = Protocol::HTTP::Headers.new([ + ["custom-bin", "AQIDBA"], + ["custom-bin", "AQIDBA=="], + ["custom-bin", "aGk"] + ]) + expect(subject.extract(headers)["custom-bin"]).to be == ["\x01\x02\x03\x04".b, "\x01\x02\x03\x04".b, "hi".b] + end + + it "decodes unpadded scalar metadata" do + expect(subject.extract({"custom-bin" => "aGk"})["custom-bin"]).to be == "hi".b + end + + it "rejects malformed base64" do + ["!invalid", "A", "aGk===", "aG k", "Zg=", "Z=g", "-w", "-w==", "_w", "_w==", "Zg==\n", "Zh==", "Zh"].each do |value| + expect{subject.extract({"custom-bin" => value})}.to raise_exception(ArgumentError) + end + end + let(:headers) do Protocol::HTTP::Headers.new([ ["content-type", "application/grpc+proto"], @@ -114,6 +152,18 @@ def headers.to_h end with ".assign_status!" do + it "only includes backtraces when explicitly enabled" do + error = StandardError.new("Failure") + error.set_backtrace(["/private/service.rb:42"]) + headers = subject.build + subject.assign_status!(headers, status: Protocol::GRPC::Status::INTERNAL, error: error) + expect(headers["backtrace"]).to be_nil + expect(subject.extract_message(headers)).to be == "Failure" + + subject.assign_status!(headers, status: Protocol::GRPC::Status::INTERNAL, error: error, backtrace: true) + expect(headers["backtrace"]).to be == error.backtrace + end + it "assigns status to headers" do headers = Protocol::HTTP::Headers.new([], nil, policy: Protocol::GRPC::HEADER_POLICY) subject.assign_status!(headers, status: Protocol::GRPC::Status::OK) diff --git a/test/protocol/grpc/middleware.rb b/test/protocol/grpc/middleware.rb index 6163f43..58d6741 100644 --- a/test/protocol/grpc/middleware.rb +++ b/test/protocol/grpc/middleware.rb @@ -255,7 +255,7 @@ def say_hello(_input, output, _call) expect(message).to be == "Resource not found" end - it "adds backtrace to headers when error has backtrace" do + it "omits backtraces even when the error has one" do error = StandardError.new("Test error") error.set_backtrace([ "/path/to/file.rb:10:in `method'", @@ -268,20 +268,8 @@ def say_hello(_input, output, _call) error: error ) - # Access backtrace directly from headers (Split header returns array) - backtrace = response.headers["backtrace"] - - expect(backtrace).to be_a(Array) - expect(backtrace.length).to be == 2 - expect(backtrace[0]).to be == "/path/to/file.rb:10:in `method'" - expect(backtrace[1]).to be == "/path/to/file.rb:5:in `block'" - - # Also verify it's accessible via metadata extraction (for client-side usage) - metadata = Protocol::GRPC::Metadata.extract(response.headers) - backtrace_from_metadata = metadata["backtrace"] - # Metadata extraction may return string or array depending on how headers.each works - # But the important thing is that it's present and can be parsed - expect(backtrace_from_metadata).not.to be_nil + expect(response.headers["backtrace"]).to be_nil + expect(Protocol::GRPC::Metadata.extract(response.headers)).not.to have_keys("backtrace") end it "does not add backtrace when error has no backtrace" do @@ -370,12 +358,7 @@ def say_hello(_input, _output, _call) status = Protocol::GRPC::Metadata.extract_status(response.headers) expect(status).to be == Protocol::GRPC::Status::INTERNAL - # Verify backtrace is accessible directly from headers - backtrace = response.headers["backtrace"] - expect(backtrace).to be_a(Array) - expect(backtrace.length).to be == 2 - expect(backtrace[0]).to be == "/handler.rb:5:in `say_hello'" - expect(backtrace[1]).to be == "/handler.rb:2:in `call'" + expect(response.headers["backtrace"]).to be_nil end end