bugGNU Octave - Bugs: bug #57341, iso-8859-1 encoding triggers error...

 
 

bug #57341: iso-8859-1 encoding triggers error in get_first_help_sentence.m

Submitter:  Juan Pablo Carbajal <juanpi>
Submitted:  Mon 02 Dec 2019 09:18:35 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  juanpi Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 08 Dec 2019 04:08:56 PM UTC, comment #13: 

I added a FIXME note about the missing file name and pushed the patch here:
http://hg.savannah.gnu.org/hgweb/octave/rev/3f5026fd8da8

Marking as ready for test.

Markus Mützel <mmuetzel>
Group administrator
Thu 05 Dec 2019 07:01:14 PM UTC, comment #12: 

The attached patch emits a warning when invalid UTF-8 byte sequences have been replaced:

>> txt = get_first_help_sentence('foo_iso_8859_1')

warning: Invalid UTF-8 byte sequences have been replaced.
warning: called from
    get_first_help_sentence at line 61 column 23

txt =  This characters will trigger the error: � � �


I'd like to include the file name. But I don't know if it possible to get that from the FILE pointer.

(file #48004)

Markus Mützel <mmuetzel>
Group administrator
Thu 05 Dec 2019 02:46:32 PM UTC, comment #11: 

Great! So, would you add the warning to your patch, or you would postponed it to the caller? In the latter case then we need an indication that chanracters were replaced.

Juan Pablo Carbajal <juanpi>
Group Member
Thu 05 Dec 2019 02:44:08 PM UTC, comment #10: 

I don't know what gedit is doing. I am currently on Windows. And using Notepad++, I can see the encoding that is used for displaying the current file in the bottom bar (most files I use say "ANSI" or "UTF-8"). It looks like some kind of heuristics might be at work here as well.

Editors have the "simple" task to "just" display and save text. So re-encoding is probably not necessary. They probably just display the text in any given encoding.
In contrast, strings in Octave can be passed to a number of different libraries (all potentially having different requirements on the encoding of strings). To avoid having to track the encoding from and to each interface, we opted to try and convert at the interfaces and consistently use UTF-8 inside Octave.

I agree that we should issue a warning if characters had to be replaced.

Markus Mützel <mmuetzel>
Group administrator
Thu 05 Dec 2019 02:17:40 PM UTC, comment #9: 

What about issuing a warning when the encoding is unkown? This way I would have got the headsup!

Juan Pablo Carbajal <juanpi>
Group Member
Thu 05 Dec 2019 02:16:24 PM UTC, comment #8: 

Indeed it is heuristics, details are here

https://linux.die.net/man/1/file

Then I guess your solution was correct. It also doesn't sound reasonable to me to assume unknown encoding means iso-8859-1

However, if this fix was present when I was working with the iso-8859-1 encoded file:

1. I wouldn't have got an error

2. The docstring will not be readable

So in my situation it seems that the error is better than the patch (maybe a more clear error message), because I could then go an identify the files with the funny encoding, and get the right docstrings.
This patch would have silently avoided the error but produced the wrong docstrings. The latter is undesirable.

How does, e.g. gedit, handles this encoding issue? Both files are correctly shown in gedit when I open them. Are they also re-encodig?

Juan Pablo Carbajal <juanpi>
Group Member
Thu 05 Dec 2019 01:42:12 PM UTC, comment #7: 

I felt like we shouldn't treat ISO-8859-1 any more special than any other encoding. That's why I opted to replace invalid UTF-8 byte sequences with the replacement character if the mfile encoding is set to "UTF-8".
I don't know how "file -i" determines the encoding of a text file. Maybe it uses some sort of heuristics (difficult and error prone), or it defaults to ISO-8859-1 (Latin-1) if the file contains invalid UTF? The latter is more likely because the character sequence in your example file doesn't resemble a word in any Western language.
It would be no problem to fall back to ISO-8859-1 by default. Is this what we want to do, though? That would be slightly US/Western centralist, imho.
But it's ok with me if we agreed on that.

It's already possible to correctly read the file prior to the patch by using the code snippet from the verbatim block in comment #3.

Markus Mützel <mmuetzel>
Group administrator
Thu 05 Dec 2019 01:25:11 PM UTC, comment #6: 

But cannot the encoding be obtained from the file itself, e.g. the information provided by

file -i filename.m

?

I would say it is better to use that info first rather than falling back to the unknown situation.


file -i

gave the right encoding information.

Juan Pablo Carbajal <juanpi>
Group Member
Thu 05 Dec 2019 01:23:46 PM UTC, comment #5: 

But cannot the encoding be obtained from the file itself, e.g. the information provided by +verbatim+file -i filename.m-verbatim-?
I would say it is better to use that info first rather than falling back to the unknown situation.
For example I hit the bug with a package with more than 200 files, where 4 of them had encodings that Octave couldn't handle, but +verbatim+file -i-verbatim- gave the right encoding information.

Juan Pablo Carbajal <juanpi>
Group Member
Thu 05 Dec 2019 01:16:20 PM UTC, comment #4: 

oh, I see actually that this is what you patch is trying to do. Why then you do not get the symbols right?

If I change the encoding of the file using iconv the symbols appear just fine, check the attached file, which is

foo_iso_8859_1.m

converted to uft-8 using


iconv -f ISO-8859-1 -t UTF-8//TRANSLIT foo_iso_8859_1.m  > foo_conv.m


I get then


> help foo_conv
warning: function name 'foo_iso_8859_1' does not agree with function filename '/home/juanpi/Downloads/foo_conv.m'
warning: called from
    help at line 95 column 20
'foo_conv' is a function from the file /home/juanpi/Downloads/foo_conv.m

 This characters will trigger the error: ü ° ±



(file #48000)

Juan Pablo Carbajal <juanpi>
Group Member
Thu 05 Dec 2019 01:15:09 PM UTC, comment #3: 

If you know the encoding before reading the file, set the .m file encoding accordingly:

__mfile_encoding__ ("iso-8859-1");
clear functions  # necessary if the file was already parsed
txt = get_first_help_sentence('foo_iso_8859_1')


The proposed change is to avoid the error if the encoding is unknown or not set correctly.

Markus Mützel <mmuetzel>
Group administrator
Thu 05 Dec 2019 01:06:27 PM UTC, comment #2: 

Is this the solution?
The symbols do render well if the encoding is changed, for example using iconv
So why not changing the encoding to the octave standard when the text is read? (not the encoding of the file)

Juan Pablo Carbajal <juanpi>
Group Member
Wed 04 Dec 2019 09:52:00 PM UTC, comment #1: 

The attached patch (very similar to the one for bug #57107) replaces invalid UTF-8 with the replacement character before processing the file.

The output with the patch is:

>> txt = get_first_help_sentence('foo_iso_8859_1')
txt =  This characters will trigger the error: � � �



(file #47999)

Markus Mützel <mmuetzel>
Group administrator
Mon 02 Dec 2019 09:18:35 AM UTC, original submission:  


The two attached files contain the same symbols but with different encodings. The iso-8859-1 fails:


octave-gui:1> txt = get_first_help_sentence('foo_utf_8')
txt =  This characters will trigger the error: ü ° ±

octave-gui:2> txt = get_first_help_sentence('foo_iso_8859_1')
error: regexp: the input string is invalid UTF-8
error: called from
    get_first_help_sentence>first_sentence_plain_text at line 89 column 14
    get_first_help_sentence at line 66 column 22


Juan Pablo Carbajal <juanpi>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #48004:  bug57341_input_invalid_utf8_v2.patch added by mmuetzel (7KiB - application/octet-stream)
file #48000:  foo_conv.m added by juanpi (96B - text/x-objcsrc)
file #47999:  bug57341_input_invalid_utf8.patch added by mmuetzel (7KiB - application/octet-stream)
file #47963:  foo_iso_8859_1.m added by juanpi (93B - text/x-objcsrc)
file #47964:  foo_utf_8.m added by juanpi (91B - text/x-objcsrc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by mmuetzel (Updated the item)
  • -email is unavailable- added by juanpi (Submitted the item)
  • -email is unavailable- added by juanpi
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-01-04 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2019-12-08 mmuetzel StatusPatch Submitted Ready For Test
    2019-12-05 mmuetzel Attached File- Added bug57341_input_invalid_utf8_v2.patch, #48004
    2019-12-05 juanpi Attached File- Added foo_conv.m, #48000
    2019-12-04 mmuetzel Attached File- Added bug57341_input_invalid_utf8.patch, #47999
        StatusNone Patch Submitted
    2019-12-02 juanpi Attached File- Added foo_iso_8859_1.m, #47963
        Attached File- Added foo_utf_8.m, #47964
        Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code