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

Dumb objectiveconfiguration (loop) can overload server - Warning would be useful #1017

Open
bundabrg opened this issue Nov 12, 2019 · 4 comments
Labels
Bug A bug in the code or in a documentation. Confirmed Something is confimed. Feature Request New feature or request.

Comments

@bundabrg
Copy link
Contributor

Reported by Toffy in Discord:

I found an easy way to crash a server.

  • Make an objective that on finish, will run 2 events, one, removing a certain journal entry, and another one "objective finish objectivename" , the objectivename will be the objective finishing itself.

  • Since the objective finishing, runs another event finishing itself, it will finish itself over and over, keep running the journal entry update over and over, eventually overusing server resources.

I haven't tested with other events, only journal entry updates

@bundabrg bundabrg added the Bug A bug in the code or in a documentation. label Nov 12, 2019
@J0B10
Copy link
Member

J0B10 commented Nov 12, 2019

I guess you can also do this with the folder event:

Create a folder event without a delay that executes itself and another event.
In fact every recursive call of events without a delay will at some point lead into a heap space or a stack overflow exception and crash the server.

I don't see a big issue here cause it is quite obvious that that shouldn't be done and it isn't your job to prevent every eventuality where the plugin could be misused.

Nevertheless if you could write a simple fix it would improve usability.
But atm I see no simple way of detecting these kind of recursive calls.
Maybe the wiki should be updated to make all quest writers aware of this issue.

@TofPlays
Copy link
Contributor

I didn't report it as a bug, just noted it, However I'm all for improving usability. And yeah it is user's responsibility to use those things wisely.

@bundabrg
Copy link
Contributor Author

I logged it mainly just to look closer later. Infinite loops don't necessarily need protection from but as it is easy to get into this scenario with how BQ has assets across multiple files either a timeout or a check during loading the info might be implemented.

@Wolf2323
Copy link
Member

Wolf2323 commented Mar 3, 2020

This could help detecting loops, if we want to fix it https://stackoverflow.com/a/2663147/10134392

@Wolf2323 Wolf2323 added this to TODO in BetonQuest 1.12.0 via automation Jun 13, 2020
@Wolf2323 Wolf2323 removed this from TODO in BetonQuest 1.12.0 Jun 30, 2020
@Wolf2323 Wolf2323 added this to To do in BetonQuest 2.0 Deprecated via automation Aug 29, 2020
@Wolf2323 Wolf2323 added the Confirmed Something is confimed. label Sep 30, 2020
@Wolf2323 Wolf2323 added this to TODO in BetonQuest 2.0.0 via automation Dec 30, 2020
@Wolf2323 Wolf2323 removed this from To do in BetonQuest 2.0 Deprecated Dec 30, 2020
@Wolf2323 Wolf2323 moved this from TODO to TODO Pre-Release 2 in BetonQuest 2.0.0 Dec 30, 2020
@SaltyAimbOtter SaltyAimbOtter changed the title Self finishing objective can crash server Dumb objectiveconfiguration (loop) can overload server - Warning would be useful Jan 9, 2021
@Wolf2323 Wolf2323 added the Feature Request New feature or request. label May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in the code or in a documentation. Confirmed Something is confimed. Feature Request New feature or request.
Projects
Status: Todo
BetonQuest 2.0.0
  
TODO Pre-Release 2
Development

No branches or pull requests

4 participants