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

Disable risky memory debugging tools by default #402

Merged
merged 5 commits into from
Oct 1, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ collapse_results|`true`|If multiple timing results exist in a single page, colla
max_traces_to_show|20|Maximum number of mini profiler timing blocks to show on one page
html_container|`body`|The HTML container (as a jQuery selector) to inject the mini_profiler UI into
show_total_sql_count|`false`|Displays the total number of SQL executions.
enable_advanced_debugging_tools|`false`|Enables memory debugging tools that can be used via the UI.
OsamaSayegh marked this conversation as resolved.
Show resolved Hide resolved

### Custom middleware ordering (required if using `Rack::Deflate` with Rails)

Expand Down
3 changes: 2 additions & 1 deletion lib/mini_profiler/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def self.default
@disable_env_dump = false
@max_sql_param_length = 0 # disable sql parameter collection by default
@skip_sql_param_names = /password/ # skips parameters with the name password by default
@enable_advanced_debugging_tools = false

# ui parameters
@autorized = true
Expand All @@ -60,7 +61,7 @@ def self.default
:base_url_path, :disable_caching, :disable_env_dump, :enabled,
:flamegraph_sample_rate, :logger, :pre_authorize_cb, :skip_paths,
:skip_schema_queries, :storage, :storage_failure, :storage_instance,
:storage_options, :user_provider
:storage_options, :user_provider, :enable_advanced_debugging_tools
attr_accessor :skip_sql_param_names, :suppress_encoding, :max_sql_param_length

# ui accessors
Expand Down
17 changes: 17 additions & 0 deletions lib/mini_profiler/profiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ def request_authorized?
Thread.current[:mp_authorized]
end

def advanced_tools_message
<<~TEXT
This feature is disabled by default, to enable set the enable_advanced_debugging_tools option to true in Mini Profiler config.
TEXT
end
end

#
Expand Down Expand Up @@ -147,6 +152,14 @@ def config
@config
end

def advanced_debugging_enabled?
config.enable_advanced_debugging_tools
end

def tool_disabled_message(client_settings)
client_settings.handle_cookie(text_result(Rack::MiniProfiler.advanced_tools_message))
end

def call(env)

start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
Expand Down Expand Up @@ -195,12 +208,14 @@ def call(env)

# profile gc
if query_string =~ /pp=profile-gc/
return tool_disabled_message(client_settings) if !advanced_debugging_enabled?
current.measure = false if current
return client_settings.handle_cookie(Rack::MiniProfiler::GCProfiler.new.profile_gc(@app, env))
end

# profile memory
if query_string =~ /pp=profile-memory/
return tool_disabled_message(client_settings) if !advanced_debugging_enabled?
query_params = Rack::Utils.parse_nested_query(query_string)
options = {
ignore_files: query_params['memory_profiler_ignore_files'],
Expand Down Expand Up @@ -308,11 +323,13 @@ def call(env)
end

if query_string =~ /pp=env/ && !config.disable_env_dump
OsamaSayegh marked this conversation as resolved.
Show resolved Hide resolved
return tool_disabled_message(client_settings) if !advanced_debugging_enabled?
body.close if body.respond_to? :close
return client_settings.handle_cookie(dump_env env)
end

if query_string =~ /pp=analyze-memory/
return tool_disabled_message(client_settings) if !advanced_debugging_enabled?
body.close if body.respond_to? :close
return client_settings.handle_cookie(analyze_memory)
end
Expand Down
18 changes: 17 additions & 1 deletion spec/integration/middleware_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,21 @@ def decompressed_response
end
end

describe 'when enable_advanced_debugging_tools is false' do
def app
Rack::Builder.new do
use Rack::MiniProfiler
run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
end
end
it 'advanced tools are disabled' do
%w{env analyze-memory profile-gc profile-memory}.each do |p|
do_get(pp: p)
expect(last_response.body).to eq(Rack::MiniProfiler.advanced_tools_message)
end
end
end

describe 'with analyze-memory query' do
def app
Rack::Builder.new do
Expand All @@ -32,7 +47,8 @@ def app
end
end

it 'should return ObjectSpace statistics' do
it 'should return ObjectSpace statistics if advanced tools are enabled' do
Rack::MiniProfiler.config.enable_advanced_debugging_tools = true
do_get(pp: 'analyze-memory')
expect(last_response.body).to include('Largest strings:')
end
Expand Down
1 change: 1 addition & 0 deletions spec/integration/mini_profiler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ def load_prof(response)
describe 'disable_env_dump config option' do
context 'default (not configured' do
it 'allows env dump' do
Rack::MiniProfiler.config.enable_advanced_debugging_tools = true
get '/html?pp=env'

expect(last_response.body).to include('QUERY_STRING')
Expand Down