Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate internal request method names #475

Merged
merged 2 commits into from Sep 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 21 additions & 4 deletions lib/better_errors/middleware.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand All @@ -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

Expand Down Expand Up @@ -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
173 changes: 168 additions & 5 deletions spec/better_errors/middleware_spec.rb
Expand Up @@ -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' }

Expand Down Expand Up @@ -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
Expand All @@ -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