Issue with frontier patterns #3

Closed
opened 2017-12-27 18:31:12 +00:00 by technomancy · 11 comments
technomancy commented 2017-12-27 18:31:12 +00:00 (Migrated from github.com)

%f[set], a frontier pattern; such item matches an empty string at
any position such that the next character belongs to set and the
previous character does not belong to set. The set set is
interpreted as previously described. The beginning and the end of
the subject are handled as if they were the character '\0'.

-- Lua 5.2 Reference Manual, §6.4.1

In the reference interpreter (Lua 5.3.4), the code belows prints a blank line and nil, indicating that the first pattern matched ('o' is not in [%.], but '.' is) and that the second one didn't.

In LuaJ 3.0.1, however, the code fails with

  org.luaj.vm2.LuaError: frontier-pattern-handling.lua:9 vm error: java.lang.ArrayIndexOutOfBoundsException: -1
  stack traceback:
    frontier-pattern-handling.lua:9: in main chunk
    [Java]: in ?
    at org.luaj.vm2.LuaClosure.execute(Unknown Source)
    at org.luaj.vm2.LuaClosure.onInvoke(Unknown Source)
    at org.luaj.vm2.LuaClosure.invoke(Unknown Source)
    at lua.processScript(Unknown Source)
    at lua.main(Unknown Source)
  Caused by: java.lang.ArrayIndexOutOfBoundsException: -1
    at org.luaj.vm2.lib.StringLib$MatchState.match_class(Unknown Source)
    at org.luaj.vm2.lib.StringLib$MatchState.matchbracketclass(Unknown Source)
    at org.luaj.vm2.lib.StringLib$MatchState.match(Unknown Source)
    at org.luaj.vm2.lib.StringLib$MatchState.start_capture(Unknown Source)
    at org.luaj.vm2.lib.StringLib$MatchState.match(Unknown Source)
    at org.luaj.vm2.lib.StringLib.str_find_aux(Unknown Source)
    at org.luaj.vm2.lib.StringLib$match.invoke(Unknown Source)
print(string.match("foo.", "(%f[%.])"))
print(string.match("foo", "(%f[%.])"))
> %f[set], a frontier pattern; such item matches an empty string at > any position such that the next character belongs to set and the > previous character does not belong to set. The set set is > interpreted as previously described. The beginning and the end of > the subject are handled as if they were the character '\0'. -- Lua 5.2 Reference Manual, §6.4.1 In the reference interpreter (Lua 5.3.4), the code belows prints a blank line and nil, indicating that the first pattern matched ('o' is not in [%.], but '.' is) and that the second one didn't. In LuaJ 3.0.1, however, the code fails with ``` org.luaj.vm2.LuaError: frontier-pattern-handling.lua:9 vm error: java.lang.ArrayIndexOutOfBoundsException: -1 stack traceback: frontier-pattern-handling.lua:9: in main chunk [Java]: in ? at org.luaj.vm2.LuaClosure.execute(Unknown Source) at org.luaj.vm2.LuaClosure.onInvoke(Unknown Source) at org.luaj.vm2.LuaClosure.invoke(Unknown Source) at lua.processScript(Unknown Source) at lua.main(Unknown Source) Caused by: java.lang.ArrayIndexOutOfBoundsException: -1 at org.luaj.vm2.lib.StringLib$MatchState.match_class(Unknown Source) at org.luaj.vm2.lib.StringLib$MatchState.matchbracketclass(Unknown Source) at org.luaj.vm2.lib.StringLib$MatchState.match(Unknown Source) at org.luaj.vm2.lib.StringLib$MatchState.start_capture(Unknown Source) at org.luaj.vm2.lib.StringLib$MatchState.match(Unknown Source) at org.luaj.vm2.lib.StringLib.str_find_aux(Unknown Source) at org.luaj.vm2.lib.StringLib$match.invoke(Unknown Source) ``` ```lua print(string.match("foo.", "(%f[%.])")) print(string.match("foo", "(%f[%.])")) ```
headcr4sh commented 2018-01-01 15:11:44 +00:00 (Migrated from github.com)

Thanks for the bug report.

