Wolfmans Howlings

A programmers Blog about Ruby, Rails and a few other issues

Bit Vector Preferences

Posted by Jim Morris Wed, 08 Aug 2007 06:53:09 GMT

In my latest web project I potentially have a lot of boolean preferences, which I use for enabling or disabling various email notifications to users.

Rather than having to add a migration everytime I want to add a new preference, I thought I would use the composed_of feature in my model and compose the boolean preferences from a bitvector. That way I can simply modify my model to add new preferences rather than add new columns to the database.

I also wanted this to be easy to add new boolean preferences, so I use some Macros (I guess you could also call it Meta-Programming) to do all the repetitive code.

The result is a little class in my Person model called Preferences, one integer field in my persons database called preferences, and a composed_of :preferences in the Person model, and of course the following class in the person.rb model.

class Person < ActiveRecord::Base

  #...

  # place to store bit vector preferences
  # to add a new preference:-
  #   add symbol of preference to @@bits with bit allocation
  #   update initialize defaults if initial default is true
  class Preferences
    @@bits= {
      :comment_notifications => 1,
      :friendship_notifications => 2,
      :event_notifications => 4,
      :misc_notifications => 8 }

    # create a reader for each preference
    @@bits.each_key do |a|
      attr_reader a
    end

    # Initialize from integer or Hash
    def initialize(prefs)
      if prefs.nil?
        # set the defaults to false if not been set before
        @@bits.each do |a, v|
          instance_variable_set("@#{a}", false)  
        end
        # override default here        
        @comment_notifications= true
        @event_notifications= true
      elsif prefs.is_a?(Hash)
        # initialize from parameter Hash, and default to false if absent from hash
        @@bits.each do |a, v|
          instance_variable_set("@#{a}", false)  
        end

        prefs.each do |k,v|
          raise(ArgumentError, "Unknown preference #{k}") unless @@bits.has_key?(k.to_sym)
          instance_variable_set("@#{k}", true) if v == '1'
        end
      else
        # create from integer bit vector
        @@bits.each do |a, v|
          instance_variable_set("@#{a}", (prefs & v) != 0 ? true : false)  
        end
      end
    end

    # returns bit vector of preferences
    def preferences
      bv= 0
      @@bits.each do |a, v|
        bv |= instance_variable_get("@#{a}") ? v : 0  
      end
      return bv
    end

    # create a predicate for each preference
    @@bits.each_key do |a|
      alias_method((a.to_s + '?').to_sym, a)
    end
  end

  # access preferences as bit vector
  composed_of :preferences

  #...

end

All I need to do to add new preferences is add it to the @@bits class variable, which is a Hash of the preference name as a symbol and the bit it sets in the integer (actually the value of the bit, bit0 is 1, bit1 is 2 etc). The rest of the code is derived from the class variable.

To make things easier I also add a predicate for each preference, so I can access @person.preferences.comment_notifications? to see if any comment notifications are required for instance.

One other thing I do in the initialize method is set up defaults for the preferences. This is only really needed if it is being added as an after thought, and the column preferences is NULL in the database.

Because composed_of classes are immutable you must always create a whole new one to update them, so I also allow initialize to be called with a Hash, which can come straight from the controller. The last case of initialize is being passed the integer from the database, expanding it into the various boolean instance variables. The preferences method does the reverse and converts the boolean instance variables into the bit vector. Calls to these are all taken care of by ActiveRecord.

An example of it being called from the controller is...

@person.preferences= Person::Preferences.new(params[:preferences])

presuming you have a bunch of check boxes in your view which are passed in as part of the preferences hash.

Because I added this later I had one migration to initially add the new column...

add_column :people, :preferences, :integer

Posted in  | Tags , , ,  | 1 comment | no trackbacks

RSpec testing all actions of a controller

Posted by Jim Morris Sat, 28 Jul 2007 21:23:07 GMT

