Daniel Fone

Ruby/Rails Engineer

Devise causing a CookieOverflow error

tl;dr If you're going to submit forms via GET, keep your parameter names short!

Today I came across an exception that was caused by a collision of two uncommon circumstances:

How It Happens

  1. A user who signed in some time ago has the form open. They submit the form which GETs a very long url from the server.
  2. Devise attempts to authenticate the user and realises that their session has expired.
  3. Devise attempts to redirect the user to the sign in form, but first it stores the url in the session as user_return_to.
  4. The url is too long to fit in the session and a ActionDispatch::Cookies::CookieOverflow exception is raised.

Reproducible Test Case

Fortunately, it’s very easy to replicate the problem in tests. Anytime an unauthenticated user requests a url approaching ActionDispatch::Cookies::MAX_COOKIE_SIZE, Devise will attempt to store the the url and cause the exception to be raised.

  # /spec/requests/large_request_spec.rb
  require 'spec_helper'

  RSpec.describe 'A very large request', type: :request do
    it 'should not overflow cookies' do
      get '/', foo: 'x' * ActionDispatch::Cookies::MAX_COOKIE_SIZE
      expect(response).to redirect_to '/users/sign_in'
    end
  end

This reliably produces

1) A very large request should not overflow cookies
   Failure/Error: get '/', foo: 'bar' * 1000
   ActionDispatch::Cookies::CookieOverflow:
     ActionDispatch::Cookies::CookieOverflow

Solutions

There were a couple of approaches that I didn’t want to pursue.

Ultimately, I wanted to solve this at the Devise level. The problematic behaviour is defined in the store_location module, which is included into Devise’s failure app.

I wrote a simple initializer to monkey patch the method and prevent it storing excessively long urls in the session.

The downside to this approach is that the user loses their place in the form, but it is far better than an unhandled exception when submitting. I’ve submitted a pull request to the Devise project, but to be honest this is such an edge case that it may not warrant it. We’ll see what happens.

comments powered by Disqus