Merge pull request #475 from BetterErrors/feature/correct-xhr-mime-type

Validate internal request method names

Author: RobinDaugherty

lib/better_errors/middleware.rb

@@ -75,7 +75,7 @@ def allow_ip?(env)
     def better_errors_call(env)
       case env["PATH_INFO"]
       when %r{/__better_errors/(?<id>.+?)/(?<method>\w+)\z}
-        internal_call env, $~
+        internal_call(env, $~[:id], $~[:method])
       when %r{/__better_errors/?\z}
         show_error_page env
       else
@@ -145,9 +145,10 @@ def backtrace_frames
       end
     end
 
-    def internal_call(env, opts)
+    def internal_call(env, id, method)
+      return not_found_json_response unless %w[variables eval].include?(method)
       return no_errors_json_response unless @error_page
-      return invalid_error_json_response if opts[:id] != @error_page.id
+      return invalid_error_json_response if id != @error_page.id
 
       request = Rack::Request.new(env)
       return invalid_csrf_token_json_response unless request.cookies[CSRF_TOKEN_COOKIE_NAME]
@@ -156,7 +157,9 @@ def internal_call(env, opts)
       body = JSON.parse(request.body.read)
       return invalid_csrf_token_json_response unless request.cookies[CSRF_TOKEN_COOKIE_NAME] == body['csrfToken']
 
-      response = @error_page.send("do_#{opts[:method]}", body)
+      return not_acceptable_json_response unless request.content_type == 'application/json'
+
+      response = @error_page.send("do_#{method}", body)
       [200, { "Content-Type" => "application/json; charset=utf-8" }, [JSON.dump(response)]]
     end
 
@@ -200,5 +203,19 @@ def invalid_csrf_token_json_response
           "or something went wrong.",
       )]]
     end
+
+    def not_found_json_response
+      [404, { "Content-Type" => "application/json; charset=utf-8" }, [JSON.dump(
+        error: "Not found",
+        explanation: "Not a recognized internal call.",
+      )]]
+    end
+
+    def not_acceptable_json_response
+      [406, { "Content-Type" => "application/json; charset=utf-8" }, [JSON.dump(
+        error: "Request not acceptable",
+        explanation: "The internal request did not match an acceptable content type.",
+      )]]
+    end
   end
 end

spec/better_errors/middleware_spec.rb

@@ -294,7 +294,7 @@ def initialize(message, original_exception = nil)
       let(:request_env) {
         Rack::MockRequest.env_for("/__better_errors/#{id}/variables", input: StringIO.new(JSON.dump(request_body_data)))
       }
-      let(:request_body_data) { {"index": 0} }
+      let(:request_body_data) { { "index" => 0 } }
       let(:json_body) { JSON.parse(body) }
       let(:id) { 'abcdefg' }
 
@@ -356,16 +356,159 @@ def initialize(message, original_exception = nil)
               request_env["HTTP_COOKIE"] = "BetterErrors-CSRF-Token=csrfToken123"
             end
 
-            it 'returns the HTML content' do
-              expect(error_page).to receive(:do_variables).and_return(html: "<content>")
+            context 'when the Content-Type of the request is application/json' do
+              before do
+                request_env['CONTENT_TYPE'] = 'application/json'
+              end
+
+              it 'returns JSON containing the HTML content' do
+                expect(error_page).to receive(:do_variables).and_return(html: "<content>")
+                expect(json_body).to match(
+                  'html' => '<content>',
+                )
+              end
+            end
+
+            context 'when the Content-Type of the request is application/json' do
+              before do
+                request_env['HTTP_CONTENT_TYPE'] = 'application/json'
+              end
+
+              it 'returns a JSON error' do
+                expect(json_body).to match(
+                  'error' => 'Request not acceptable',
+                  'explanation' => /did not match an acceptable content type/,
+                )
+              end
+            end
+          end
+
+          context 'when the body csrfToken does not match the CSRF token cookie' do
+            let(:request_body_data) { { "index" => 0, "csrfToken" => "csrfToken123" } }
+            before do
+              request_env["HTTP_COOKIE"] = "BetterErrors-CSRF-Token=csrfToken456"
+            end
+
+            it 'returns a JSON error' do
+              expect(json_body).to match(
+                'error' => 'Invalid CSRF Token',
+                'explanation' => /session might have been cleared/,
+              )
+            end
+          end
+
+          context 'when there is no CSRF token in the request' do
+            it 'returns a JSON error' do
               expect(json_body).to match(
-                'html' => '<content>',
+                'error' => 'Invalid CSRF Token',
+                'explanation' => /session might have been cleared/,
               )
             end
           end
