Merge pull request #950 from opf/fix/autocomplete_escapeing_4858

escape work package data for auto-completion #4858

app/controllers/work_packages/auto_completes_controller.rb

@@ -27,6 +27,8 @@
 # See doc/COPYRIGHT.rdoc for more details.
 #++
 
+require 'rack/utils'
+
 class WorkPackages::AutoCompletesController < ApplicationController
   before_filter :find_project
 
@@ -55,9 +57,9 @@ class WorkPackages::AutoCompletesController < ApplicationController
 
   def wp_hash_with_string
     @work_packages.map do |wp|
-      hash = wp.attributes
-      hash['to_s'] = wp.to_s
-      hash
+      Hash[ wp.attributes.map do |key,value|
+        [ key, Rack::Utils.escape_html(value) ]
+      end << ['to_s', Rack::Utils.escape_html(wp.to_s)] ]
     end
   end
 

spec/controllers/work_packages/auto_completes_controller_spec.rb

@@ -117,6 +117,27 @@ describe WorkPackages::AutoCompletesController do
       end
     end
 
+    describe "returns work package for given id" do
+      render_views
+      let(:work_package_4) { FactoryGirl.create(:work_package,
+                                          id: 666,
+                                          subject: "<script>alert('danger!');</script>",
+                                          project: project) }
+      let(:expected_values) { work_package_4 }
+
+      before { get :index,
+                   project_id: project.id,
+                   q: work_package_4.id,
+                   format: :json }
+
+      it_behaves_like "successful response"
+      it_behaves_like "contains expected values"
+
+      it "should escape html" do
+        response.body.should_not include '<script>'
+      end
+    end
+
     describe :cross_project_work_package_relations do
       let(:project_2) { FactoryGirl.create(:project,
                                            parent: project) }
@@ -127,7 +148,7 @@ describe WorkPackages::AutoCompletesController do
       let(:work_package_4) { FactoryGirl.create(:work_package,
                                                 project: project_2) }
 
-      before do 
+      before do
         member_2
 
         work_package_4
@@ -153,7 +174,7 @@ describe WorkPackages::AutoCompletesController do
       context "with scope all but w/o cross project relations" do
         before do
           Setting.stub(:cross_project_work_package_relations?).and_return(false)
-          
+
           get :index,
               project_id: project.id,
               q: work_package_4.id,

spec/views/work_package/auto_complete/index_spec.rb

@@ -0,0 +1,45 @@
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2013 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+
+describe 'work_packages/auto_completes/index.html.erb' do
+  let(:work_package) { FactoryGirl.build( :work_package,
+                                          :subject => '<script>alert("do not alert this");</script>') }
+
+  it 'escapes work package subject in auto-completion' do
+    assign :work_packages, [work_package]
+    render
+    # there are items
+    response.should have_selector "li"
+    # but there is not script tag
+    response.should_not have_selector "script"
+    # normal text should be included
+    response.should include "do not alert this"
+  end
+end