search

openwebwork / webwork2

Course management front end for WeBWorK
http://webwork.maa.org/wiki/Main_Page
Other
141 stars 164 forks source link

Fix trim_spaces UTF-8 without BOM - Update Utils.pm #2304

Closed tm-lcarvalho closed 6 months ago

tm-lcarvalho commented 6 months ago

Always when we have a csv in UTF-8 with BOM we have a error "Wide character in crypt" error message when passwords with certain "wide" characters

drgrice1 commented 6 months ago

It is debatable if this is the correct thing to do. The issue is that a UTF-8 encoded csv file should NOT have a BOM in it. UTF-8 encoding does not support BOMs. So this is fixing invalid files that you are working with.

Furthermore, this should not be done in the trim_spaces method of the Utils.pm file. That method is used for other things than just passwords.

Do you have a test csv file and method to demonstrate the issue that you are trying to fix? Please provide an example that does this.

pstaabp commented 6 months ago

This should be targeted at the develop branch instead.

tm-lcarvalho commented 6 months ago

Thanks @drgrice1! I agree with you, but I don't have control over where the files are created to be imported. Normally they use a CSV file and then use Excel to export it in UTF-8(the professors who own this course follow this process). However, this specific file came with a BOM. We use this list to import students into WebWork,, and whenever there's a BOM, we encounter the 'Wide character in crypt' error. I have attached two files, one with a BOM and one without a BOM. Our workflow involves uploading the list to WebWork and import the list: WeBWorK -> TEMP Course Tests -> Instructor Tools -> Classlist Editor -> Import users from what file> -> -> Import

╰─$ hexdump -n 3 -C StudentsList_course_csv_utf-8_w_BOM.csv                                                                                                                                                                                 130 ↵
00000000  ef bb bf                                          |...|
00000003
╰─$ hexdump -n 3 -C StudentsList_course_csv_utf-8_withot_BOM.csv
00000000  32 33 37                                          |237|
00000003

StudentsList_course_csv_utf-8_w_BOM.csv StudentsList_course_csv_utf-8_withot_BOM.csv

tm-lcarvalho commented 6 months ago

@pstaabp done

drgrice1 commented 6 months ago

Unfortunately, it is not so easy as to just retarget to the develop branch (as you can see by looking at what Github now shows has changed). You will either need to interactively rebase onto the develop branch, drop all commits that this adds from main, and the force push, or just start over from the develop branch and recreate this pull request.

tm-lcarvalho commented 6 months ago

@drgrice1 don't worry, I do that ;-)