A pattern I find very helpful is to find all the actions in a controller and apply a test to all those actions.

For instance this is useful for automatically testing all actions are protected from unauthorized access when using a login system.

One nice feature of this pattern is that if you add an action to a controller it will automatically be tested. This is less helpful if you use

before_filter :login_required, :except => {...}

as it will automatically be protected, but there are other use cases where this is not the situation. Just as in the except clause above you need to explicitly add any action that does not need to be tested to an exception list, which is supported by this pattern.

Here are the methods I use to test for login accessibility.

module MySpecHelper

  # get all actions for specified controller
  def get_all_actions(cont)
    c= Module.const_get(cont.to_s.pluralize.capitalize + "Controller")
    c.public_instance_methods(false).reject{ |action| ['rescue_action'].include?(action) }
  end

  # test actions fail if not logged in
  # opts[:exclude] contains an array of actions to skip
  # opts[:include] contains an array of actions to add to the test in addition
  # to any found by get_all_actions
  def controller_actions_should_fail_if_not_logged_in(cont, opts={})
    except= opts[:except] || []
    actions_to_test= get_all_actions(cont).reject{ |a| except.include?(a) }
    actions_to_test += opts[:include] if opts[:include]
    actions_to_test.each do |a|
      #puts "... #{a}"
      get a
      response.should_not be_success
      response.should redirect_to('http://test.host/login')
      flash[:warning].should == @login_warning
   end
 end
end

I put this in my spec_helper.rb and include it as shown here:

describe "When Logged out" do
  include MySpecHelper
  controller_name :events

  before(:each) do
    controller.stub!(:current_user).and_return(:false)
    @login_warning= "You need to be logged in to do that"
  end

  # test all actions require login except the ones specified
  # add new_comment as it is not seen by the automatic collector
  it "actions should fail" do
    controller_actions_should_fail_if_not_logged_in(:input, 
                              :except => ['index', 'show', 'tagged'], 
                              :include => ['new_comment'])
  end
end

The get_all_actions method collects all the public un-inherited methods in the given controller, these will consist of all the accessible actions in that controller. I explicitly exclude rescue_action as it is created by RSpec itself and should not be tested. Note it will not see any actions that are in application.rb so you need to add those to the list manually of you want them tested. (See the :include option in the example).

The controller_actions_should_fail_if_not_logged_in could be put in the spec itself rather than the spec_helper, but as I call this from all my controller specs it is more DRY to put it here. This method takes the controller name and an option array of actions names to ignore. This method tests all the actions and makes sure I get the expected result of the filter failing due to not being logged in.

I show an example spec that uses this to test my events controller, it mocks the login calls to say I am not logged in, and then tests them with the exceptions of the actions in this controller that do not require one to be logged in.

This pattern can be extended to test all sorts of things, and is especially useful for testing things where you can add an action and forget to do something in a filter to protect it. Make sure the default is on the side of caution though. IE you need to explicitly except actions rather than include actions.

Another example is something I recently stumbled upon in my RESTful controllers. In many cases it is good to use a verify statement to make sure that the RESTful actions actually can only be called with PUT, POST or DELETE and fail if called with GET. I use this statement in my controllers to enforce this...

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :put, :only => [ :update ], :add_flash => { :error => "Operation Failed" }, :redirect_to => { :action => :index }
verify :method => :post, :only => [ :create, :new_comment ], :add_flash => { :error => "Operation Failed" }, :redirect_to => { :action => :index }
verify :method => :delete, :only => [ :destroy ], :add_flash => { :error => "Operation Failed" }, :redirect_to => { :action => :index }

I test this in my specs using this in the MySpecHelper Module

  def controller_actions_should_fail_with_get(cont, except=[])
    actions_to_test= get_all_actions(cont).reject{ |a| except.include?(a) }
    actions_to_test.each do |a|
      #puts "... #{a}"
      get a
      response.should redirect_to("http://test.host/#{cont.to_s.pluralize}")
      flash[:error].should == 'Operation Failed'
    end
  end