Unfortunately, this repo is just an in-official mirror of the original luaj repo which is hosted over at SourceForge (https://sourceforge.net/projects/luaj/). This repo was my humble attempt to migrate the sources from the old CVS archives whilst preserving all of the commit history.

I tried to contact Ian Farmer and James Roseborough (the original luaj authors) a bunch of times, because I wanted to hand over the ownership of the GitHub organization (luaj) and the project (luaj).
Unfortunately -- as of today -- I was not able to reach them... :-(

(Long story short: I am not a luaj maintainer/committer and I am not able to cut any new releases which might end up in Maven Central or elsewhere,... so please don't be disappointed if I won't be able to triage your issue, provide fixes or release a patched version of luaj)

Thanks for the bug report. Unfortunately, this repo is just an in-official mirror of the original luaj repo which is hosted over at SourceForge (https://sourceforge.net/projects/luaj/). This repo was my humble attempt to migrate the sources from the old CVS archives whilst preserving all of the commit history. I tried to contact Ian Farmer and James Roseborough (the original luaj authors) a bunch of times, because I wanted to hand over the ownership of the GitHub organization (luaj) and the project (luaj). Unfortunately -- as of today -- I was not able to reach them... :-( (Long story short: I am not a luaj maintainer/committer and I am not able to cut any new releases which might end up in Maven Central or elsewhere,... so please don't be disappointed if I won't be able to triage your issue, provide fixes or release a patched version of luaj)
technomancy commented 2018-01-01 17:15:58 +00:00 (Migrated from github.com)

No problem; thanks for the explanation.

No problem; thanks for the explanation.
jberkel commented 2018-01-25 20:36:46 +00:00 (Migrated from github.com)

@headcr4sh it's a shame this project is no longer officially maintained. maybe jitpack.io could be an option to provide jar files from this repository? I have some other patches I could contribute (mostly build-system related).

@headcr4sh it's a shame this project is no longer officially maintained. maybe jitpack.io could be an option to provide jar files from this repository? I have some other patches I could contribute (mostly build-system related).
headcr4sh commented 2018-01-26 21:32:22 +00:00 (Migrated from github.com)

@jberkel
Well,... since neither Ian nor James have answered to my contact requests so far, it might be a viable alternative to keep this unofficial fork alive by just working on it nevertheless.

If you are interested in pushing the project forward (jitpack.io might indeed be an option, since no-one I know would be able to push artifacts under the org.luaj groupId to Maven central) I am willing to merge in any useful changes. The original history of the original authors is preserved and I don't think it would do any harm...

I will make sure that the README file gets adjusted to make it absolutely clear that this repo is a fork without any contributions / knowledge of the original authors so far, to make sure that no more false assumptions are being made.

@jberkel Well,... since neither Ian nor James have answered to my contact requests so far, it might be a viable alternative to keep this unofficial fork alive by just working on it nevertheless. If you are interested in pushing the project forward (jitpack.io might indeed be an option, since no-one I know would be able to push artifacts under the org.luaj groupId to Maven central) I am willing to merge in any useful changes. The original history of the original authors is preserved and I don't think it would do any harm... I will make sure that the README file gets adjusted to make it absolutely clear that this repo is a fork without any contributions / knowledge of the original authors so far, to make sure that no more false assumptions are being made.
jberkel commented 2018-01-30 15:11:35 +00:00 (Migrated from github.com)

@headcr4sh cool, I'll revisit my patches and then submit a PR here.

@headcr4sh cool, I'll revisit my patches and then submit a PR here.
jberkel commented 2018-02-01 09:09:30 +00:00 (Migrated from github.com)

@headcr4sh I noticed that your mirror is missing two changes from the CVS:

  • Add synchronization to CoerceJavaToLua.COERCIONS map. Jim Roseborough james@roseborough.com (Sat Jan 23 2016 19:11:21)
  • "Fix JsePlatform.luaMain() to provide an "arg" table in the chunk's environment." Jim Roseborough james@roseborough.com (Sat Jan 23 2016 17:52:59)
@headcr4sh I noticed that your mirror is missing two changes from the CVS: * Add synchronization to CoerceJavaToLua.COERCIONS map. Jim Roseborough <james@roseborough.com> (Sat Jan 23 2016 19:11:21) * "Fix JsePlatform.luaMain() to provide an "arg" table in the chunk's environment." Jim Roseborough <james@roseborough.com> (Sat Jan 23 2016 17:52:59)
headcr4sh commented 2018-02-01 11:50:43 +00:00 (Migrated from github.com)

Ok. Thanks for the information. I'll try to convert these two commits once I find the time (if anyone else with cvs2git knowledge kann provide a PR I'd also gladly merge that one)

Ok. Thanks for the information. I'll try to convert these two commits once I find the time (if anyone else with cvs2git knowledge kann provide a PR I'd also gladly merge that one)
jberkel commented 2018-02-01 12:04:53 +00:00 (Migrated from github.com)

I don't think you can update via cvs2git, so you might need to do a reimport. I don't remember which tool I used for the conversion, you can find the repo here, maybe it could be used as a base: https://gitlab.com/jberkel/luaj

I don't think you can update via cvs2git, so you might need to do a reimport. I don't remember which tool I used for the conversion, you can find the repo here, maybe it could be used as a base: https://gitlab.com/jberkel/luaj
plamentotev commented 2018-03-03 21:25:59 +00:00 (Migrated from github.com)

I think I found where the issue is. Here is a patch:

--- StringLib.java      2018-03-03 23:06:25.584417500 +0200
+++ luaj-3.0.1/src/core/org/luaj/vm2/lib/StringLib.java 2018-03-03 23:06:53.158652400 +0200
@@ -1066,9 +1066,12 @@
                                                        error("Missing [ after %f in pattern");
                                                }
                                                int ep = classend( poffset );
