Permalink
Browse files

Merge pull request #71 from SealedEnvelope/raw-output-in-notifier

Don't html escape build changeset, output and project settings in the build report email.

Happily merged. Thanks for the pull request, and thanks for including a test!
  • Loading branch information...
2 parents 69ba6c8 + 3e33eb1 commit a7b64f39f6b89e4c2884d3233410870e249347f7 @bguthrie bguthrie committed Aug 31, 2011
Showing with 24 additions and 14 deletions.
  1. +4 −4 app/views/build_mailer/build_report.html.erb
  2. +20 −10 test/unit/plugins/email_notifier_test.rb
@@ -3,7 +3,7 @@
CHANGES
-------
-<%= @build.changeset %>
+<%= raw @build.changeset %>
<% unless @failures_and_errors.empty? -%>
TEST FAILURES AND ERRORS
@@ -19,7 +19,7 @@ See <%= "#{@build.url}" %> for details.
CHANGES
-------
-<%= @build.changeset %>
+<%= raw @build.changeset %>
BUILD LOG
---------
@@ -28,9 +28,9 @@ The output below has been truncated to <%= number_to_human_size(Configuration.ma
View <%= link_to "the full log", build_artifact_path(:project => @build.project.name, :build => @build.label, :path => 'build.log') %> for the rest.
<% end %>
-<%= @build.output %>
+<%= raw @build.output %>
PROJECT SETTINGS
----------------
-<%= @build.project_settings %>
+<%= raw @build.project_settings %>
<% end -%>
@@ -2,11 +2,12 @@
class EmailNotifierTest < ActiveSupport::TestCase
include FileSandbox
-
+
BUILD_LOG = <<-EOL
blah blah blah
something built
tests passed / failed / etc
+ <this should be plain text & should not be escaped>
EOL
setup do
@@ -22,10 +23,10 @@ class EmailNotifierTest < ActiveSupport::TestCase
@notifier = EmailNotifier.new
@notifier.emails = ["default@gmail.com", "ccrb@thoughtworks.com"]
@notifier.from = 'cruisecontrol@thoughtworks.com'
-
+
@project.add_plugin(@notifier, :test_email_notifier)
end
-
+
teardown do
teardown_sandbox
end
@@ -61,7 +62,7 @@ class EmailNotifierTest < ActiveSupport::TestCase
CruiseControl::Log.expects(:event).never
BuildMailer.expects(:build_report).never
@notifier.emails = []
- @notifier.build_finished(failing_build)
+ @notifier.build_finished(failing_build)
end
test "should log a single recipient if an email is sent" do
@@ -83,11 +84,11 @@ class EmailNotifierTest < ActiveSupport::TestCase
CruiseControl::Log.expects(:event).with("Error sending e-mail - current server settings are :\n :foo = 5", :error)
mock_mail = mock("Email")
mock_mail.expects(:deliver).raises('oh noes!')
-
+
BuildMailer.expects(:build_report).returns mock_mail
-
+
@notifier.emails = ['foo@crapty.com']
-
+
assert_raise_with_message(RuntimeError, 'oh noes!') do
@notifier.build_finished(failing_build)
end
@@ -97,7 +98,7 @@ class EmailNotifierTest < ActiveSupport::TestCase
Configuration.expects(:email_from).returns('central@foo.com')
@notifier.from = nil
build = failing_build()
-
+
BuildMailer.expects(:build_report).with(build, ['default@gmail.com', 'ccrb@thoughtworks.com'],
'central@foo.com', 'myproj build 5 failed', 'The build failed.').returns mock(:deliver => true)
@@ -108,7 +109,7 @@ class EmailNotifierTest < ActiveSupport::TestCase
Configuration.stubs(:dashboard_url).returns("http://www.my.com")
@notifier.emails = ['foo@happy.com']
@notifier.build_finished(failing_build)
-
+
mail = ActionMailer::Base.deliveries[0]
assert_match /http:\/\/www.my.com\/builds\/myproj\/5/, mail.body.to_s
end
@@ -121,10 +122,19 @@ class EmailNotifierTest < ActiveSupport::TestCase
mail = ActionMailer::Base.deliveries[0]
assert_match /Note: if you set Configuration\.dashboard_url in site_config\.rb/, mail.body.to_s
end
+
+ test "should contain unescaped build output in the notification email if the dashboard url is not set" do
+ Configuration.stubs(:dashboard_url).returns(nil)
+ @notifier.emails = ['foo@happy.com']
+ @notifier.build_finished(failing_build)
+
+ mail = ActionMailer::Base.deliveries[0]
+ assert_match /<this should be plain text & should not be escaped>/, mail.body.to_s
+ end
end
private
-
+
def failing_build
@build.stubs(:failed?).returns(true)
@build.stubs(:output).returns(BUILD_LOG)

0 comments on commit a7b64f3

Please sign in to comment.