and an example of its use in a spec...

  it "actions should fail if not post or put" do
    controller_actions_should_fail_with_get(:event, ['index', 'show', 'edit', 'new'])
  end                                                                                     

Now whenever I add an action, the default is that it will fail with a GET, unless I add it to the exclude list in the spec, this will remind me to check if the action required PUT, POST or DELETE instead and to add it to the verify if so or add it to the specs exclude list if not.

These automatic tests keep me honest, especially in the last case where you really don't want a GET to be able to delete something.

I hope this pattern is useful to you.

Posted in ,  | Tags , ,  | 12 comments | no trackbacks

RSpec testing views for escaped HTML

Posted by Jim Morris Sat, 07 Jul 2007 01:22:00 GMT

For my social networking site snowdogsr.us I decided to escape all user input that gets displayed. I know people like to trick out their profiles with HTML but I want to avoid the various hacks that it allows.

So thinking I had done a good job of using h everywhere I output user input fields, I decided to see if I could actually test this with RSpec view tests.

I recently switched to RSpec for my testing needs, its cool :)

One thing it does is isolate the various things for testing using built in mocking, and views can be entirely tested standalone without accessing a model or a controller.

So how would it do testing for escaped user input I wondered?

Very well actually.

An example is worth a thousand words, so here is my RSpec for my home page.

BTW I found about 4 places where embedded HTML in user input was bleeding through, so it was well worth the effort.

So this goes in spec/views/home/home_spec.rb...

  it "should escape all user input" do
    @place= mock_model(Place, :name => 'place name<b>', :location => 'place location<b>', :tag_list => "place taglist <b>", :rated? => false)
    @event= mock_model(Event, :name => 'event name<b>', :where => 'event where<b>', :tag_list => "event taglist <b>", :date_time => DateTime.now, :hosted_by => 'Event host<b>')
    @post= mock_model(Input, :input => 'input body <b>', :tag_list => "post taglist <b>", :updated_at => DateTime.now, :created_at => DateTime.now, :created_by => 'post created by person<b>', :rated? => false)
    @picture= mock_model(Picture, :public_filename => "filename<b>.png")
    @pictures= [@picture]
    @pet= mock_model(Pet, :name => 'pet name<b>', :owned_by => "pet owner <b>", :breed => 'breed <b>', :description => "pet description <b>", :neutered => true, :gender => 'M<b>', :pictures => @pictures, :owned_by? => false)

    @posts= [@post]
    @events= [@event]
    @places= [@place]
    @top_places= [@place]
    @new_pets= [@pet]

    @comment= mock_model(Comment)
    @comment.stub!(:user).and_return(@user)
    @comment.stub!(:created_at).and_return(DateTime.now)
    @comment.stub!(:comment).and_return('comment body <b>')
    @comments= [@comment]
    @post.should_receive(:comments).and_return(@comments)

    @new_stuff= []
    @new_stuff << {:list => @posts, :title => 'Posts', :link => '#'}
    @new_stuff << {:list => @events, :title => 'Events', :link => '#'}
    @new_stuff << {:list => @places, :title => 'Places', :link => '#'}

    @top= []
    @top << {:list => @top_places, :title => 'Hot Places', :link => '#'}

    assigns[:new_stuff] = @new_stuff
    assigns[:top] = @top
    assigns[:new_pets] = @new_pets

    render "/home/logged_in"   
    #puts excerpt(response.body, "<b>")
    response.should_not have_text(/<b>/)
  end

Its quite complex as the home page renders a lot of summaries of the various lists I have.

First I mock the models that are called, and stub out the calls that are made to them. I force them all to return an embedded <b> which I don't use anyway, and with the new CSS oriented web styles shouldn't be used in HTML anyway.

Then I just test that <b> does not appear anywhere. If I have correctly used h to escape all the inputs then it should be rendered as &lt;b&gt; instead.

