bastian / bstats-backend Goto Github PK

Regarding PaperMC/Paper#4914

While this was a good change on our end, I have a suggestion for improving it on your end.

The library p-queue is my favorite promise based queue library for NPM, highly used.

Use this library to throttle submission tasks to ensure only X amount can be concurrently processing at a time.

This should be a rather simple and quick change (Sorry don't have time to PR this, but I'm sure you get the gist of what I'm suggesting)

Furthermore, bstats submissions are not end of the world if the request is dropped.

I suggest once the submission has been received over the http endpoints, to not await the promise for completion of the submission, and just quickly tell the submitter "got it" once validation is done, submitting the data into the p-queue to fire when ever its turn comes.

Shutdown of the process does need to block until p-queue's instance .onIdle() method resolves to ensure all pending tasks are flushed before shutdown finishes.

Finally, one final optimization that could be made is if the p-queue size > X, to write the request data to a temporary data file on disk, unload the request object from memory (releasing references to the object), and submit into the p-queue a task that will load the data from FS before proceeding.

You could just do this for all requests to keep it simple, then the p-queue tasks only hold a reference to a temporary file name, and shutdown could let those pending files stay on disk and then only need to flush in-processing tasks at shutdown.

This would guarantee memory stays light even if 10k pending submissions are waiting.

Happy to provide a code review if you need for this (I've done this kind of logic for my own work use plenty)

I'm creating a multi-server project like this, but for other purposes, and when I was analyzing the http requests from the plug-ins to the backend, I wondered what would tell me if those requests come from a plug-in or from who uses a postman on the internet trying to mess up my data. And after a lot of research, I surrendered to looking at other open source projects, so I looked at yours and realized that you don't do any of that...

i copied the json sent to the server, set it to a single random plugin and changed the data. shutted down the server, opened my postman and sent this json:

https://bStats.org/submitData/bukkit

{
    "serverUUID": "8c5c4bc6-1f94-460f-9c34-677e07629866",
    "playerAmount": 7395,
    "onlineMode": 1,
    "bukkitVersion": "git-Spigot-21fe707-e1ebe52 (MC: 1.8.8)",
    "bukkitName": "CraftBukkit",
    "javaVersion": "1.8.0_271",
    "osName": "Windows 10",
    "osArch": "amd64",
    "osVersion": "10.0",
    "coreCount": 6,
    "plugins": [
        {
            // EssCore is the random plugin
            "pluginName": "EssCore",
            "id": 2729,
            "pluginVersion": "1.1.0",
            "customCharts": []
        }
    ]
}

And as you can see here (https://prnt.sc/xddqlx) it worked.

I remembered several times that friends asked me if a developer was good and some showed me their bstats graphics with a supposed incongruity of the data looking like the print. I'm not saying that anyone has done this before, but it would be a good way to pass development reliability.

I don't understand authentication models very well or even if it is possible to detect a server or not, but I would try to find a way, as it would be very bad for a project of this level to have untrusted data.

At the moment, CORS is enabled.:

Currently, this is no problem as the backend only serves data that is publically available. But as soon as we introduce protected routes, it should be disabled.