Error Handling in Scaffolds

rubyonrails-talk
1

Howdy all,

I've never been one to use scaffolds in my Rails projects, but I'm looking at it right now as a possible time saver. I spend a ridiculous amount of time building the forms, and slightly less (but more than I'd like) building the controller(s) for a new model when developing my applications.

However, I've taken a quick look at both the built in script/generate scaffold and it seems to be sorely lacking in error handling of any kind.

For example, say I generate a quick scaffold for "Event": script/generate scaffold Event ...

Some resulting code looks like this:

  def edit     @event = Event.find(params[:id])   end

What happens if I pass a non-existent ID there? An ActiveRecord::RecordNotFound exception gets raised, causing an error 500 to be displayed to the user in production, and maybe an exception notification email cluttering my inbox.

It doesn't stop there. Look at this:

  def create     @event = Event.new(params[:event])

    respond_to do |format|       if @event.save         flash[:notice] = 'Event was successfully created.'         format.html { redirect_to(@event) }         format.xml { render :xml => @event, :status => :created, :location => @event }       else         format.html { render :action => "new" }         format.xml { render :xml => @event.errors, :status => :unprocessable_entity }       end     end   end

Okay, all well and good. However, what happens if I execute GET on this action instead of POST? My understanding is that GET requests are *strictly* to "get" data. However, in theory, I can pass a bunch of URL encoded variables to this action and create a new event, leaving this application wide open to XSS (assuming of course an authorized user is logged in and clicks a malicious link)

Please PLEASE don't see this as a "bitch and moan" post - far from it. I point this out to show my own ignorance: I assume that I'm missing something here.

So, what's the deal? Is this REALLY lacking in error support/ handling, or am I just not using it right?

Thanks :slight_smile:

2

Howdy all,

I've never been one to use scaffolds in my Rails projects, but I'm looking at it right now as a possible time saver. I spend a ridiculous amount of time building the forms, and slightly less (but more than I'd like) building the controller(s) for a new model when developing my applications.

However, I've taken a quick look at both the built in script/generate scaffold and it seems to be sorely lacking in error handling of any kind.

For example, say I generate a quick scaffold for "Event": script/generate scaffold Event ...

Some resulting code looks like this:

def edit @event = Event.find(params[:id]) end

What happens if I pass a non-existent ID there? An ActiveRecord::RecordNotFound exception gets raised, causing an error 500 to be displayed to the user in production, and maybe an exception notification email cluttering my inbox.

It doesn't stop there. Look at this:

def create @event = Event.new(params[:event])

respond_to do |format| if @event.save flash[:notice] = 'Event was successfully created.' format.html { redirect_to(@event) } format.xml { render :xml => @event, :status => :created, :location => @event } else format.html { render :action => "new" } format.xml { render :xml => @event.errors, :status => :unprocessable_entity } end end end

Okay, all well and good. However, what happens if I execute GET on this action instead of POST? My understanding is that GET requests are *strictly* to "get" data. However, in theory, I can pass a bunch of URL encoded variables to this action and create a new event, leaving this application wide open to XSS (assuming of course an authorized user is logged in and clicks a malicious link)

I don't think you can get to create with a GET. Try it and see. Unless you have added a route yourself of course.

Colin

3

Hi Colin,

Thanks for the reply. I just did try this and unfortunately, my original assessment was correct: vulnerable to XSS.

Here's how to repeat: Make a new rails app, then generate a scaffold: script/generate scaffold Event when:datetime until:datetime title:string description:string

Next, migrate your DB as-is: rake db:migrate

Fire up the server: script/server

Now, you were partially correct in that you can't execute GET on the create action - unless you go all the way:

http://localhost:3000/events/create/999?event[when]=2009-12-25%2005:15:15

Will create a new event that "starts" (the when variable) on December 25, 2009 at 05:15:15.

Can anyone tell me if I'm failing to use some other "rails magic", or if scaffolding is just plain intended not to worry about this, leaving it to the programmer?

4

Hi Colin,

Thanks for the reply. I just did try this and unfortunately, my original assessment was correct: vulnerable to XSS.

Here's how to repeat: Make a new rails app, then generate a scaffold: script/generate scaffold Event when:datetime until:datetime title:string description:string

Next, migrate your DB as-is: rake db:migrate

Fire up the server: script/server

Now, you were partially correct in that you can't execute GET on the create action - unless you go all the way:

http://localhost:3000/events/create/999?event[when]=2009-12-25%2005:15:15

Will create a new event that "starts" (the when variable) on December 25, 2009 at 05:15:15.

Can anyone tell me if I'm failing to use some other "rails magic", or if scaffolding is just plain intended not to worry about this, leaving it to the programmer?

I was wrong when I said this would not work unless you have added a route of your own. However if you look at the end of routes.rb you will see:

  # Install the default routes as the lowest priority.   # Note: These default routes make all actions in every controller accessible via GET requests. You should   # consider removing or commenting them out if you're using named routes and resources.   map.connect ':controller/:action/:id'   map.connect ':controller/:action/:id.:format'

I am just off to double check that I have removed them on all of my systems. Also to check that in my tests I have checks to make sure that GETs fail where appropriate.

Colin

5

Yep, I missed the standard routing at the end of routes.rb. However, if I remove that, it would definitely cause problems with my existing (non-scaffolded) controllers. But it's probably still a great way to go with a fresh app.

And yes, I always test with multiple get/posts on actions that can modify data on the server. I'm just curious as to what extent scaffolding is supposed to solve the "problem" of getting off and going quickly :slight_smile:

Thanks for your input!