I have noticed on several instances that when exporting a MARC framework it is encoded in ISO-8859-1, which should be UTF-8, the expected encoding for importing frameworks. Also, when chossing SQL as the export format, single quotes are escaped with backslashes, but these backslashes are not removed when importing.
Created attachment 22740 [details] [review] Bug 9114: import/export routines should use UTF-8 Currently both the import_export_framework.pl script and the associated libs output data with Perl's default encoding, ISO-8859. This patch properly sets the binmode to UTF-8 for the CGI script, and makes the C4::ImportExportFramework routines apply the ":encoding(UTF-8)" layer to file descriptors. To test: Export step test - Use some non-ASCII character(s) in some field description in a chosen framework. - Export the framework at Administration > MARC frameworks - Run this to check the file is ISO-8859 encoded: $ file export_XXX.csv export_XXX.csv: ISO-8859 text, with very long lines (Note: try SQL and other output formats too. But not ODS) - Apply the patch - Export the framework again (change the name), and test encoding: $ file export_XXX_2.csv export_XXX_2.csv: UTF-8 Unicode text Import step test I assume you have two files, export_XXX.csv (ISO-8859 encoded) and export_XXX_2.csv (XXX will depend on your framework's code) - Reset your testing branch to master - Import export_XXX.csv - The string with non-ASCII chars is truncated at the first non-ASCII char's position (Note: this is the current behaviour). - Import export_XXX_2.csv - The non-ASCII chars are broken, the logs show errors on non-UNICODE chars. (Note: even thou UTF-8 is the expected encoding it is treated as ISO-8859). - Apply the patch - Import the good (UTF-8 as expected) file and check everything worked as expected. Regards To+ Sponsored-by: Universidad Nacional de Cordoba
Hello WITHOUT your patch, when I run $ file ~/Téléchargements/export_.csv I get: /home/msaby/Téléchargements/export_.csv: UTF-8 Unicode text I also put some chinese characters in the name of a field, and when I import the saved framework, I can see those characters in the new framework in Koha. So, can you confirm the issue, and your test plan? Mathieu
Tomas: Thank you for your patch. I have a comment however :) You use binmode on STDOUT in the module, but you print to a file handle. So that should not be enough. I would suggest to replace the binmode by adding the encoding to the output file handles. Note that I also had a wide character warning in my log on admin/import_export_framework. You probably need to touch code there as well? I tested with a Chinese character; that went well (export and re-import). Since you actually leave it to Perl to decide the encoding, Perl made the right choice here. But with some Latin characters only, export and re-import made them disappear.. Failed QA
(In reply to M. de Rooy from comment #3) > Tomas: Thank you for your patch. I have a comment however :) You use binmode > on STDOUT in the module, but you print to a file handle. So that should not > be enough. I would suggest to replace the binmode by adding the encoding to > the output file handles. Note that I also had a wide character warning in my > log on admin/import_export_framework. You probably need to touch code there > as well? > I tested with a Chinese character; that went well (export and re-import). > Since you actually leave it to Perl to decide the encoding, Perl made the > right choice here. But with some Latin characters only, export and re-import > made them disappear.. > > Failed QA Some confusion: The binmode is not in the module, but printing to the file handle is. So please correct.
Even more confusion: Please forget the former two comments. I am retesting. Something went wrong. The warning on the wide char is not from your patch after all. Back to Needs Signoff
Created attachment 24450 [details] [review] Bug 9114: import/export routines should use UTF-8 Currently both the import_export_framework.pl script and the associated libs output data with Perl's default encoding, ISO-8859. This patch properly sets the binmode to UTF-8 for the CGI script, and makes the C4::ImportExportFramework routines apply the ":encoding(UTF-8)" layer to file descriptors. To test: Export step test - Use some non-ASCII character(s) in some field description in a chosen framework. - Export the framework at Administration > MARC frameworks - Run this to check the file is ISO-8859 encoded: $ file export_XXX.csv export_XXX.csv: ISO-8859 text, with very long lines (Note: try SQL and other output formats too. But not ODS) - Apply the patch - Export the framework again (change the name), and test encoding: $ file export_XXX_2.csv export_XXX_2.csv: UTF-8 Unicode text Import step test I assume you have two files, export_XXX.csv (ISO-8859 encoded) and export_XXX_2.csv (XXX will depend on your framework's code) - Reset your testing branch to master - Import export_XXX.csv - The string with non-ASCII chars is truncated at the first non-ASCII char's position (Note: this is the current behaviour). - Import export_XXX_2.csv - The non-ASCII chars are broken, the logs show errors on non-UNICODE chars. (Note: even thou UTF-8 is the expected encoding it is treated as ISO-8859). - Apply the patch - Import the good (UTF-8 as expected) file and check everything worked as expected. Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested export and re-import with CSV containing Latin chars and containing Chinese chars.
Created attachment 24514 [details] [review] Bug 9114: import/export routines should use UTF-8 Currently both the import_export_framework.pl script and the associated libs output data with Perl's default encoding, ISO-8859. This patch properly sets the binmode to UTF-8 for the CGI script, and makes the C4::ImportExportFramework routines apply the ":encoding(UTF-8)" layer to file descriptors. To test: Export step test - Use some non-ASCII character(s) in some field description in a chosen framework. - Export the framework at Administration > MARC frameworks - Run this to check the file is ISO-8859 encoded: $ file export_XXX.csv export_XXX.csv: ISO-8859 text, with very long lines (Note: try SQL and other output formats too. But not ODS) - Apply the patch - Export the framework again (change the name), and test encoding: $ file export_XXX_2.csv export_XXX_2.csv: UTF-8 Unicode text Import step test I assume you have two files, export_XXX.csv (ISO-8859 encoded) and export_XXX_2.csv (XXX will depend on your framework's code) - Reset your testing branch to master - Import export_XXX.csv - The string with non-ASCII chars is truncated at the first non-ASCII char's position (Note: this is the current behaviour). - Import export_XXX_2.csv - The non-ASCII chars are broken, the logs show errors on non-UNICODE chars. (Note: even thou UTF-8 is the expected encoding it is treated as ISO-8859). - Apply the patch - Import the good (UTF-8 as expected) file and check everything worked as expected. Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested export and re-import with CSV containing Latin chars and containing Chinese chars. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch breaks the ODS format. ODS is a fancy ZIP file, and therefore needs to be written in binary mode. I also observed that strings got double-encoded upon export to the Excel XML format. Marking failed QA for the regressions. For folks testing future version of this patch, please test importing and exporting all four formats. I also want to highlight a part of Marcel's comment 3: to reproduce the bug, you actually don't want Chinese characters or the like in the framework you're exporting -- you want Western European diacritical characters that are to be found in the ISO-8859-1 character set. As Marcel observed, if you've got non-Latin1 characters in your string, Perl is much more likely to emit UTF8. Of course, you should test with non-Latin1 characters too, but they're less likely to trigger the bug.
(In reply to Galen Charlton from comment #8) > Marking failed QA for the regressions. thx for scrutiny++
(In reply to Galen Charlton from comment #8) > This patch breaks the ODS format. ODS is a fancy ZIP file, and therefore > needs to be written in binary mode. I also observed that strings got > double-encoded upon export to the Excel XML format. The patch broke the ODS format, I've fixed easily. The Excel XML format is broken on master actually. I'll try to fix it too. Thanks for the insight Galen.
Created attachment 24630 [details] [review] Bug 9114: Make frameworks import/export routines correctly use UTF-8 Currently both the import_export_framework.pl script outputs data with Perl's default encoding, ISO-8859. This patch properly sets the binmode to UTF-8 when exporting SQL and CSV files using the PerlIO layer (":encoding(UTF-8)") for STDOUT. To test: Export step test - Use some ASCII character(s) with DIACRITICS in some field description in a chosen framework. - Export the framework at Administration > MARC frameworks - Run this to check the file is ISO-8859 encoded: $ file export_XXX.csv export_XXX.csv: ISO-8859 text, with very long lines (Note: try SQL and other output formats too. But not ODS) - Apply the patch - Export the framework again (change the name), and test encoding: $ file export_XXX_2.csv export_XXX_2.csv: UTF-8 Unicode text Import step test I assume you have two files, export_XXX.csv (ISO-8859 encoded) and export_XXX_2.csv (XXX will depend on your framework's code) - Reset your testing branch to master - Import export_XXX.csv - The string with non-ASCII chars is truncated at the first non-ASCII char's position (Note: this is the current behaviour). - Import export_XXX_2.csv - The non-ASCII chars are broken, the logs show errors on non-UNICODE chars. (Note: even thou UTF-8 is the expected encoding it is treated as ISO-8859). - Apply the patch - Import the good (UTF-8 as expected) file and check everything worked as expected. No double encoding should occur with either combination of formats. Regards To+ Sponsored-by: Universidad Nacional de Cordoba
Created attachment 26285 [details] [review] Bug 9114: Make frameworks import/export routines correctly use UTF-8 Currently both the import_export_framework.pl script outputs data with Perl's default encoding, ISO-8859. This patch properly sets the binmode to UTF-8 when exporting SQL and CSV files using the PerlIO layer (":encoding(UTF-8)") for STDOUT. To test: Export step test - Use some ASCII character(s) with DIACRITICS in some field description in a chosen framework. - Export the framework at Administration > MARC frameworks - Run this to check the file is ISO-8859 encoded: $ file export_XXX.csv export_XXX.csv: ISO-8859 text, with very long lines (Note: try SQL and other output formats too. But not ODS) - Apply the patch - Export the framework again (change the name), and test encoding: $ file export_XXX_2.csv export_XXX_2.csv: UTF-8 Unicode text Import step test I assume you have two files, export_XXX.csv (ISO-8859 encoded) and export_XXX_2.csv (XXX will depend on your framework's code) - Reset your testing branch to master - Import export_XXX.csv - The string with non-ASCII chars is truncated at the first non-ASCII char's position (Note: this is the current behaviour). - Import export_XXX_2.csv - The non-ASCII chars are broken, the logs show errors on non-UNICODE chars. (Note: even thou UTF-8 is the expected encoding it is treated as ISO-8859). - Apply the patch - Import the good (UTF-8 as expected) file and check everything worked as expected. No double encoding should occur with either combination of formats. Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Magnus Enger <digitalutvikling@gmail.com> I put some Norwegian and accented letters in a fremawork to test. Before the patch, the exported CSV came out as ISO-8859, after the patch it came out as UTF-8. ODS and XML (viewed in LibreOffice) both looked good, before and after the patch. Importing the ISO-8859 CSV cut off the strings at the first non-ASCII char. Importing the UTF-8 CSV worked as epected.
Comment on attachment 26285 [details] [review] Bug 9114: Make frameworks import/export routines correctly use UTF-8 Review of attachment 26285 [details] [review]: ----------------------------------------------------------------- ::: admin/import_export_framework.pl @@ +56,5 @@ > ExportFramework($frameworkcode, \$strXml, $format); > + > + # Correctly set the encoding to output plain text in UTF-8 > + if ( $format eq 'csv' || > + $format eq 'sql' ) { Note that 'sql' was removed as an export option in security bug 11666.
(In reply to Galen Charlton from comment #13) > Comment on attachment 26285 [details] [review] [review] > Bug 9114: Make frameworks import/export routines correctly use UTF-8 > > Review of attachment 26285 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: admin/import_export_framework.pl > @@ +56,5 @@ > > ExportFramework($frameworkcode, \$strXml, $format); > > + > > + # Correctly set the encoding to output plain text in UTF-8 > > + if ( $format eq 'csv' || > > + $format eq 'sql' ) { > > Note that 'sql' was removed as an export option in security bug 11666. I'll attach a new and correct patch.
Created attachment 26320 [details] [review] Bug 9114: Make frameworks import/export routines correctly use UTF-8 Currently both the import_export_framework.pl script outputs data with Perl's default encoding, ISO-8859. This patch properly sets the binmode to UTF-8 when exporting SQL and CSV files using the PerlIO layer (":encoding(UTF-8)") for STDOUT. To test: Export step test - Use some ASCII character(s) with DIACRITICS in some field description in a chosen framework. - Export the framework at Administration > MARC frameworks - Run this to check the file is ISO-8859 encoded: $ file export_XXX.csv export_XXX.csv: ISO-8859 text, with very long lines (Note: try SQL and other output formats too. But not ODS) - Apply the patch - Export the framework again (change the name), and test encoding: $ file export_XXX_2.csv export_XXX_2.csv: UTF-8 Unicode text Import step test I assume you have two files, export_XXX.csv (ISO-8859 encoded) and export_XXX_2.csv (XXX will depend on your framework's code) - Reset your testing branch to master - Import export_XXX.csv - The string with non-ASCII chars is truncated at the first non-ASCII char's position (Note: this is the current behaviour). - Import export_XXX_2.csv - The non-ASCII chars are broken, the logs show errors on non-UNICODE chars. (Note: even thou UTF-8 is the expected encoding it is treated as ISO-8859). - Apply the patch - Import the good (UTF-8 as expected) file and check everything worked as expected. No double encoding should occur with either combination of formats. Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Magnus Enger <digitalutvikling@gmail.com> I put some Norwegian and accented letters in a fremawork to test. Before the patch, the exported CSV came out as ISO-8859, after the patch it came out as UTF-8. ODS and XML (viewed in LibreOffice) both looked good, before and after the patch. Importing the ISO-8859 CSV cut off the strings at the first non-ASCII char. Importing the UTF-8 CSV worked as epected.
Created attachment 26350 [details] [review] [PASSED QA] Bug 9114: Make frameworks import/export routines correctly use UTF-8 Currently both the import_export_framework.pl script outputs data with Perl's default encoding, ISO-8859. This patch properly sets the binmode to UTF-8 when exporting SQL and CSV files using the PerlIO layer (":encoding(UTF-8)") for STDOUT. To test: Export step test - Use some ASCII character(s) with DIACRITICS in some field description in a chosen framework. - Export the framework at Administration > MARC frameworks - Run this to check the file is ISO-8859 encoded: $ file export_XXX.csv export_XXX.csv: ISO-8859 text, with very long lines (Note: try SQL and other output formats too. But not ODS) - Apply the patch - Export the framework again (change the name), and test encoding: $ file export_XXX_2.csv export_XXX_2.csv: UTF-8 Unicode text Import step test I assume you have two files, export_XXX.csv (ISO-8859 encoded) and export_XXX_2.csv (XXX will depend on your framework's code) - Reset your testing branch to master - Import export_XXX.csv - The string with non-ASCII chars is truncated at the first non-ASCII char's position (Note: this is the current behaviour). - Import export_XXX_2.csv - The non-ASCII chars are broken, the logs show errors on non-UNICODE chars. (Note: even thou UTF-8 is the expected encoding it is treated as ISO-8859). - Apply the patch - Import the good (UTF-8 as expected) file and check everything worked as expected. No double encoding should occur with either combination of formats. Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Magnus Enger <digitalutvikling@gmail.com> I put some Norwegian and accented letters in a fremawork to test. Before the patch, the exported CSV came out as ISO-8859, after the patch it came out as UTF-8. ODS and XML (viewed in LibreOffice) both looked good, before and after the patch. Importing the ISO-8859 CSV cut off the strings at the first non-ASCII char. Importing the UTF-8 CSV worked as epected. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works as expected, passes tests and QA script.
Pushed to master. Thanks, Tomás!
Pushed to 3.14.x, will be in 3.14.08
Pushed to 3.12.x, will be in 3.12.13