Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StackOverFlowError still not fixed #3797

Closed
TheDGOfficial opened this issue Mar 8, 2021 · 6 comments
Closed

StackOverFlowError still not fixed #3797

TheDGOfficial opened this issue Mar 8, 2021 · 6 comments
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. priority: low Issues that are not harmful to the experience but are related to useful changes or additions.

Comments

@TheDGOfficial
Copy link
Contributor

Description

StackOverFlowError on very long lines. Still not fixed after #3195, though that only fixed (well, not a fix but a warning) it on aliases. It still occurs on scripts. Though this an user error (having very long lines), it prints a very long stack-trace and says it to report to here, so giving a more user friendly Skript#error would be better.

Besides, it is not fully a user error. While having long script lines is often bad code, this still a Java exception and our problem. If JDK didn't used a recursive regex implementation that would not be a problem - but it is. Perhaps the regex triggering this error should be optimized. That would can speed up parsing speed, too.

Steps to Reproduce

Use this script and start the server or reload optionally.

on command:
 if command is "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "":

Might want to increase the line's character count by just repeating it more if you don't get the error (the max stack size JDK option is platform dependent)

Tested on Java version (System: 64-bit Windows 10):
image

Can also test with Java 16 and 17 early access if desired, I don't think results will change however.

Expected Behavior

Successfully parsing and/or getting a meaningful, friendly error (in case of success, we would get an empty configuration section warning since there is no other code after the if condition)

Errors / Screenshots

Very long stacktrace repeating like this (ommitted, very very long, can provide full if desired but it doesn't contain anything related to Skript):

java.lang.StackOverflowError: null
        at java.util.regex.Pattern$Branch.match(Pattern.java:4800) ~[?:?]
        at java.util.regex.Pattern$GroupHead.match(Pattern.java:4855) ~[?:?]
        at java.util.regex.Pattern$Loop.match(Pattern.java:4964) ~[?:?]
        at java.util.regex.Pattern$GroupTail.match(Pattern.java:4886) ~[?:?]
        at java.util.regex.Pattern$BranchConn.match(Pattern.java:4763) ~[?:?]
        at java.util.regex.Pattern$CharProperty.match(Pattern.java:3996) ~[?:?]
        at java.util.regex.Pattern$Branch.match(Pattern.java:4800) ~[?:?]
        at java.util.regex.Pattern$GroupHead.match(Pattern.java:4855) ~[?:?]
        at java.util.regex.Pattern$Loop.match(Pattern.java:4964) ~[?:?]
        ... ommitted

Server Information

  • Server version/platform: 1.16.5/Paper
  • Skript version: 2.5.3

Additional Context

I've managed out to find the cause on actually where this occurs on Skript side and will provide a PR.

@TPGamesNL TPGamesNL added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. priority: low Issues that are not harmful to the experience but are related to useful changes or additions. labels Mar 8, 2021
@Ryan12439
Copy link

Ryan12439 commented Mar 8, 2021

I have had this bug as well, and it has caused me a world of trouble. I’ve tried many suggestions from other threads. I’ve made my lines short (my longest line is about 15 words long), I’ve used both OpenJDK and Oracle, versions 8, 11, and 15, to no avail. This issue being fixed would make my life much easier. I should add that I am also using 1.16.5 Paper and Skript 2.5.3

@TheDGOfficial
Copy link
Contributor Author

TheDGOfficial commented Mar 9, 2021 via email

@TheDGOfficial
Copy link
Contributor Author

TheDGOfficial commented Mar 9, 2021 via email

@Ryan12439
Copy link

Ryan12439 commented Mar 9, 2021

The thing I’ve read about Xss is that it increases the likelihood of a memory overflow error. I am running 64 bit Java. I’m not exactly sure how to test the PR but I will try to figure it out and let you know.

edit: Not memory overflow but out of memory

@Ryan12439
Copy link

Ok, I got your pull enabled, and somehow it fixed the regex error instead of just warning me about it...

@TheDGOfficial
Copy link
Contributor Author

TheDGOfficial commented Mar 9, 2021 via email

@Whimsyturtle Whimsyturtle added the completed The issue has been fully resolved and the change will be in the next Skript update. label Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. priority: low Issues that are not harmful to the experience but are related to useful changes or additions.
Projects
None yet
Development

No branches or pull requests

5 participants