+        end
+      end
+    end
+
+    context "requesting eval for a specific frame" do
+      let(:env) { {} }
+      let(:response_env) {
+        app.call(request_env)
+      }
+      let(:request_env) {
+        Rack::MockRequest.env_for("/__better_errors/#{id}/eval", input: StringIO.new(JSON.dump(request_body_data)))
+      }
+      let(:request_body_data) { { "index" => 0, source: "do_a_thing" } }
+      let(:json_body) { JSON.parse(body) }
+      let(:id) { 'abcdefg' }
+
+      context 'when no errors have been recorded' do
+        it 'returns a JSON error' do
+          expect(json_body).to match(
+            'error' => 'No exception information available',
+            'explanation' => /application has been restarted/,
+          )
+        end
+
+        context 'when Middleman is in use' do
+          let!(:middleman) { class_double("Middleman").as_stubbed_const }
+          it 'returns a JSON error' do
+            expect(json_body['explanation'])
+              .to match(/Middleman reloads all dependencies/)
+          end
+        end
+
+        context 'when Shotgun is in use' do
+          let!(:shotgun) { class_double("Shotgun").as_stubbed_const }
+
+          it 'returns a JSON error' do
+            expect(json_body['explanation'])
+              .to match(/The shotgun gem/)
+          end
+
+          context 'when Hanami is also in use' do
+            let!(:hanami) { class_double("Hanami").as_stubbed_const }
+            it 'returns a JSON error' do
+              expect(json_body['explanation'])
+                .to match(/--no-code-reloading/)
+            end
+          end
+        end
+      end
+
+      context 'when an error has been recorded' do
+        let(:error_page) { ErrorPage.new(exception, env) }
+        before do
+          app.instance_variable_set('@error_page', error_page)
+        end
+
+        context 'but it does not match the request' do
+          it 'returns a JSON error' do
+            expect(json_body).to match(
+              'error' => 'Session expired',
+              'explanation' => /no longer available in memory/,
+            )
+          end
+        end
+
+        context 'and its ID matches the requested ID' do
+          let(:id) { error_page.id }
+
+          context 'when the body csrfToken matches the CSRF token cookie' do
+            let(:request_body_data) { { "index" => 0, "csrfToken" => "csrfToken123" } }
+            before do
+              request_env["HTTP_COOKIE"] = "BetterErrors-CSRF-Token=csrfToken123"
+            end
+
+            context 'when the Content-Type of the request is application/json' do
+              before do
+                request_env['CONTENT_TYPE'] = 'application/json'
+              end
+
+              it 'returns JSON containing the eval result' do
+                expect(error_page).to receive(:do_eval).and_return(prompt: '#', result: "much_stuff_here")
+                expect(json_body).to match(
+                  'prompt' => '#',
+                  'result' => 'much_stuff_here',
+                )
+              end
+            end
+
+            context 'when the Content-Type of the request is application/json' do
+              before do
+                request_env['HTTP_CONTENT_TYPE'] = 'application/json'
+              end
+
+              it 'returns a JSON error' do
+                expect(json_body).to match(
+                  'error' => 'Request not acceptable',
+                  'explanation' => /did not match an acceptable content type/,
+                )
+              end
+            end
+          end
 
           context 'when the body csrfToken does not match the CSRF token cookie' do
-            let(:request_body_data) { {"index": 0, "csrfToken": "csrfToken123"} }
+            let(:request_body_data) { { "index" => 0, "csrfToken" => "csrfToken123" } }
             before do
               request_env["HTTP_COOKIE"] = "BetterErrors-CSRF-Token=csrfToken456"
             end
@@ -389,5 +532,25 @@ def initialize(message, original_exception = nil)
         end
       end
     end
+
+    context "requesting an invalid internal method" do
+      let(:env) { {} }
+      let(:response_env) {
+        app.call(request_env)
+      }
+      let(:request_env) {
+        Rack::MockRequest.env_for("/__better_errors/#{id}/invalid", input: StringIO.new(JSON.dump(request_body_data)))
+      }
+      let(:request_body_data) { { "index" => 0 } }
+      let(:json_body) { JSON.parse(body) }
+      let(:id) { 'abcdefg' }
+
+      it 'returns a JSON error' do
+        expect(json_body).to match(
+          'error' => 'Not found',
+          'explanation' => /recognized internal call/,
+        )
+      end
+    end
   end
 end