search

instructure / canvas-lms

The open LMS by Instructure, Inc.
https://github.com/instructure/canvas-lms/wiki
GNU Affero General Public License v3.0
5.7k stars 2.51k forks source link

Gradebook CSV import treats escaped commas as field separators. #1394

Open eritain opened 5 years ago

eritain commented 5 years ago

Summary:

CSV ("comma-separated values") can and does represent values that contain commas. It uses ASCII double quotation marks around the value to indicate that the comma is part of the value, not a field separator. For example, a number may be formatted "1,234.56"

Gradebook import ignores the quotation marks, which spreads the value across multiple fields that it's not supposed to be in.

Steps to reproduce:

  1. Put a score over 999 in Gradebook, or several scores such that a total exceeds 999. On another row, keep all numbers below 999.
  2. Export CSV.
  3. Re-import CSV.

Expected behavior:

Grades are unchanged.

Actual behavior:

Values exceeding 999 are incorrectly spread across multiple cells. All subsequent values on the same row land in the wrong column.

Additional notes:

Gradebook version history is good, but a rollback function would be better. Much, much better.

There are widely used, tested, freely available libraries that parse CSV correctly.

Fuzz testing would probably have caught this bug.

stale[bot] commented 3 years ago

Thanks for contributing to this issue. As it has been 2 years since the last activity, we are automatically closing the issue in 30 days. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please respond before the issue is closed, or post a message on the mailing list. We'll gladly take a look again!