Merge pull request #23 from Shopify/validate-request-id-formatting

ensure request ids contain alphanumerics,dashes,underscores

Author: topherbullock

README.md

@@ -82,6 +82,54 @@ method returned `nil`, or all method calls of a batch request returned `nil`. It
 is up to the integration to apply the appropriate transport-layer semantics
 (e.g. returning a 204 No Content).
 
+### ID Validation
+
+By default, string request IDs are validated to contain only alphanumeric
+characters, dashes, and underscores.
+
+**Note:** The JSON-RPC 2.0 specification does not specify a default ID validation pattern, but this default validation
+is recommended to protect against XSS vulnerabilities when IDs are reflected in responses.
+
+```rb
+# Default behavior - accepts alphanumerics, dashes, underscores
+request = { jsonrpc: '2.0', id: 'request-123_abc', method: 'add', params: {a: 1, b: 2} }
+JsonRpcHandler.handle(request) { |method_name| ... }
+# => {"jsonrpc":"2.0","id":"request-123_abc","result":3}
+
+# Rejects potentially dangerous characters
+request = { jsonrpc: '2.0', id: '<script>alert("xss")</script>', method: 'add', params: {a: 1, b: 2} }
+JsonRpcHandler.handle(request) { |method_name| ... }
+# => {"jsonrpc":"2.0","id":null,"error":{"code":-32600,"message":"Invalid Request","data":"Request ID must match validation pattern, or be an integer or null"}}
+```
+
+You can customize the validation pattern by passing the `id_validation_pattern`
+parameter:
+
+```rb
+# Allow email-like IDs with a custom pattern
+custom_pattern = /\A[a-zA-Z0-9_.\-@]+\z/
+request = { jsonrpc: '2.0', id: 'user@example.com', method: 'add', params: {a: 1, b: 2} }
+
+JsonRpcHandler.handle(request, id_validation_pattern: custom_pattern) do |method_name|
+  # ...
+end
+
+# Also works with handle_json
+JsonRpcHandler.handle_json(request_json, id_validation_pattern: custom_pattern) do |method_name|
+  # ...
+end
+```
+
+To disable ID validation entirely (not recommended), pass
+`nil` as the pattern:
+
+```rb
+# Accepts any string
+JsonRpcHandler.handle(request, id_validation_pattern: nil) do |method_name|
+  # ...
+end
+```
+
 ## Development
 
 After checking out the repo:

lib/json_rpc_handler.rb

@@ -17,18 +17,21 @@ class ErrorCode
     ParseError = -32700
   end
 
+  DEFAULT_ALLOWED_ID_CHARACTERS = /\A[a-zA-Z0-9_-]+\z/.freeze
+
   module_function
 
-  def handle(request, &method_finder)
+  def handle(request, id_validation_pattern: DEFAULT_ALLOWED_ID_CHARACTERS, &method_finder)
+
     if request.is_a? Array
-      return error_response id: :unknown_id, error: {
+      return error_response id: :unknown_id, id_validation_pattern:, error: {
         code: ErrorCode::InvalidRequest,
         message: 'Invalid Request',
         data: 'Request is an empty array',
       } if request.empty?
 
       # Handle batch requests
-      responses = request.map { |req| process_request req, &method_finder }.compact
+      responses = request.map { |req| process_request req, id_validation_pattern:, &method_finder }.compact
 
       # A single item is hoisted out of the array
       return responses.first if responses.one?
@@ -37,22 +40,22 @@ def handle(request, &method_finder)
       responses if responses.any?
     elsif request.is_a? Hash
       # Handle single request
-      process_request request, &method_finder
+      process_request request, id_validation_pattern:, &method_finder
     else
-      error_response id: :unknown_id, error: {
+      error_response id: :unknown_id, id_validation_pattern:, error: {
         code: ErrorCode::InvalidRequest,
         message: 'Invalid Request',
         data: 'Request must be an array or a hash',
       }
     end
   end
 
-  def handle_json(request_json, &method_finder)
+  def handle_json(request_json, id_validation_pattern: DEFAULT_ALLOWED_ID_CHARACTERS, &method_finder)
     begin
       request = JSON.parse request_json, symbolize_names: true
-      response = handle request, &method_finder
+      response = handle request, id_validation_pattern:, &method_finder
     rescue JSON::ParserError
-      response =error_response id: :unknown_id, error: {
+      response = error_response id: :unknown_id, id_validation_pattern:, error: {
         code: ErrorCode::ParseError,
         message: 'Parse error',
         data: 'Invalid JSON',
@@ -62,16 +65,16 @@ def handle_json(request_json, &method_finder)
     response.to_json if response
   end
 
-  def process_request(request, &method_finder)
+  def process_request(request, id_validation_pattern:, &method_finder)
     id = request[:id]
 
     error = case
       when !valid_version?(request[:jsonrpc]) then 'JSON-RPC version must be 2.0'
-      when !valid_id?(request[:id]) then 'Request ID must be a string or an integer or null'
+      when !valid_id?(request[:id], id_validation_pattern) then 'Request ID must match validation pattern, or be an integer or null'
       when !valid_method_name?(request[:method]) then 'Method name must be a string and not start with "rpc."'
     end
 
-    return error_response id: :unknown_id, error: {
+    return error_response id: :unknown_id, id_validation_pattern:, error: {
       code: ErrorCode::InvalidRequest,
       message: 'Invalid Request',
       data: error,
@@ -81,7 +84,7 @@ def process_request(request, &method_finder)
     params = request[:params]
 
     unless valid_params? params
-      return error_response id:, error: {
+      return error_response id:, id_validation_pattern:, error: {
         code: ErrorCode::InvalidParams,
         message: 'Invalid params',
         data: 'Method parameters must be an array or an object or null',
@@ -92,7 +95,7 @@ def process_request(request, &method_finder)
       method = method_finder.call method_name
 
       if method.nil?
-        return error_response id:, error: {
+        return error_response id:, id_validation_pattern:, error: {
           code: ErrorCode::MethodNotFound,
           message: 'Method not found',
           data: method_name,
@@ -103,7 +106,7 @@ def process_request(request, &method_finder)
 
       success_response id:, result:
     rescue StandardError => e
-      error_response id:, error: {
+      error_response id:, id_validation_pattern:, error: {
         code: ErrorCode::InternalError,
         message: 'Internal error',
         data: e.message,
@@ -115,8 +118,12 @@ def valid_version?(version)
     version == Version::V2_0
   end
 
-  def valid_id?(id)
-    id.is_a?(String) || id.is_a?(Integer) || id.nil?
+
+  def valid_id?(id, pattern = nil)
+    return true if id.nil? || id.is_a?(Integer)
+    return false unless id.is_a?(String)
+
+    pattern ? id.match?(pattern) : true
   end
 
   def valid_method_name?(method)
@@ -135,10 +142,10 @@ def success_response(id:, result:)
     } unless id.nil?
   end
 
-  def error_response(id:, error:)
+  def error_response(id:, id_validation_pattern:, error:)
     {
       jsonrpc: Version::V2_0,
-      id: valid_id?(id) ? id : nil,
+      id: valid_id?(id, id_validation_pattern) ? id : nil,
       error: error.compact,
     } unless id.nil?
   end