allow more flexible widths/heights in user content

old UserContent.css_size was really weird about what it would accept and
when it would return a String vs. a Float. the times it returned a
Float, it would make api_user_content explode. fix that and add some
specs. the vulnerable code was exercised, among other places, in the
assignment json, which impacts gradebooks and other UI features.

fixes #9881

test-plan:
  - create an assignment in a course
  - in the assignment description, include the html
    <object width='100%' />
  - try and view the gradebook for the course
  - it should not have an ajax request error

Change-Id: I02e824414013347730185fbf7f7fb94a951f3e77
Reviewed-on: https://gerrit.instructure.com/12895
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>

Author: lukfugl

lib/user_content.rb

@@ -72,11 +72,23 @@ def self.find_user_content(html)
     end
   end
 
+  # TODO: try and discover the motivation behind the "huhs"
   def self.css_size(val)
-    res = val.to_f
-    res = nil if res == 0
-    res = (res + 10).to_s + "px" if res && res.to_s == val
-    res
+    if !val || val.to_f == 0
+      # no value, non-numeric value, or 0 value (whether "0", "0px", "0%",
+      # etc.); ignore
+      nil
+    elsif val == "#{val.to_f.to_s}%" || val == "#{val.to_f.to_s}px"
+      # numeric percentage or specific px value; use as is
+      val
+    elsif val.to_f.to_s == val
+      # unadorned numeric value; make px (after adding 10... huh?)
+      (val.to_f + 10).to_s + "px"
+    else
+      # numeric value embedded, but has additional text we didn't recognize;
+      # just extract the numeric part (without a px... huh?)
+      val.to_f.to_s
+    end
   end
 
   class HtmlRewriter

spec/apis/v1/assignment_groups_api_spec.rb

@@ -237,4 +237,19 @@
 
     json.each { |group| group['group_weight'].should be_nil }
   end
+
+  it "should not explode on assignments with <objects> with percentile widths" do
+    course_with_teacher(:active_all => true)
+    group = @course.assignment_groups.create!(:name => 'group')
+    assignment = @course.assignments.create!(:title => "test", :assignment_group => group, :points_possible => 10)
+    assignment.description = '<object width="100%" />'
+    assignment.save!
+
+    api_call(:get, "/api/v1/courses/#{@course.id}/assignment_groups.json?include[]=assignments",
+             :controller => 'assignment_groups',
+             :action => 'index',
+             :format => 'json',
+             :course_id => @course.id.to_s,
+             :include => ['assignments'])
+  end
 end

spec/lib/user_content_spec.rb

@@ -0,0 +1,65 @@
+#
+# Copyright (C) 2011 Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas 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 Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
+
+describe UserContent do
+  describe "find_user_content" do
+    it "should not yield non-string width/height fields" do
+      doc = Nokogiri::HTML::DocumentFragment.parse('<object width="100%" />')
+      UserContent.find_user_content(doc) do |node, uc|
+        uc.width.should == '100%'
+      end
+    end
+  end
+
+  describe "css_size" do
+    it "should be nil for non-numbers" do
+      UserContent.css_size(nil).should be_nil
+      UserContent.css_size('').should be_nil
+      UserContent.css_size('non-number').should be_nil
+    end
+
+    it "should be nil for numbers that equate to 0" do
+      UserContent.css_size('0%').should be_nil
+      UserContent.css_size('0px').should be_nil
+      UserContent.css_size('0').should be_nil
+    end
+
+    it "should preserve percents" do
+      UserContent.css_size('100%').should == '100%'
+    end
+
+    it "should preserve px" do
+      UserContent.css_size('100px').should == '100px'
+    end
+
+    # TODO: these ones are questionable
+    it "should add 10 to raw numbers and make them px" do
+      UserContent.css_size('100').should == '110px'
+    end
+
+    it "should be nil for numbers with an unrecognized prefix" do
+      UserContent.css_size('x-100').should be_nil
+    end
+
+    it "should keep just the raw number from numbers with an unrecognized suffix" do
+      UserContent.css_size('100-x').should == '100'
+    end
+  end
+end