-                                               int previous = ( soffset == 0 ) ? 0 : s.luaByte( soffset - 1 );
+                                               int previous = ( soffset == 0 ) ? '\0' : s.luaByte( soffset - 1 );
+                                               // The C version does not do this check
+                                               // because in C strings are '\0' terminated
+                                               int next = ( soffset == s.length() ) ? '\0' : s.luaByte( soffset );
                                                if ( matchbracketclass( previous, poffset, ep - 1 ) ||
-                                                        matchbracketclass( s.luaByte( soffset ), poffset, ep - 1 ) )
+                                                        !matchbracketclass( next, poffset, ep - 1 ) )
                                                        return -1;
                                                poffset = ep;
                                                continue;

I would create PR once #4 is resolved.

As for the commits. This could be easily fix once the source forge is back online. I've created new issue to track this better #5

I think I found where the issue is. Here is a patch: ```diff --- StringLib.java 2018-03-03 23:06:25.584417500 +0200 +++ luaj-3.0.1/src/core/org/luaj/vm2/lib/StringLib.java 2018-03-03 23:06:53.158652400 +0200 @@ -1066,9 +1066,12 @@ error("Missing [ after %f in pattern"); } int ep = classend( poffset ); - int previous = ( soffset == 0 ) ? 0 : s.luaByte( soffset - 1 ); + int previous = ( soffset == 0 ) ? '\0' : s.luaByte( soffset - 1 ); + // The C version does not do this check + // because in C strings are '\0' terminated + int next = ( soffset == s.length() ) ? '\0' : s.luaByte( soffset ); if ( matchbracketclass( previous, poffset, ep - 1 ) || - matchbracketclass( s.luaByte( soffset ), poffset, ep - 1 ) ) + !matchbracketclass( next, poffset, ep - 1 ) ) return -1; poffset = ep; continue; ``` I would create PR once #4 is resolved. As for the commits. This could be easily fix once the source forge is back online. I've created new issue to track this better #5
Enyby commented 2018-09-13 23:25:15 +00:00 (Migrated from github.com)

@plamentotev
I think You can now make PR.

@plamentotev I think You can now make PR.
plamentotev commented 2018-09-16 09:40:45 +00:00 (Migrated from github.com)

@Enyby to be honest I forgot the details about this issue. I may take a look at it again but not sure if I would have time soon. Feel free to use the patch and create PR if you want.

@Enyby to be honest I forgot the details about this issue. I may take a look at it again but not sure if I would have time soon. Feel free to use the patch and create PR if you want.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: open-autonomous-connection/luaj#3