bugGNU Octave - Bugs: bug #35910, Incorrect regex matching of...

 
 

bug #35910: Incorrect regex matching of multi-byte UTF-8 characters

Submitted by:  Burkart Lingner <burkart>
Submitted on:  Tue 20 Mar 2012 04:29:25 PM UTC  
 
Category:  Libraries Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Patch Submitted Assigned to:  Rik <rik5>
Originator Name:  Open/Closed:  Open
Release:  dev Operating System:  Any

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Tue 22 Oct 2019 05:46:06 PM UTC, comment #30: 

I updated the wording of the error message.  I also downloaded and compiled a version of libpcre without UTF support.  The configure test correctly errors out when using this library.  I checked in everything here https://hg.savannah.gnu.org/hgweb/octave/rev/d7b18d44ae4e.

Anything else left to do before closing this report?

Rik <rik5>
Project AdministratorIn charge of this item.
Tue 22 Oct 2019 07:35:55 AM UTC, comment #29: 

Yes, that's right: The patch from comment #25 (or something similar) still needs to be checked in.
But some additional testing wouldn't hurt. I am not too familiar with the autoconf macros. So I don't know whether the changes are correct.
I could try and test whether it breaks the Windows cross-builds (possibly later tonight).
It would also be good if someone could check whether running the test really fails for a PCRE that is compiled without UTF-8 support.

Markus Mützel <mmuetzel>
Project Member
Mon 21 Oct 2019 10:27:51 PM UTC, comment #28: 

I made a few changes to the NEWS file and then checked in the second documentation patch here https://hg.savannah.gnu.org/hgweb/octave/rev/7dc31256c5e4.

Does the patch from comment #25 also need to be checked in?

Rik <rik5>
Project AdministratorIn charge of this item.
Fri 18 Oct 2019 05:09:05 PM UTC, comment #27: 

Thanks, Guillaume, for the feedback. The attached patch is a little more wordy in the announcement in the NEWS file.
Could someone else please polish the language?