The response.should_not have_text(/<b>/) should do that test.

One cool thing is the mocking will tell you if any new inputs (ie calls to model attributes) have been added, or if you have forgotten any. So this should keep you honest in the future if you add new attributes that need escaping.

The

assigns[:new_stuff] = @new_stuff
assigns[:top] = @top
assigns[:new_pets] = @new_pets

Sets the assigns to the variables that my view uses,simulating what the controller would pass in.

The mock_model calls at the top also use a shortcut to define all the attributes that get called, and what they return. You can also explicitly do this...

@post.should_receive(:comments).and_return(@comments)

If you read the RSpec docs you can see that you can also test for parameters passed in, how many times it is called and various other nice things.

I added this snippet taken from the rails helpers to aid in finding any errant HTML that bleads through. (I'm not sure how to call it from the RSpec so I just copied the code into a private method).


 private

  def excerpt(text, phrase, radius = 100, excerpt_string = "...")
    if text.nil? || phrase.nil? then return end
    phrase = Regexp.escape(phrase)

    if found_pos = text.chars =~ /(#{phrase})/i
      start_pos = [ found_pos - radius, 0 ].max
      end_pos   = [ found_pos + phrase.chars.length + radius, text.chars.length ].min

      prefix  = start_pos > 0 ? excerpt_string : ""
      postfix = end_pos < text.chars.length ? excerpt_string : ""

      prefix + text.chars[start_pos..end_pos].strip + postfix
    else
      nil
    end
  end

and you can see the call that shows me where the errant <b> is...

puts excerpt(response.body, "<b>")

I also have some setup code that handles the login and log out mocking, but I'll leave that for the end user to sort out ;)

So I think this will make sure that now and in the future this particular view will not bleed user input HTML.

Once I did the complex one above the rest of the views were much easier and quicker to implement. Here is an example of a really simple one...

  it "should escape all user input" do
    @person= mock_model(Person, :name => 'person name <b>', :first_name => 'person first name <b>', :last_name => 'person last name <b>', :alias => 'person alias <b>', :show_gender => 'Male', :about_me => 'about <b>', :updated_at => DateTime.now, :created_at => DateTime.now, :pets => [])    

    assigns[:person] = @person

    render "/people/show"   

    response.should_not have_text(/<b>/)
  end

Couldn't be much simpler, but I found one place where I was not escaping the HTML!

Posted in ,  | Tags , ,  | 4 comments | no trackbacks

REST scaffold_resource security warning

Posted by Jim Morris Tue, 26 Jun 2007 22:14:04 GMT

This one is so blatantly obvious it bit me in the Butt at 4am this morning when I had to get up and fix it! I am so embarrassed, luckily no private data got out, as no-one has entered any private data yet.

I used the script/generate scaffold_resource to get started, and I left in those nice format.xml things in, thinking I may use them in the future. For the most part this is not a problem, but one of my controllers is a profile table. Much of the data in there is public anyway so no big deal, but a few columns are private data like email, date of birth, phone numbers etc. These are specifically private and not viewable publicly. This is enforced but not having a view that shows any of that stuff to the general public.

However the tricky little scaffold-generated code...

  def index
    @profiles = Profile.find(:all, :order => "first_name, last_name, alias")

    respond_to do |format|
      format.html # index.rhtml
      format.xml  { render :xml => @profiles.to_xml }
     end
  end

Has this cool .to_xml stanza, which happily takes every column and converts it to XML and sends it back as a response to the query /profiles.xml

Yikes, I woke up with a start when I realized that, and rushed to test it and yep it works as it is supposed to.

Obviously this is easy to fix, Just exclude the attributes you don't want shown:

@profiles.to_xml(:only => [:first_name, :last_name])

But it sure is a nasty back door if you forget!

Caveat Programmer!

Posted in  | Tags ,  | 4 comments | no trackbacks

Older posts: 1 2 3 ... 6