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

Allow editor links to work inside an iframe or with CSP that prohibits other protocols #440

Merged
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
22 changes: 22 additions & 0 deletions README.md
Expand Up @@ -35,6 +35,28 @@ end

_Note: If you discover that Better Errors isn't working - particularly after upgrading from version 0.5.0 or less - be sure to set `config.consider_all_requests_local = true` in `config/environments/development.rb`._

### Optional: Set `EDITOR`

For many reasons outside of Better Errors, you should have the `EDITOR` environment variable set to your preferred
editor.
Better Errors, like many other tools, will use that environment variable to show a link that opens your
editor to the file and line from the console.

By default the links will open TextMate-protocol links.

To see if your editor is supported or to set up a different editor, see [the wiki](https://github.com/BetterErrors/better_errors/wiki/Link-to-your-editor).

### Optional: Set `BETTER_ERRORS_INSIDE_FRAME`

If your application is running inside of an iframe, or if you have a Content Security Policy that disallows links
to other protocols, the editor links will not work.

To work around this set `BETTER_ERRORS_INSIDE_FRAME=1` in the environment, and the links will include `target=_blank`,
allowing the link to open regardless of the policy.

_This works because it opens the editor from a new browser tab, escaping from the restrictions of your site._
_Unfortunately it leaves behind an empty tab each time, so only use this if needed._

## Security

**NOTE:** It is *critical* you put better\_errors only in the **development** section of your Gemfile.
Expand Down
9 changes: 8 additions & 1 deletion lib/better_errors/templates/variable_info.erb
@@ -1,7 +1,14 @@
<header class="trace_info clearfix">
<div class="title">
<h2 class="name"><%= @frame.name %></h2>
<div class="location"><span class="filename"><a href="<%= editor_url(@frame) %>"><%= @frame.pretty_path %></a></span></div>
<div class="location">
<span class="filename">
<a
href="<%= editor_url(@frame) %>"
<%= ENV.key?('BETTER_ERRORS_INSIDE_FRAME') ? "target=_blank" : '' %>
><%= @frame.pretty_path %></a>
</span>
</div>
</div>
<div class="code_block clearfix">
<%== html_formatted_code_block @frame %>
Expand Down
44 changes: 30 additions & 14 deletions spec/better_errors/error_page_spec.rb
Expand Up @@ -76,15 +76,44 @@ module BetterErrors
end

context "variable inspection" do
let(:html) { error_page.do_variables("index" => 0)[:html] }
let(:exception) { exception_binding.eval("raise") rescue $! }

it 'includes an editor link for the full path of the current frame' do
expect(html).to have_tag('.location .filename') do
with_tag('a[href*="better_errors"]')
end
end

context 'when BETTER_ERRORS_INSIDE_FRAME is set in the environment' do
before do
ENV['BETTER_ERRORS_INSIDE_FRAME'] = '1'
end
after do
ENV['BETTER_ERRORS_INSIDE_FRAME'] = nil
end

it 'includes an editor link with target=_blank' do
expect(html).to have_tag('.location .filename') do
with_tag('a[href*="better_errors"][target="_blank"]')
end
end
end

context 'when BETTER_ERRORS_INSIDE_FRAME is not set in the environment' do
it 'includes an editor link without target=_blank' do
expect(html).to have_tag('.location .filename') do
with_tag('a[href*="better_errors"]:not([target="_blank"])')
end
end
end

context "when binding_of_caller is loaded" do
before do
skip "binding_of_caller is not loaded" unless BetterErrors.binding_of_caller_available?
end

it "shows local variables" do
html = error_page.do_variables("index" => 0)[:html]
expect(html).to have_tag('div.variables tr') do
with_tag('td.name', text: 'local_a')
with_tag('pre', text: ':value_for_local_a')
Expand All @@ -96,7 +125,6 @@ module BetterErrors
end

it "shows instance variables" do
html = error_page.do_variables("index" => 0)[:html]
expect(html).to have_tag('div.variables tr') do
with_tag('td.name', text: '@inst_c')
with_tag('pre', text: ':value_for_inst_c')
Expand All @@ -123,7 +151,6 @@ module BetterErrors
}

it "does not include that value" do
html = error_page.do_variables("index" => 0)[:html]
expect(html).to have_tag('div.variables tr') do
with_tag('td.name', text: 'local_a')
with_tag('pre', text: ':value_for_local_a')
Expand All @@ -147,7 +174,6 @@ module BetterErrors

it "does not show filtered variables" do
allow(BetterErrors).to receive(:ignored_instance_variables).and_return([:@inst_d])
html = error_page.do_variables("index" => 0)[:html]
expect(html).to have_tag('div.variables tr') do
with_tag('td.name', text: '@inst_c')
with_tag('pre', text: ':value_for_inst_c')
Expand All @@ -174,7 +200,6 @@ module BetterErrors
let(:content) { 'A' * 480 }

it "shows the variable content" do
html = error_page.do_variables("index" => 0)[:html]
expect(html).to have_tag('div.variables', text: %r{#{content}})
end
end
Expand All @@ -199,7 +224,6 @@ def inspect
}

it "shows the variable content" do
html = error_page.do_variables("index" => 0)[:html]
expect(html).to have_tag('div.variables', text: /shortval/)
end
end
Expand All @@ -212,7 +236,6 @@ def inspect
let(:content) { 'A' * 1101 }

it "includes an indication that the variable was too large" do
html = error_page.do_variables("index" => 0)[:html]
expect(html).not_to have_tag('div.variables', text: %r{#{content}})
expect(html).to have_tag('div.variables', text: /Object too large/)
end
Expand All @@ -230,7 +253,6 @@ def initialize
}

it "does not attempt to show the class name" do
html = error_page.do_variables("index" => 0)[:html]
expect(html).to have_tag('div.variables tr') do
with_tag('td.name', text: '@big_anonymous')
with_tag('.unsupported', text: /Object too large/)
Expand Down Expand Up @@ -258,7 +280,6 @@ def initialize
let(:content) { 'A' * 480 }

it "shows the variable content" do
html = error_page.do_variables("index" => 0)[:html]
expect(html).to have_tag('div.variables', text: %r{#{content}})
end
end
Expand All @@ -283,7 +304,6 @@ def inspect
}

it "shows the variable content" do
html = error_page.do_variables("index" => 0)[:html]
expect(html).to have_tag('div.variables', text: /shortval/)
end
end
Expand All @@ -297,7 +317,6 @@ def inspect

it "includes an indication that the variable was too large" do

html = error_page.do_variables("index" => 0)[:html]
expect(html).not_to have_tag('div.variables', text: %r{#{content}})
expect(html).to have_tag('div.variables', text: /Object too large/)
end
Expand All @@ -316,7 +335,6 @@ def initialize
}

it "does not attempt to show the class name" do
html = error_page.do_variables("index" => 0)[:html]
expect(html).to have_tag('div.variables tr') do
with_tag('td.name', text: '@big_anonymous')
with_tag('.unsupported', text: /Object too large/)
Expand All @@ -340,7 +358,6 @@ def initialize
let(:content) { 'A' * 100_001 }

it "includes the content of large variables" do
html = error_page.do_variables("index" => 0)[:html]
expect(html).to have_tag('div.variables', text: %r{#{content}})
expect(html).not_to have_tag('div.variables', text: /Object too large/)
end
Expand All @@ -353,7 +370,6 @@ def initialize
end

it "tells the user to add binding_of_caller to their gemfile to get fancy features" do
html = error_page.do_variables("index" => 0)[:html]
expect(html).not_to have_tag('div.variables', text: /gem "binding_of_caller"/)
end
end
Expand Down