diff --git a/Gemfile.lock b/Gemfile.lock index 7326421..a013f3e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - hooks-ruby (0.0.5) + hooks-ruby (0.0.6) dry-schema (~> 1.14, >= 1.14.1) grape (~> 2.3) puma (~> 6.6) diff --git a/docs/configuration.md b/docs/configuration.md index 0fefa74..3c618f5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -95,6 +95,12 @@ When set to `true`, enables a catch-all route that will handle requests to unkno **Default:** `false` **Example:** `false` +### `normalize_headers` + +When set to `true`, normalizes incoming HTTP headers by lowercasing and trimming them. This ensures consistency in header names and values. + +**Default:** `true` + ## Endpoint Options ### `path` diff --git a/docs/handler_plugins.md b/docs/handler_plugins.md index 09f8003..3532e2f 100644 --- a/docs/handler_plugins.md +++ b/docs/handler_plugins.md @@ -15,8 +15,8 @@ Handler plugins are Ruby classes that extend the `Hooks::Plugins::Handlers::Base class Example < Hooks::Plugins::Handlers::Base # Process a webhook payload # - # @param payload [Hash, String] webhook payload (symbolized keys by default) - # @param headers [Hash] HTTP headers (symbolized keys by default) + # @param payload [Hash, String] webhook payload (pure JSON with string keys) + # @param headers [Hash] HTTP headers (string keys, optionally normalized - default is normalized) # @param config [Hash] Endpoint configuration # @return [Hash] Response data def call(payload:, headers:, config:) @@ -31,9 +31,9 @@ end The `payload` parameter can be a Hash or a String. If the payload is a String, it will be parsed as JSON. If it is a Hash, it will be passed directly to the handler. The payload can contain any data that the webhook sender wants to send. -By default, the payload is parsed as JSON (if it can be) and then symbolized. This means that the keys in the payload will be converted to symbols. You can disable this auto-symbolization of the payload by setting the environment variable `HOOKS_SYMBOLIZE_PAYLOAD` to `false` or by setting the `symbolize_payload` option to `false` in the global configuration file. +The payload is parsed as JSON (if it can be) and returned as a pure Ruby hash with string keys, maintaining the original JSON structure. This ensures that the payload is always a valid JSON representation that can be easily serialized and processed. -**TL;DR**: The payload is almost always a Hash with symbolized keys, regardless of whether the original payload was a Hash or a JSON String. +**TL;DR**: The payload is almost always a Hash with string keys, regardless of whether the original payload was a Hash or a JSON String. For example, if the client sends the following JSON payload: @@ -50,10 +50,10 @@ It will be parsed and passed to the handler as: ```ruby { - hello: "world", - foo: ["bar", "baz"], - truthy: true, - coffee: {is: "good"} + "hello" => "world", + "foo" => ["bar", "baz"], + "truthy" => true, + "coffee" => {"is" => "good"} } ``` @@ -61,13 +61,13 @@ It will be parsed and passed to the handler as: The `headers` parameter is a Hash that contains the HTTP headers that were sent with the webhook request. It includes standard headers like `host`, `user-agent`, `accept`, and any custom headers that the webhook sender may have included. -By default, the headers are normalized (lowercased and trimmed) and then symbolized. This means that the keys in the headers will be converted to symbols, and any hyphens (`-`) in header names are converted to underscores (`_`). You can disable header symbolization by setting the environment variable `HOOKS_SYMBOLIZE_HEADERS` to `false` or by setting the `symbolize_headers` option to `false` in the global configuration file. +By default, the headers are normalized (lowercased and trimmed) but kept as string keys to maintain their JSON representation. Header keys are always strings, and any normalization simply ensures consistent formatting (lowercasing and trimming whitespace). You can disable header normalization by setting the environment variable `HOOKS_NORMALIZE_HEADERS` to `false` or by setting the `normalize_headers` option to `false` in the global configuration file. -**TL;DR**: The headers are almost always a Hash with symbolized keys, with hyphens converted to underscores. +**TL;DR**: The headers are always a Hash with string keys, optionally normalized for consistency. For example, if the client sends the following headers: -``` +```text Host: hooks.example.com User-Agent: foo-client/1.0 Accept: application/json, text/plain, */* @@ -79,25 +79,23 @@ X-Forwarded-Proto: https Authorization: Bearer ``` -They will be normalized and symbolized and passed to the handler as: +They will be normalized and passed to the handler as: ```ruby { - host: "hooks.example.com", - user_agent: "foo-client/1.0", - accept: "application/json, text/plain, */*", - accept_encoding: "gzip, compress, deflate, br", - client_name: "foo", - x_forwarded_for: "", - x_forwarded_host: "hooks.example.com", - x_forwarded_proto: "https", - authorization: "Bearer " # a careful reminder that headers *can* contain sensitive information! + "host" => "hooks.example.com", + "user-agent" => "foo-client/1.0", + "accept" => "application/json, text/plain, */*", + "accept-encoding" => "gzip, compress, deflate, br", + "client-name" => "foo", + "x-forwarded-for" => "", + "x-forwarded-host" => "hooks.example.com", + "x-forwarded-proto" => "https", + "authorization" => "Bearer " # a careful reminder that headers *can* contain sensitive information! } ``` -It should be noted that the `headers` parameter is a Hash with **symbolized keys** (not strings) by default. They are also normalized (lowercased and trimmed) to ensure consistency. - -You can disable header symbolization by either setting the environment variable `HOOKS_SYMBOLIZE_HEADERS` to `false` or by setting the `symbolize_headers` option to `false` in the global configuration file. +It should be noted that the `headers` parameter is a Hash with **string keys** (not symbols). They are optionally normalized (lowercased and trimmed) to ensure consistency. You can disable header normalization by either setting the environment variable `HOOKS_NORMALIZE_HEADERS` to `false` or by setting the `normalize_headers` option to `false` in the global configuration file. diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index 3a30a40..7c144a0 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -102,14 +102,13 @@ def self.create(config:, endpoints:, log:) validate_auth!(raw_body, headers, endpoint_config, config) end - payload = parse_payload(raw_body, headers, symbolize: config[:symbolize_payload]) + payload = parse_payload(raw_body, headers, symbolize: false) handler = load_handler(handler_class_name) - normalized_headers = config[:normalize_headers] ? Hooks::Utils::Normalize.headers(headers) : headers - symbolized_headers = config[:symbolize_headers] ? Hooks::Utils::Normalize.symbolize_headers(normalized_headers) : normalized_headers + processed_headers = config[:normalize_headers] ? Hooks::Utils::Normalize.headers(headers) : headers response = handler.call( payload:, - headers: symbolized_headers, + headers: processed_headers, config: endpoint_config ) diff --git a/lib/hooks/app/helpers.rb b/lib/hooks/app/helpers.rb index bbe1659..c5127e8 100644 --- a/lib/hooks/app/helpers.rb +++ b/lib/hooks/app/helpers.rb @@ -44,9 +44,9 @@ def enforce_request_limits(config) # # @param raw_body [String] The raw request body # @param headers [Hash] The request headers - # @param symbolize [Boolean] Whether to symbolize keys in parsed JSON (default: true) - # @return [Hash, String] Parsed JSON as Hash (optionally symbolized), or raw body if not JSON - def parse_payload(raw_body, headers, symbolize: true) + # @param symbolize [Boolean] Whether to symbolize keys in parsed JSON (default: false) + # @return [Hash, String] Parsed JSON as Hash with string keys, or raw body if not JSON + def parse_payload(raw_body, headers, symbolize: false) # Optimized content type check - check most common header first content_type = headers["Content-Type"] || headers["CONTENT_TYPE"] || headers["content-type"] || headers["HTTP_CONTENT_TYPE"] @@ -55,6 +55,7 @@ def parse_payload(raw_body, headers, symbolize: true) begin # Security: Limit JSON parsing depth and complexity to prevent JSON bombs parsed_payload = safe_json_parse(raw_body) + # Note: symbolize parameter is kept for backward compatibility but defaults to false parsed_payload = parsed_payload.transform_keys(&:to_sym) if symbolize && parsed_payload.is_a?(Hash) return parsed_payload rescue JSON::ParserError, ArgumentError => e diff --git a/lib/hooks/core/config_loader.rb b/lib/hooks/core/config_loader.rb index 4be5042..46145d9 100644 --- a/lib/hooks/core/config_loader.rb +++ b/lib/hooks/core/config_loader.rb @@ -20,9 +20,7 @@ class ConfigLoader production: true, endpoints_dir: "./config/endpoints", use_catchall_route: false, - symbolize_payload: true, - normalize_headers: true, - symbolize_headers: true + normalize_headers: true }.freeze SILENCE_CONFIG_LOADER_MESSAGES = ENV.fetch( @@ -142,9 +140,7 @@ def self.load_env_config "HOOKS_ENVIRONMENT" => :environment, "HOOKS_ENDPOINTS_DIR" => :endpoints_dir, "HOOKS_USE_CATCHALL_ROUTE" => :use_catchall_route, - "HOOKS_SYMBOLIZE_PAYLOAD" => :symbolize_payload, "HOOKS_NORMALIZE_HEADERS" => :normalize_headers, - "HOOKS_SYMBOLIZE_HEADERS" => :symbolize_headers, "HOOKS_SOME_STRING_VAR" => :some_string_var # Added for test } @@ -156,7 +152,7 @@ def self.load_env_config case config_key when :request_limit, :request_timeout env_config[config_key] = value.to_i - when :use_catchall_route, :symbolize_payload, :normalize_headers, :symbolize_headers + when :use_catchall_route, :normalize_headers # Convert string to boolean env_config[config_key] = %w[true 1 yes on].include?(value.downcase) else diff --git a/lib/hooks/core/config_validator.rb b/lib/hooks/core/config_validator.rb index c610dd6..af47972 100644 --- a/lib/hooks/core/config_validator.rb +++ b/lib/hooks/core/config_validator.rb @@ -26,7 +26,6 @@ class ValidationError < StandardError; end optional(:environment).filled(:string, included_in?: %w[development production]) optional(:endpoints_dir).filled(:string) optional(:use_catchall_route).filled(:bool) - optional(:symbolize_payload).filled(:bool) optional(:normalize_headers).filled(:bool) end diff --git a/lib/hooks/plugins/handlers/base.rb b/lib/hooks/plugins/handlers/base.rb index 094e887..e891082 100644 --- a/lib/hooks/plugins/handlers/base.rb +++ b/lib/hooks/plugins/handlers/base.rb @@ -15,7 +15,7 @@ class Base # Process a webhook request # # @param payload [Hash, String] Parsed request body (JSON Hash) or raw string - # @param headers [Hash] HTTP headers (symbolized keys by default) + # @param headers [Hash] HTTP headers (string keys, optionally normalized - default is normalized) # @param config [Hash] Merged endpoint configuration including opts section (symbolized keys) # @return [Hash, String, nil] Response body (will be auto-converted to JSON) # @raise [NotImplementedError] if not implemented by subclass diff --git a/lib/hooks/version.rb b/lib/hooks/version.rb index bc6c228..12aa89f 100644 --- a/lib/hooks/version.rb +++ b/lib/hooks/version.rb @@ -4,5 +4,5 @@ module Hooks # Current version of the Hooks webhook framework # @return [String] The version string following semantic versioning - VERSION = "0.0.5".freeze + VERSION = "0.0.6".freeze end diff --git a/spec/acceptance/plugins/handlers/team1_handler.rb b/spec/acceptance/plugins/handlers/team1_handler.rb index ea36ca1..9973446 100644 --- a/spec/acceptance/plugins/handlers/team1_handler.rb +++ b/spec/acceptance/plugins/handlers/team1_handler.rb @@ -25,7 +25,7 @@ def call(payload:, headers:, config:) # Process the payload based on type if payload.is_a?(Hash) - event_type = payload[:event_type] || "unknown" + event_type = payload["event_type"] || "unknown" case event_type when "deployment" diff --git a/spec/integration/header_symbolization_spec.rb b/spec/integration/header_symbolization_spec.rb index d51376c..e7b9358 100644 --- a/spec/integration/header_symbolization_spec.rb +++ b/spec/integration/header_symbolization_spec.rb @@ -4,10 +4,9 @@ require_relative "../../lib/hooks" -describe "Header Symbolization Integration" do +describe "Header Normalization Integration" do let(:config) do { - symbolize_headers: true, normalize_headers: true } end @@ -21,33 +20,11 @@ } end - context "when symbolize_headers is enabled (default)" do - it "normalizes and symbolizes headers" do - normalized_headers = config[:normalize_headers] ? Hooks::Utils::Normalize.headers(headers) : headers - symbolized_headers = config[:symbolize_headers] ? Hooks::Utils::Normalize.symbolize_headers(normalized_headers) : normalized_headers + context "when normalize_headers is enabled (default)" do + it "normalizes headers but keeps string keys" do + processed_headers = config[:normalize_headers] ? Hooks::Utils::Normalize.headers(headers) : headers - expect(symbolized_headers).to eq({ - content_type: "application/json", - x_github_event: "push", - user_agent: "test-agent", - accept_encoding: "gzip, br" - }) - end - end - - context "when symbolize_headers is disabled" do - let(:config) do - { - symbolize_headers: false, - normalize_headers: true - } - end - - it "normalizes but does not symbolize headers" do - normalized_headers = config[:normalize_headers] ? Hooks::Utils::Normalize.headers(headers) : headers - symbolized_headers = config[:symbolize_headers] ? Hooks::Utils::Normalize.symbolize_headers(normalized_headers) : normalized_headers - - expect(symbolized_headers).to eq({ + expect(processed_headers).to eq({ "content-type" => "application/json", "x-github-event" => "push", "user-agent" => "test-agent", @@ -56,19 +33,17 @@ end end - context "when both symbolize_headers and normalize_headers are disabled" do + context "when normalize_headers is disabled" do let(:config) do { - symbolize_headers: false, normalize_headers: false } end it "passes headers through unchanged" do - normalized_headers = config[:normalize_headers] ? Hooks::Utils::Normalize.headers(headers) : headers - symbolized_headers = config[:symbolize_headers] ? Hooks::Utils::Normalize.symbolize_headers(normalized_headers) : normalized_headers + processed_headers = config[:normalize_headers] ? Hooks::Utils::Normalize.headers(headers) : headers - expect(symbolized_headers).to eq(headers) + expect(processed_headers).to eq(headers) end end end diff --git a/spec/unit/lib/hooks/app/helpers_spec.rb b/spec/unit/lib/hooks/app/helpers_spec.rb index 54564a0..8925c51 100644 --- a/spec/unit/lib/hooks/app/helpers_spec.rb +++ b/spec/unit/lib/hooks/app/helpers_spec.rb @@ -121,7 +121,7 @@ def error!(message, code) result = helper.parse_payload(body, headers) - expect(result).to eq({ key: "value" }) + expect(result).to eq({ "key" => "value" }) end it "parses JSON that looks like JSON without content type" do @@ -130,7 +130,7 @@ def error!(message, code) result = helper.parse_payload(body, headers) - expect(result).to eq({ key: "value" }) + expect(result).to eq({ "key" => "value" }) end it "parses JSON arrays" do @@ -142,15 +142,15 @@ def error!(message, code) expect(result).to eq([{ "key" => "value" }]) end - it "symbolizes keys by default" do + it "does not symbolize keys by default" do headers = { "Content-Type" => "application/json" } body = '{"string_key": "value", "nested": {"inner_key": "inner_value"}}' result = helper.parse_payload(body, headers) expect(result).to eq({ - string_key: "value", - nested: { "inner_key" => "inner_value" } # Only top level is symbolized + "string_key" => "value", + "nested" => { "inner_key" => "inner_value" } }) end @@ -171,7 +171,7 @@ def error!(message, code) result = helper.parse_payload(body, headers) - expect(result).to eq({ key: "value" }) + expect(result).to eq({ "key" => "value" }) end it "handles lowercase content-type" do @@ -180,7 +180,7 @@ def error!(message, code) result = helper.parse_payload(body, headers) - expect(result).to eq({ key: "value" }) + expect(result).to eq({ "key" => "value" }) end it "handles HTTP_CONTENT_TYPE" do @@ -189,7 +189,7 @@ def error!(message, code) result = helper.parse_payload(body, headers) - expect(result).to eq({ key: "value" }) + expect(result).to eq({ "key" => "value" }) end end @@ -212,7 +212,7 @@ def error!(message, code) result = helper.parse_payload(nested_json, headers) - expect(result).to eq({ level1: { "level2" => { "level3" => { "value" => "test" } } } }) + expect(result).to eq({ "level1" => { "level2" => { "level3" => { "value" => "test" } } } }) end it "returns raw body when JSON exceeds size limits" do diff --git a/spec/unit/lib/hooks/core/config_loader_spec.rb b/spec/unit/lib/hooks/core/config_loader_spec.rb index ff015ef..f986125 100644 --- a/spec/unit/lib/hooks/core/config_loader_spec.rb +++ b/spec/unit/lib/hooks/core/config_loader_spec.rb @@ -19,9 +19,7 @@ production: true, endpoints_dir: "./config/endpoints", use_catchall_route: false, - symbolize_payload: true, - normalize_headers: true, - symbolize_headers: true + normalize_headers: true ) end end @@ -188,9 +186,7 @@ it "converts boolean environment variables correctly" do ENV["HOOKS_USE_CATCHALL_ROUTE"] = "true" - ENV["HOOKS_SYMBOLIZE_PAYLOAD"] = "1" ENV["HOOKS_NORMALIZE_HEADERS"] = "yes" - ENV["HOOKS_SYMBOLIZE_HEADERS"] = "on" # Add a non-boolean var to ensure it's not misinterpreted ENV["HOOKS_SOME_STRING_VAR"] = "test_value" @@ -198,9 +194,7 @@ config = described_class.load expect(config[:use_catchall_route]).to be true - expect(config[:symbolize_payload]).to be true expect(config[:normalize_headers]).to be true - expect(config[:symbolize_headers]).to be true expect(config[:some_string_var]).to eq("test_value") # Check the string var end end @@ -373,12 +367,12 @@ handler: "ValidHandler" ) end - it "allows opt-out via environment variable" do - ENV["HOOKS_SYMBOLIZE_HEADERS"] = "false" + it "allows environment variable setup" do + ENV["HOOKS_NORMALIZE_HEADERS"] = "false" config = described_class.load - expect(config[:symbolize_headers]).to be false + expect(config[:normalize_headers]).to be false end end end diff --git a/spec/unit/lib/hooks/core/config_validator_spec.rb b/spec/unit/lib/hooks/core/config_validator_spec.rb index bd53518..7ec7923 100644 --- a/spec/unit/lib/hooks/core/config_validator_spec.rb +++ b/spec/unit/lib/hooks/core/config_validator_spec.rb @@ -15,7 +15,6 @@ environment: "development", endpoints_dir: "./custom/endpoints", use_catchall_route: true, - symbolize_payload: false, normalize_headers: false } @@ -115,14 +114,12 @@ it "coerces boolean-like string values" do config = { use_catchall_route: "true", - symbolize_payload: "false", normalize_headers: "1" } result = described_class.validate_global_config(config) expect(result[:use_catchall_route]).to be true - expect(result[:symbolize_payload]).to be false expect(result[:normalize_headers]).to be true # "1" gets coerced to true end