diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..d700895 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,32 @@ +name: Ruby checks + +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-22.04 + timeout-minutes: 15 + strategy: + fail-fast: false + matrix: + ruby: ['2.7', '3.0', '3.1', '3.2', '3.3', '3.4', '4.0', jruby] + env: + IPGEO_RUN_LIVE_TESTS: 'false' + IPGEO_RUN_LIVE_HARDENING: 'false' + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 + with: + persist-credentials: false + - uses: ruby/setup-ruby@de492cf36cbab201753d720ce4f4e336cd688c94 + with: + ruby-version: ${{ matrix.ruby }} + bundler-cache: true + - run: bundle exec rspec + - run: bundle exec rake build + - run: bundle exec rake install diff --git a/.gitignore b/.gitignore index f0c58df..c8923ad 100755 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,6 @@ .gems/ *.gem spec/examples.txt +Gemfile.lock +vendor/ +pkg/ diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d6b6055..455962e 100755 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -3,7 +3,7 @@ LANG: "C.UTF-8" before_script: - ruby -v - - bundle config set --local deployment true + - bundle config set --local path vendor/bundle - bundle install -j $(nproc) parallel: matrix: @@ -11,7 +11,7 @@ image: "ruby:$RUBY_VERSION" cache: paths: - - vendor/ruby + - vendor/bundle key: 'ruby-$RUBY_VERSION' gem: diff --git a/README.md b/README.md index bb37990..eaa79a3 100644 --- a/README.md +++ b/README.md @@ -331,7 +331,7 @@ puts IpgeolocationSdk.to_pretty_json(response.data) puts IpgeolocationSdk.to_pretty_json(response.data, :full) ``` -Compact mode omits `nil` fields. Full mode keeps them. +Compact mode omits `nil` fields. Full mode keeps them. When serializing a client configuration, both modes replace its API key with `[REDACTED]`. The original key remains available through `config.api_key` and is used for authentication. ## Errors diff --git a/ipgeolocation_sdk.gemspec b/ipgeolocation_sdk.gemspec index bf372da..71334de 100755 --- a/ipgeolocation_sdk.gemspec +++ b/ipgeolocation_sdk.gemspec @@ -22,6 +22,7 @@ Gem::Specification.new do |s| s.add_development_dependency "rake", "~> 13.0" s.add_development_dependency "rspec", "~> 3.13" + s.add_development_dependency "bigdecimal", ">= 1.4" s.files = [ "CHANGELOG.md", diff --git a/lib/ipgeolocation_sdk/client.rb b/lib/ipgeolocation_sdk/client.rb index 6512034..b46c089 100644 --- a/lib/ipgeolocation_sdk/client.rb +++ b/lib/ipgeolocation_sdk/client.rb @@ -28,6 +28,10 @@ def initialize(config, transport = nil) attr_reader :config + def inspect + "#<#{self.class} config=#{@config.inspect} closed=#{@closed}>" + end + def lookup_ip_geolocation(request = nil) ensure_open typed_request = to_lookup_request(request) diff --git a/lib/ipgeolocation_sdk/client_config.rb b/lib/ipgeolocation_sdk/client_config.rb index 1e3c4a7..a4e5fb3 100644 --- a/lib/ipgeolocation_sdk/client_config.rb +++ b/lib/ipgeolocation_sdk/client_config.rb @@ -26,6 +26,10 @@ def initialize( ) end + def inspect + "#<#{self.class} #{to_h(mode: JsonOutputMode::FULL).inspect}>" + end + private def normalize_api_key(value) diff --git a/lib/ipgeolocation_sdk/serde.rb b/lib/ipgeolocation_sdk/serde.rb index 1971212..4c5694d 100644 --- a/lib/ipgeolocation_sdk/serde.rb +++ b/lib/ipgeolocation_sdk/serde.rb @@ -164,6 +164,7 @@ def to_plain_data(value, mode = JsonOutputMode::COMPACT) item = value.public_send(name) next if normalized_mode == JsonOutputMode::COMPACT && item.nil? + item = "[REDACTED]" if value.is_a?(IpGeolocationClientConfig) && name == :api_key && !item.nil? result[name.to_s] = to_plain_data(item, normalized_mode) end when Array diff --git a/lib/ipgeolocation_sdk/transport.rb b/lib/ipgeolocation_sdk/transport.rb index dd624cd..8e5ee3c 100644 --- a/lib/ipgeolocation_sdk/transport.rb +++ b/lib/ipgeolocation_sdk/transport.rb @@ -15,6 +15,16 @@ class HttpResponseData < ValueObject class NetHttpTransport DEFAULT_MAX_RESPONSE_BODY_CHARS = 128 * 1024 * 1024 + INCOMPLETE_UTF8 = /(?: + [\xC2-\xDF] | + \xE0[\xA0-\xBF]? | + [\xE1-\xEC\xEE-\xEF][\x80-\xBF]? | + \xED[\x80-\x9F]? | + \xF0(?:[\x90-\xBF][\x80-\xBF]?)? | + [\xF1-\xF3][\x80-\xBF]{0,2} | + \xF4(?:[\x80-\x8F][\x80-\xBF]?)? + )\z/nx.freeze + private_constant :INCOMPLETE_UTF8 def initialize(max_response_body_chars: DEFAULT_MAX_RESPONSE_BODY_CHARS) raise ArgumentError, "max_response_body_chars must be a positive integer" unless max_response_body_chars.is_a?(Integer) && max_response_body_chars.positive? @@ -29,9 +39,10 @@ def send_request(request) http.open_timeout = request.connect_timeout http.read_timeout = request.read_timeout - response = http.request(build_net_http_request(uri, request)) - body = response.body - body = body.nil? ? read_response_body(response) : normalize_response_text(body.to_s) + body = nil + response = http.request(build_net_http_request(uri, request)) do |stream| + body = read_response_body(stream) + end HttpResponseData.new( status_code: response.code.to_i, @@ -78,22 +89,26 @@ def build_net_http_request(uri, request) end def read_response_body(response) - body_parts = [] - - response.read_body do |chunk| - body_parts << normalize_response_text(chunk.to_s) + body = String.new(encoding: Encoding::UTF_8) + pending = String.new(encoding: Encoding::BINARY) + size = 0 + append = lambda do |text| + normalized = text.force_encoding(Encoding::UTF_8).scrub + size += normalized.length + if size > @max_response_body_chars + raise TransportError, "Response body exceeded max size of #{@max_response_body_chars} characters" + end + body << normalized end - body_parts.join - end - - def normalize_response_text(text) - normalized = text.encode(Encoding::UTF_8, invalid: :replace, undef: :replace) - if normalized.length > @max_response_body_chars - raise TransportError, "Response body exceeded max size of #{@max_response_body_chars} characters" + response.read_body do |chunk| + bytes = pending << chunk.to_s.b + pending = bytes.slice!(INCOMPLETE_UTF8) || String.new(encoding: Encoding::BINARY) + append.call(bytes) end + append.call(pending) - normalized + body end def normalize_headers(headers) diff --git a/spec/ip_location/client_config_spec.rb b/spec/ip_location/client_config_spec.rb index 23a3956..343d21e 100644 --- a/spec/ip_location/client_config_spec.rb +++ b/spec/ip_location/client_config_spec.rb @@ -28,4 +28,34 @@ described_class.new(connect_timeout: 0) end.to raise_error(IpgeolocationSdk::ValidationError, "connect_timeout must be greater than zero") end + + it "redacts the API key in config and client inspection without changing authentication" do + config = described_class.new(api_key: "private-test-key") + client = IpgeolocationSdk::IpGeolocationClient.new(config) + + expect(config.inspect).to include("[REDACTED]") + expect(config.inspect).not_to include("private-test-key") + expect(client.inspect).not_to include("private-test-key") + expect(config.api_key).to eq("private-test-key") + end + + it "redacts configuration keys in hashes and nested JSON in both output modes" do + config = described_class.new(api_key: "private-test-key", read_timeout: 15) + + [:compact, :full].each do |mode| + expect(config.to_h(mode: mode)).to include("api_key" => "[REDACTED]", "read_timeout" => 15.0) + [IpgeolocationSdk.to_json({ config: [config] }, mode), + IpgeolocationSdk.to_pretty_json(config, mode)].each do |json| + expect(json).to include("[REDACTED]") + expect(json).not_to include("private-test-key") + end + end + end + + it "keeps absent API keys absent in compact output and nil in full output" do + config = described_class.new(request_origin: "https://ipgeolocation.io") + + expect(config.to_h).not_to have_key("api_key") + expect(config.to_h(mode: :full)).to include("api_key" => nil) + end end diff --git a/spec/ip_location/client_spec.rb b/spec/ip_location/client_spec.rb index d323edf..54c42c9 100644 --- a/spec/ip_location/client_spec.rb +++ b/spec/ip_location/client_spec.rb @@ -85,6 +85,17 @@ def close ) end + it "does not inspect request data retained by a custom transport" do + response = IpgeolocationSdk::HttpResponseData.new(status_code: 200, body: "{}", headers: {}) + transport = RecordingTransport.new(response) + client = described_class.new(config, transport) + client.lookup_ip_geolocation + + expect(transport.requests.first.url).to include("apiKey=paid-key") + expect(client.inspect).not_to include("paid-key") + expect(client.inspect).to include("[REDACTED]") + end + it "requires api_key for bulk lookup" do client = described_class.new(api_key: nil, request_origin: "https://ipgeolocation.io", base_url: config.base_url) diff --git a/spec/ip_location/transport_spec.rb b/spec/ip_location/transport_spec.rb index 58feebd..63aa97c 100644 --- a/spec/ip_location/transport_spec.rb +++ b/spec/ip_location/transport_spec.rb @@ -8,13 +8,15 @@ body: "{\"ip\":\"8.8.8.8\"}", to_hash: { "x-test" => ["ok"] } ) + allow(response).to receive(:read_body).and_yield("{\"ip\":\"8.8.8.8\"}".b) + expect(response).not_to receive(:body) http = instance_double("Net::HTTP") allow(Net::HTTP).to receive(:new).with("api.ipgeolocation.io", 443).and_return(http) allow(http).to receive(:use_ssl=).with(true) allow(http).to receive(:open_timeout=).with(5.0) allow(http).to receive(:read_timeout=).with(10.0) - allow(http).to receive(:request).and_return(response) + allow(http).to receive(:request).and_yield(response).and_return(response) request = IpgeolocationSdk::HttpRequestData.new( url: "https://api.ipgeolocation.io/v3/ipgeo?ip=8.8.8.8", diff --git a/spec/ip_location/transport_streaming_spec.rb b/spec/ip_location/transport_streaming_spec.rb new file mode 100644 index 0000000..7bfb94a --- /dev/null +++ b/spec/ip_location/transport_streaming_spec.rb @@ -0,0 +1,116 @@ +require "spec_helper" +require "socket" + +RSpec.describe IpgeolocationSdk::NetHttpTransport do + let(:request) do + IpgeolocationSdk::HttpRequestData.new( + url: "http://127.0.0.1/v3/ipgeo", + method: "GET", headers: {}, body: nil, + connect_timeout: 1.0, read_timeout: 0.1 + ) + end + + def stream_response(chunks, limit: 100) + response = instance_double("Net::HTTPResponse", code: "200", body: chunks.join, to_hash: {}) + allow(response).to receive(:read_body) do |&block| + chunks.each { |chunk| block.call(chunk) } + end + http = instance_double("Net::HTTP") + allow(Net::HTTP).to receive(:new).and_return(http) + allow(http).to receive(:use_ssl=).with(false) + allow(http).to receive(:open_timeout=).with(1.0) + allow(http).to receive(:read_timeout=).with(0.1) + allow(http).to receive(:request) do |_, &block| + block.call(response) if block + response + end + described_class.new(max_response_body_chars: limit).send_request(request).body + end + + it "preserves UTF-8 characters split at every byte boundary" do + text = "東京 café 😀\r\n" + chunks = text.b.bytes.map { |byte| byte.chr(Encoding::BINARY) } + + expect(stream_response(chunks, limit: text.length)).to eq(text) + end + + it "counts accumulated characters rather than bytes or individual chunks" do + expect(stream_response(["東".b, "京".b], limit: 2)).to eq("東京") + expect { stream_response(["東".b, "京".b, "!".b], limit: 2) } + .to raise_error(IpgeolocationSdk::TransportError, "Response body exceeded max size of 2 characters") + end + + it "replaces malformed and incomplete UTF-8 sequences" do + expect(stream_response(["a\xFFb\xE6".b, "\x9D".b])).to eq("a\uFFFDb\uFFFD") + end + + it "preserves valid text adjacent to consecutive malformed sequences" do + bytes = [0x8d, 0xdf, 0x61].pack("C*") + expect(stream_response([bytes])).to eq("\uFFFD\uFFFDa") + expect(stream_response(bytes.bytes.map { |byte| byte.chr(Encoding::BINARY) })).to eq("\uFFFD\uFFFDa") + end + + it "handles invalid UTF-8 prefixes without dropping following text" do + [[0xe0, 0x80], [0xed, 0xa0], [0xf0, 0x80], [0xf4, 0x90]].each do |prefix| + bytes = (prefix + [0x61]).pack("C*") + expected = bytes.dup.force_encoding(Encoding::UTF_8).scrub + expect(stream_response(bytes.bytes.map { |byte| byte.chr(Encoding::BINARY) })).to eq(expected) + end + end + + it "matches whole-body UTF-8 replacement for arbitrary bytes and chunk boundaries" do + random = Random.new(1234) + 100.times do + bytes = Array.new(random.rand(1..100)) { random.rand(256) }.pack("C*") + chunks = [] + offset = 0 + while offset < bytes.bytesize + chunk = bytes.byteslice(offset, random.rand(1..8)) + chunks << chunk + offset += chunk.bytesize + end + expected = bytes.dup.force_encoding(Encoding::UTF_8).scrub + expect(stream_response(chunks, limit: expected.length)).to eq(expected) + end + end + + it "includes the replacement for an incomplete final character in the limit" do + expect { stream_response(["a\xE6\x9D".b], limit: 1) } + .to raise_error(IpgeolocationSdk::TransportError, "Response body exceeded max size of 1 characters") + end + + it "returns an empty UTF-8 string for a response with no body" do + result = stream_response([]) + expect(result).to eq("") + expect(result.encoding).to eq(Encoding::UTF_8) + end + + it "stops an actual HTTP download at the cumulative limit and closes the connection" do + server = TCPServer.new("127.0.0.1", 0) + connection_closed = Queue.new + worker = Thread.new do + socket = server.accept + while (line = socket.gets) && line != "\r\n" + end + socket.write("HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\nConnection: close\r\n\r\n") + socket.write("2\r\nab\r\n2\r\ncd\r\n") + connection_closed << (IO.select([socket], nil, nil, 2) && socket.read(1).nil?) + ensure + socket.close if socket && !socket.closed? + end + local_request = IpgeolocationSdk::HttpRequestData.new( + url: "http://127.0.0.1:#{server.addr[1]}/v3/ipgeo", + method: "GET", headers: {}, body: nil, + connect_timeout: 1.0, read_timeout: 0.1 + ) + + expect { described_class.new(max_response_body_chars: 3).send_request(local_request) } + .to raise_error(IpgeolocationSdk::TransportError, "Response body exceeded max size of 3 characters") + expect(worker.join(3)).not_to be_nil + expect(connection_closed.pop).to be_truthy + ensure + server.close if server && !server.closed? + worker.kill if worker && worker.alive? + worker.join if worker + end +end