rubyforgood / casa

Volunteer management system for nonprofit CASA, which serves foster youth in counties across America.
https://casavolunteertracking.org/
MIT License
310 stars 473 forks source link

[BUG] Cookie Overflow when trying to import CasaCase CSV #2855

Open aedwardg opened 3 years ago

aedwardg commented 3 years ago

Impacted User Types

Environment

Production (not verified in other environments)

Current Behavior

CasaCase CSV import fails when CSV only contains case numbers. Needs to be confirmed if this happens with a complete CSV as well.

Bugsnag error:

Event in production from casa in imports#create (details)
Unhandled error
ActionDispatch::Cookies::CookieOverflow: ActionDispatch::Cookies::CookieOverflow
Location
vendor/bundle/ruby/3.0.0/gems/actionpack-6.1.4.1/lib/action_dispatch/middleware/cookies.rb:678 - commit

Slack Link https://rubyforgood.slack.com/archives/C010Z8RK8E6/p1635385879085500

Expected Behavior

When uploading a CSV with CaseContact required fields, there should be no error and upload should be successful.

How to Replicate

    • Log in as an admin.
    • Click on "System Imports" in the left sidebar menu.
    • Click on "Import Cases" tab
    • Click "Choose File" and choose a file with the correct headers, and only one column (case number).
# sample_broken_file.csv
display_name,email,phone_number
2,
2,
2,
.... more incorrect columns

QA Login Details:

Link to QA site

Login Emails:

password for all users: 123456

Questions? Join Slack!

We highly recommend that you join us in slack https://rubyforgood.herokuapp.com/ #casa channel to ask questions quickly and hear about office hours (currently Wednesday 6-8pm Pacific), stakeholder news, and upcoming new issues.

FireLemons commented 3 years ago

I think one of the cookies is trying to store the CSV contents but the cookies can only have a max size of 4096 bytes or under 4096 characters image https://stackoverflow.com/a/9474262

FireLemons commented 3 years ago

Ok I think it starts at line 21 of app/controllers/imports_controller.rb where an exported_rows message is being stored in the session. image

And the exported_rows message comes from app/lib/importers/file_importer.rb which concatenates to the message every time there's a failed row. image

So if there's too many row failures the cookie goes over the max size of 4KB

shkm commented 2 years ago

Simple fix:

  1. After importing, if there are failed rows, immediately store a CSV somewhere identified by the current user ID
  2. On import, destroy any existing failed CSV for this user
  3. On #download_failed, serve said CSV.

Thoughts? I guess it'd be simplest to use activeresource on the current user for this.

Alternatively:

github-actions[bot] commented 2 days ago

This issue has been open without changes for a long time! What's up?