From 1f69b76ef30d9e064537e4e82b9d4f51be558afe Mon Sep 17 00:00:00 2001
From: Azul <azul@riseup.net>
Date: Thu, 11 May 2017 09:55:03 +0200
Subject: [PATCH] respond with 400 on invalid input

---
 lib/nickserver/client_error.rb      |  4 ++++
 lib/nickserver/error_response.rb    |  6 +++---
 lib/nickserver/reel_server.rb       | 15 ++++++++++++---
 test/integration/dispatcher_test.rb |  2 +-
 test/unit/error_response_test.rb    |  6 ++++--
 5 files changed, 24 insertions(+), 9 deletions(-)
 create mode 100644 lib/nickserver/client_error.rb

diff --git a/lib/nickserver/client_error.rb b/lib/nickserver/client_error.rb
new file mode 100644
index 0000000..bb532fd
--- /dev/null
+++ b/lib/nickserver/client_error.rb
@@ -0,0 +1,4 @@
+module Nickserver
+  class ClientError < StandardError
+  end
+end
diff --git a/lib/nickserver/error_response.rb b/lib/nickserver/error_response.rb
index 9d53630..9ec3866 100644
--- a/lib/nickserver/error_response.rb
+++ b/lib/nickserver/error_response.rb
@@ -3,12 +3,12 @@ require 'nickserver/response'
 module Nickserver
   class ErrorResponse < Nickserver::Response
     def initialize(message)
-      @status = 500
-      @message = message + "\n"
+      @status = 400
+      @message = message
     end
 
     def content
-      "#{status} #{message}"
+      JSON.generate(error: message)
     end
 
     protected
diff --git a/lib/nickserver/reel_server.rb b/lib/nickserver/reel_server.rb
index c378aca..17ad441 100644
--- a/lib/nickserver/reel_server.rb
+++ b/lib/nickserver/reel_server.rb
@@ -6,6 +6,7 @@ require 'nickserver/config'
 require 'nickserver/adapters/celluloid_http'
 require 'nickserver/dispatcher'
 require 'nickserver/logging_responder'
+require 'nickserver/client_error'
 
 module Nickserver
   class ReelServer < Reel::Server::HTTP
@@ -35,16 +36,24 @@ module Nickserver
     protected
 
     def handle_request(request)
-      logger.info "#{request.method} #{request.uri}"
-      logger.debug "  #{params(request)}"
+      log_request(request)
       handler = handler_for(request)
       handler.respond_to params(request), request.headers
+    rescue ClientError => e
+      logger.warn e
+      request.respond 400, JSON.generate(error: e.message)
     rescue StandardError => e
       logger.error e
-      logger.error e.backtrace.join "\n  "
       request.respond 500, "{}"
     end
 
+    def log_request(request)
+      logger.info "#{request.method} #{request.uri}"
+      logger.debug "  #{params(request)}"
+    rescue URI::Error => e
+      raise ClientError, e.message
+    end
+
     def handler_for(request)
       # with reel the request is the responder
       responder = LoggingResponder.new(request, logger)
diff --git a/test/integration/dispatcher_test.rb b/test/integration/dispatcher_test.rb
index b551e87..b2582bd 100644
--- a/test/integration/dispatcher_test.rb
+++ b/test/integration/dispatcher_test.rb
@@ -123,7 +123,7 @@ class Nickserver::DispatcherTest < Minitest::Test
   end
 
   def error(msg)
-    response status: 500, content: "500 #{msg}\n"
+    response status: 400, content: JSON.generate(error: msg)
   end
 
   def http_connection_error
diff --git a/test/unit/error_response_test.rb b/test/unit/error_response_test.rb
index 7242b38..2efe533 100644
--- a/test/unit/error_response_test.rb
+++ b/test/unit/error_response_test.rb
@@ -1,12 +1,14 @@
 require 'test_helper'
 require 'nickserver/error_response'
+require 'json'
 
 class ErrorResponseTest < Minitest::Test
 
   def test_content
     response = Nickserver::ErrorResponse.new "Not a valid address"
-    assert_equal "500 Not a valid address\n", response.content
-    assert_equal 500, response.status
+    assert_equal 400, response.status
+    assert_equal JSON.generate(error: "Not a valid address"),
+      response.content
   end
 
 end
-- 
GitLab