(file #47712)

Markus Mützel <mmuetzel>
Project Member
Fri 18 Oct 2019 08:43:24 AM UTC, comment #26: 

Thanks, Markus. Do you think it would be useful to mention native2unicode in the NEWS file as I might not be alone in having code 'suddenly' breaking with the new version?

Guillaume <gyom>
Thu 17 Oct 2019 08:05:07 PM UTC, comment #25: 

And another patch that adds a autoconf test whether PCRE was compiled with --enable-utf.
I could only test with a PCRE that passes. Testing would be very welcome.

(file #47709)

Markus Mützel <mmuetzel>
Project Member
Thu 17 Oct 2019 07:58:51 PM UTC, comment #24: 

The attached patch adds some documentation that the regexp* functions need UTF-8 encoded input.
It also adds info about this change to the NEWS file.

(file #47708)

Markus Mützel <mmuetzel>
Project Member
Thu 17 Oct 2019 03:14:12 PM UTC, comment #23: 

This is a general difference between Octave and Matlab not only affecting this function:
Matlab seems to use UCS-2 to encode characters (a subset of UTF-16). Octave uses UTF-8 (in more and more places - maybe consistently one day).

Maybe you could try the following?

regexprep (native2unicode (181, 'latin1'), 'u', 'micro')

The characters 0-255 of Unicode "incidentally" equate to Latin-1. That's why it happens to work in Matlab.

Markus Mützel <mmuetzel>
Project Member
Thu 17 Oct 2019 02:56:50 PM UTC, comment #22: 

bug #57064 made me notice this bug report. Describing the change would indeed be important as I already came across this:

Octave 5.1:

octave> regexprep (char (181), 'u', 'micro')
ans = �

Octave 6.0:

octave> regexprep (char (181), 'u', 'micro')
error: regexprep: internal error calling pcre_exec; the input string is invalid UTF-8

Matlab:

>> regexprep (char (181), 'u', 'micro')
ans =
    'µ'

Guillaume <gyom>
Fri 02 Aug 2019 08:07:23 PM UTC, comment #21: 

Yes, we should absolutely update the NEWS file.

Rik <rik5>
Project AdministratorIn charge of this item.
Fri 02 Aug 2019 07:47:09 PM UTC, comment #20: 

I pushed a change on default to initialize the locale / character encoding always when the interpreter starts, added the setlocale gnulib module, and deleted some extraneous checks for the setlocale function from configure

https://hg.savannah.gnu.org/hgweb/octave/rev/463fc0cfed90

This fixes the main regression I saw with this change.

The remaining question is do we want to report an error if a user runs Octave in a non-UTF-8 locale with a regular expression that contains a UTF-8 character? Probably yes, but we didn't in earlier versions of Octave. It might be worth mentioning in NEWS that regular expressions now support multi-byte character matching and that all patterns are now implicitly UTF-8.

Mike Miller <mtmiller>
Project Administrator
Wed 31 Jul 2019 09:52:50 PM UTC, comment #19: 

I don't know who uses MinGW/Cygwin, but I do know that they will instantly complain vociferously despite representing <1% of Octave users (just a guess) if something goes wrong.  So, you should probably include the setlocale gnulib module.

Rik <rik5>
Project AdministratorIn charge of this item.
Wed 31 Jul 2019 09:17:15 PM UTC, comment #18: 

Any objections to me pushing the simple patch file #47275 to call 'setlocale' to initialize the user's locale settings when Octave starts? Should we also include the 'setlocale' Gnulib module? The Gnulib manual indicates it may help with some bugs on MinGW and Cygwin systems.

Mike Miller <mtmiller>
Project Administrator
Mon 29 Jul 2019 05:26:19 PM UTC, comment #17: 

The attached patch resolves the first issue for me. When the user environment variable settings declare a UTF-8 locale, the error message goes away in octave-cli.

So the remaining question is, if the user intentionally does not declare a locale or sets the locale to a non-UTF-8 one, should we allow regexp to raise an error message now? That's still a regression, although a smaller one that can be worked around.

(file #47275)

Mike Miller <mtmiller>
Project Administrator
Mon 29 Jul 2019 04:36:05 PM UTC, comment #16: 

It's similar, but not bug #50409, because in that bug report you are clearly pointing to something being initialized in the Windows terminal emulator.

I guess the behavior I'm seeing can be grouped into two bugs / questions:

  • This change to regular expressions seems to require a UTF-8 locale be active, is this simply a call to 'setlocale'? And where should this be done for it to work correctly in octave-cli?
  • If the above is correct, what should we do in cases where a user is running Octave in a non-UTF-8 locale, for example LC_ALL=C? Should we only conditionally add the "(*UTF8)" prefix to regular expressions when the locale will support it? Or should we just let the error message I've shown happen?
Mike Miller <mtmiller>
Project Administrator
Mon 29 Jul 2019 06:59:11 AM UTC, comment #15: 

Mike: Thanks for testing.

You're probably seeing bug #50409. That bug is marked as Windows only. But I guess you can also see the same when setting up Linux with a non-UTF-8 locale.
I don't know in which file or function we should change this command line setting though.

Markus Mützel <mmuetzel>
Project Member
Sun 28 Jul 2019 11:30:28 PM UTC, comment #14: 

Ok, I can now also reproduce this on my full development system, not just in a container, but only with octave-cli.

So to repeat in a more reproducible way, here is regexp_error.m:

c = regexp ('lorem ipsum', '^\s*(⇒|=>|⊣|-\|)')

And here are three examples, showing that it works without error in interactive Octave with LANG set to include UTF-8, but errors when locale variables are not set or when running in batch mode from the command line:

$ octave-cli-6.0.0 -q
>> regexp_error
c = [](1x0)
>>

$ env -u LANG octave-cli-6.0.0 -q
>> regexp_error
error: regexp: unrecognized character after (? or (?- at position 13 of expression
error: called from
    regexp_error at line 1 column 3
>>

$ octave-cli-6.0.0 -q regexp_error.m
error: regexp: unrecognized character after (? or (?- at position 13 of expression
error: called from
    regexp_error at line 1 column 3

When octave-gui is used, these errors are not raised. So there is possibly some locale initialization that happens in the Qt framework as part of the octave-gui executable that is missing from octave-cli.

Mike Miller <mtmiller>
Project Administrator
Sun 28 Jul 2019 09:01:03 PM UTC, comment #13: 

I'm seeing some strange regressions with this change in a minimal container running Octave with some regular expressions containing UTF-8 characters (in the doctest package).

Example regexerror.m:

c = regexp ('lorem ipsum', '^\s*(⇒|=>|⊣|-\|)', 'lineanchors');

$ octave regexerror.m
error: regexp: unrecognized character after (? or (?- at position 13 of expression
error: called from
    regexerror at line 1 column 3

I get different results depending on whether this script is run from the command shell or in an interactive Octave, and whether the environment contains LANG or LC_??? variables containing UTF-8 or not. This suggests that there is something I could configure or install in my environment to fix this, but I have no idea what that is at the moment. Any ideas?

Mike Miller <mtmiller>
Project Administrator
Sun 28 Jul 2019 01:48:07 AM UTC, comment #12: 

> Right now, I guess I'm feeling selfish and would require others to modify their compile options.


I support this. I think that's an entirely reasonable thing to expect/require from a "modern" PCRE implementation.

Andrew Janke <apjanke>
Mon 22 Jul 2019 05:00:18 PM UTC, comment #11: 

> Right now, I guess I'm feeling selfish and would require others to modify their compile options.


I agree with that.

Mike Miller <mtmiller>
Project Administrator
Mon 22 Jul 2019 04:47:55 PM UTC, comment #10: 

I think we need to put in a compile-time test.  The question is, should we make it optional or not?  The existing test for PCRE in configure.ac is

### Check for PCRE regex library.

OCTAVE_CHECK_LIB(pcre, PCRE,
  [], [pcre.h pcre/pcre.h], [pcre_compile], [], [], [],
  [libpcre], [REQUIRED])

The addition of the "REQUIRED" field to the OCTAVE_CHECK_LIB macro means that the entire configure process will abort if PCRE is not available.  PCRE is a hard requirement.  So, the question is, should we require a UTF8 enabled PCRE as a hard requirement or not.

As Mike says, I bet most distributions have been compiling with UTF8 support for a long time now.  So for the vast majority of users this won't be an issue.  For those who compile their own PCRE, we can either inconvenience them by requiring that they re-compile their home-built library with UTF8 enabled, or we can inconvenience our lives by having a UTF8 check in configure and then adding #ifdef's around various pieces of code in our own codebase.

Right now, I guess I'm feeling selfish and would require others to modify their compile options.

Rik <rik5>
Project AdministratorIn charge of this item.
Mon 22 Jul 2019 04:01:50 PM UTC, comment #9: 

Thanks for the info. I wasn't aware that this was hidden behind a compile time switch.

Is mentioning this in the NEWS file enough? Or should we also include a test in configure?

Or should we make the whole UTF-8 support for regexp optional?

Markus Mützel <mmuetzel>
Project Member
Sun 21 Jul 2019 09:39:18 PM UTC, comment #8: 

According to bug #56652, this change means that Octave must be built against a PCRE library that is built with '--enable-utf8', otherwise an error is reported. The upstream PCRE library build system still requires that option in recent versions. Most distributions have probably been building with that option for many years, but if a user builds PCRE themselves, they need to know this. Should this be mentioned in NEWS?

Mike Miller <mtmiller>
Project Administrator
Sun 21 Jul 2019 04:15:43 PM UTC, comment #7: 

I ran the test again before applying the patch:

>> tic, for i=1:1e5, string = regexprep('§x', '^(.)', '$1;'); end, toc
Elapsed time is 1.10611 seconds.
>> tic, for i=1:1e5, string = regexprep('§x', '^(.)', '$1;'); end, toc
Elapsed time is 1.09538 seconds.
>> tic, for i=1:1e5, string = regexprep('§x', '^(.)', '$1;'); end, toc
Elapsed time is 1.12862 seconds.

And again with the patch:

>> tic, for i=1:1e5, string = regexprep('§x', '^(.)', '$1;'); end, toc
Elapsed time is 1.1054 seconds.
>> tic, for i=1:1e5, string = regexprep('§x', '^(.)', '$1;'); end, toc
Elapsed time is 1.14079 seconds.
>> tic, for i=1:1e5, string = regexprep('§x', '^(.)', '$1;'); end, toc
Elapsed time is 1.12645 seconds.

Since the performance doesn't seem to be impacted, I went ahead and applied the patch here:
http://hg.savannah.gnu.org/hgweb/octave/rev/94d490815aa8

Marking as ready for test.

Markus Mützel <mmuetzel>
Project Member
Tue 05 Mar 2019 04:59:51 PM UTC, comment #6: 

The attached patch prepends "(*UTF8)" to the pattern string for regular expressions.
I hope this is the correct place to smuggle that string in.

(file #46432)

Markus Mützel <mmuetzel>
Project Member
Sun 03 Mar 2019 08:10:13 PM UTC, comment #5: 

Hmm, if we can't detect any perform difference (probably because when the UTF8 directive, but neither the string or pattern use UTF8, PCRE falls back to regular matching), then we should just add it.  I don't think Octave can be made to compile well on systems 10 years or older.

So, I guess you could prepare a changeset and we could do benchmarking with/without the cset and see if there is any difference before committing

Rik <rik5>
Project AdministratorIn charge of this item.
Sun 03 Mar 2019 05:03:08 PM UTC, comment #4: 

This is still the case with Octave 5.1.
As a work-around, it is possible to start the pattern with "(*UTF8)" with PCRE 7.9 and newer (Apr 2009):

>> string = regexprep('§x', '(*UTF8)^(.)', '$1;')

string = §;x
>> fprintf('%x\n', string)

c2
a7
3b
78

I didn't see a performance impact with a primitive test:

>> tic, for i=1:1e5, string = regexprep('§x', '(*UTF8)^(.)', '$1;'); end, toc
Elapsed time is 1.20157 seconds.
>> tic, for i=1:1e5, string = regexprep('§x', '^(.)', '$1;'); end, toc
Elapsed time is 1.23969 seconds.

Should we add this automatically? Or at least document it in the docstring for "regexp"?

I can't find a relevant thread in the mailing list archives.

Markus Mützel <mmuetzel>
Project Member
Sat 19 Nov 2016 09:08:45 PM UTC, comment #3: 

This issue is still present in Octave 4.2.0.

Hartmut <hardy>
Thu 04 Jun 2015 01:35:26 AM UTC, comment #2: 

Updating status to the development release, still present.

Mike Miller <mtmiller>
Project Administrator
Sun 25 Mar 2012 08:30:49 PM UTC, comment #1: 

Confirmed on a recent tip.

Currently Octave does not support UTF8 in regular expressions, although it is not hard to add.  I've made an experimental patch that does so but it does impact performance.  UTF8 is 23% slower than non-UTF8 pattern matching. 

Adding support for UTF8 is a big enough change that I am going to take the issue to the Octave Maintainers mailing list so that we can make a decision about how best to implement this.  It is also big enough that this will come out not as a bug fix, but in the next major release of Octave (3.8.0).

Rik <rik5>
Project AdministratorIn charge of this item.
Tue 20 Mar 2012 04:29:25 PM UTC, original submission:  

When matching a single character at a position where there's a multi-byte UTF-8 character, only the first byte is matched. Depending on how this match is then processed, it can result in invalid UTF-8. Example:

string = regexprep('§x', '^(.)', '$1;')
fprintf('%x\n', string)

yields

string = ?;?x
c2
3b
a7
78

where "?" is the replacement character and the UTF-8 codes for "§", ";", and "x" are "0xC2 0xA7", "0x3B", and "0x78", respectively.

The expected output would have been

string = §;x
c2
a7
3b
78

Burkart Lingner <burkart>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #47712:  bug35910_regexp_utf8_doc_v2.patch added by mmuetzel (3KiB - application/octet-stream)
file #47709:  bug35910_configure_pcre_check.patch added by mmuetzel (2KiB - application/octet-stream)
file #47708:  bug35910_regexp_utf8_doc.patch added by mmuetzel (2KiB - application/octet-stream)
file #47275:  octave-setlocale.patch added by mtmiller (437B - text/x-patch)
file #46432:  bug35910_regexp_utf8.patch added by mmuetzel (894B - application/octet-stream)

 

Carbon-Copy List
  • -email is unavailable- added by gyom (Posted a comment)
  • -email is unavailable- added by cbm
  • -email is unavailable- added by apjanke (Posted a comment)
  • -email is unavailable- added by apjanke (Adding self to Cc list)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by hardy (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by burkart (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 18 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-10-18 mmuetzel Attached File- => Added bug35910_regexp_utf8_doc_v2.patch, #47712
    2019-10-17 mmuetzel Attached File- => Added bug35910_configure_pcre_check.patch, #47709
        StatusReady For Test => Patch Submitted
    2019-10-17 mmuetzel Attached File- => Added bug35910_regexp_utf8_doc.patch, #47708
    2019-10-16 mmuetzel Dependencies- => bugs #57064 is dependent
    2019-07-29 mtmiller Attached File- => Added octave-setlocale.patch, #47275
    2019-07-29 cbm Carbon-Copy- => Added cbm
    2019-07-21 mtmiller Dependencies- => bugs #56652 is dependent
    2019-07-21 mmuetzel StatusPatch Submitted => Ready For Test
    2019-03-05 mmuetzel Attached File- => Added bug35910_regexp_utf8.patch, #46432
        StatusIn Progress => Patch Submitted
    2019-03-05 apjanke Carbon-Copy- => Added floss@apjanke.net
    2019-03-03 mtmiller Carbon-CopyRemoved 80942 => -
    2015-06-04 mtmiller Release3.6.1 => dev
        Operating SystemGNU/Linux => Any
    2012-03-25 rik5 CategoryInterpreter => Libraries
        StatusNone => In Progress
        Assigned toNone => rik5

    Back to the top


    Powered by Savane 3.5