Prevent CookieOverflow errors for large urls #3347

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
@danielfone

This occurs if an unauthenticated user requests an excessively long url. This was a problem I encountered with large forms that were submitted via GET and user sessions timing out. Granted, it's an edge case, but the fix is quite simple.

Thoughts?

Prevent CookieOverflow errors for large urls
This occurs if an unauthenticated user requests an excessively long url.
@danielfone

This comment has been minimized.

Show comment
Hide comment
@danielfone

danielfone Nov 28, 2014

The MAX_LOCATION_SIZE could also be calculated from ActionDispatch::Cookies::MAX_COOKIE_SIZE to make it less magic.

The MAX_LOCATION_SIZE could also be calculated from ActionDispatch::Cookies::MAX_COOKIE_SIZE to make it less magic.

@danielfone

This comment has been minimized.

Show comment
Hide comment
@danielfone

danielfone Nov 28, 2014

Oops! I can squash the commits if needed.

Oops! I can squash the commits if needed.

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Nov 28, 2014

Member

I am not sure if we merge this. The biggest issue is that we transform a failure into a silent behaviour. I can see people opening issues in a short time period saying "Devise does not redirect to URLs being submitted by a form". Plus this only matters to the cookie store.

With this said, you could possibly override this in your application, but it doesn't make sense for Devise itself to ship with such fix. Thanks for the patch!

Member

josevalim commented Nov 28, 2014

I am not sure if we merge this. The biggest issue is that we transform a failure into a silent behaviour. I can see people opening issues in a short time period saying "Devise does not redirect to URLs being submitted by a form". Plus this only matters to the cookie store.

With this said, you could possibly override this in your application, but it doesn't make sense for Devise itself to ship with such fix. Thanks for the patch!

@josevalim josevalim closed this Nov 28, 2014

@danielfone

This comment has been minimized.

Show comment
Hide comment
@danielfone

danielfone Nov 28, 2014

@josevalim thanks and fair enough.

If anyone ends up here via google, I've posted an example monkey patch here https://gist.github.com/danielfone/c8ab593c326a8052651c

@josevalim thanks and fair enough.

If anyone ends up here via google, I've posted an example monkey patch here https://gist.github.com/danielfone/c8ab593c326a8052651c

@danielfone danielfone deleted the danielfone:cookie-overflow branch Dec 6, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment