Strange issue regarding code sync - allows scripts over the limit to be synced

[qsniyg]

I set up https://openuserjs.org/scripts/qsniyg/Image_Max_URL to have code sync with Github, and though I initially set it to sync from userscript_min.user.js (I created the script to import from Github, and selected that file), it appears to be syncing from userscript.user.js instead.

This isn't an issue for me, I only created userscript_min.user.js in order to fit within the 1MB limit (and even with pretty much the maximum compression without resorting to an obfuscation-like technique, it's still ~860KB), having it sync from an unminified version is definitely preferable. However, I believe this would theoretically allow any script to violate the site's limits (including, accidentally, my own), as well as the fact that it syncs from the wrong file, which I guess on a repository that contains multiple different userscripts, this could lead to issues.

When downloading the userscript now, it's ~3.3MB, which is well above the limit:

$ curl 'https://openuserjs.org/install/qsniyg/Image_Max_URL.user.js' | wc -c
3373819

[Martii]

OY! Long time boog. This section of code is going to need a massive rewrite at some point (see #1705). Got a patch forthcoming that ignores the push. As I've mentioned before the connection to GH only sends a response that we're going to process a list of scripts... so you'll still get a 200 response but it will silently ignore files that have too large of a byte count.

Thanks for the report.

Loosely related at #793

[qsniyg]

Thanks for replying!

Unfortunately though, this doesn't fix the main issue, which was that it synced from the wrong file (it synced from userscript.user.js, instead of the file I set it to sync to, which was userscript_min.user.js). Now it unfortunately just fails to sync altogether.

[Martii]

it synced from userscript.user.js

If I'm understanding that part correctly reimport your script using your min file. If that doesn't work then GH is doing something hinky with the messages that they send us. As long as your metadata blocks are consistent between the unabridged and abridged (min file)... it should fix any sync-age issues.

[qsniyg]

@Martii Thanks! The import worked fine, I guess I'll find out later if it works properly with the webhook :) If it does work, it's odd that it didn't before, really curious as to what the issue might have been.

[Martii]

I guess I'll find out later if it works properly with the webhook

Wait at least five minutes for GH's cache to expire... they do that for DoS protection on their end. Might be shorter but that's a good wait time between changes.

Thanks!

Welcome and rethanks for the report. Things should be consistent. Uploading/updating via importing your ~3MiB file took quite some time so the limits should still be applied with GH in all instances.

[Martii]

really curious as to what the issue might have been.

Pondering on this a bit more in the noggin. If your abridged and unabridged are in the same repo, since we are metadata blocks-centric and not (as much of) filename, GH may have sent the abridged first in their list and then the unabridged next in the list... thus was the bug I fixed with the limits. If the unabridged is below the limit then there is a 50% chance the abridged may make it to OUJS or the other 50% the abridged. Depends on their ordering at any specific time.

[sizzlemctwizzle]

The hook endpoint gets a json diff for each commit that occurs in a GH repo that has the webhook setup for the oujs hook endpoint. The endpoint ignores commits to any branch except master. It also only pays attention to changes to files that end in .js. Files that end in .user.js are handled differently than .js (my memory is hazy about how these are hadled). The filename is unimportant beyond that it ends in .user.js and if the username for the repo corresponds to a oujs user who has authenticated with GH, then the current source of the file (at HEAD in the master branch) is retrieved from GH. Then the userscript metadata block is parsed, and if the @name and @namespace combination match an existing script under this users account on oujs, the the new source replaces the existing on the site. It might seem complicated, but the intent is for it to be transparent with no need for configuration: like worrying about whether the file names are the same as what oujs generates. You could place the unminified version in a different branch than master, and even keep the "_min" in the filename on GH so that it is clear that it has been minified. It kind of makes since since the min version is what you want to publish, and the extra step of minification could be done in two commits (min plus rename) on top of the current unminified version when you merge that branch into master.

…

On Sun, Apr 12, 2020, 5:45 PM Marti Martz ***@***.***> wrote: really curious as to what the issue might have been. Pondering on this a bit more in the noggin. If your abridged and unabridged are in the same repo, since we are metadata blocks-centric and not filename, GH may have sent the abridged first in their list and then the unabridged next in the list... thus was the bug I fixed with the limits. If the unabridged is below the limit then there is a 50% chance the abridged may make it to OUJS *or* the other 50% the abridged. Depends on their ordering at any specific time. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1715 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAIXRQ46LIBV53TLH2F7O3RMJACNANCNFSM4MGP4TBA> .

[Martii]

Files that end in .user.js are handled differently than .js

Yup... those are user scripts "filtered" on add script.

It also only pays attention to changes to files that end in .js

Specifically to library scripts with the UserLibrary key pair.

You could place the unminified version in a different branch than master

Good idear to avoid the 50/50 chance. :)

[sizzlemctwizzle]

Historically, the filename of a userscript has never mattered (the slight exception is the url used for updating). Two scripts, regardless of filename, with the same @name, @namespace, and @Version are generally regarded as the same script. This partially the main reason for such metadata. Therefore, one alternative solution would be to give the min version a slightly higher @Version, since the min version has the slight added feature of being minified. This would ensure that it would always end up being the latest version and the one hosted by oujs.

…

On Sun, Apr 12, 2020, 7:49 PM Marti Martz ***@***.***> wrote: Files that end in .user.js are handled differently than .js Yup... those are user scripts "filtered" on add script. It also only pays attention to changes to files that end in .js Specifically to library scripts with the UserLibrary key pair. You could place the unminified version in a different branch than master Good idear to avoid the 50/50 chance. :) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1715 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAIXRVYAYZLJVC2NRJ7MT3RMJOQ3ANCNFSM4MGP4TBA> .

[Martii]

Historically...

Ideally...

Therefore, one alternative solution would be to give the min version a slightly higher @Version, since the min version has the slight added feature of being minified.

Again ideally... however we have no mandatory version increment and neither does GM/TM/etc. My Hello, World script was for a while constantly being upped from 0.0.0 to 0.0.3 (or higher) and then I originally reset it to 0.0.0. Some Authors may only change the script content and not the metadata block at the last moment. This is why I put in the OpenUserJS block with the @hash which we do check to see if there is a difference and only update the script if it's different. We don't currently enforce incrementing the @version value to my knowledge. We could but that would probably mean more work on the backend when troubles arises. Npm is a good example of what you are ideally suggesting... but also comes with its own can of worms.

Currently if the same metadata block comes through first with a higher @version that is minified and then the next in the list would be an unmified @version that is lower... that lower @version would be hosted on OUJS... and visa versa of course. Depends on how GH generates the list. It probably follows Object rules on no guarantee of order and if it's asynchronous then who knows what the order could be. It's never guaranteed to be a FIFO because there are too many variables ( git, GH, date/time factors, any pre-run scripts, any post run scripts, etc.).

So I suggest that this wouldn't be a consistent way of doing this to avoid the 50/50 possibility. Having the unminified on a different repo (which is also messy and why I didn't mention it earlier) is one option, naming the unminified as somescript.js and the minified as somescript.user.js would also solve it but eliminate direct installation/import from GH (which is why I didn't mention it earlier), and your good suggestion of putting the unminfied on a branch other than master of the same repo which makes it more easy to read/track and have a routine to generate the minified.

Believe me... if I thought everyone's internet and the server could handle bigger, I'd up the limit but when I upped it to 1MiB way back when that's pushing it imho from observation of several internet ISPs and also the server metric stats during high volume periods.

Ideally my patch should be before we get any of the buffers e.g. check the fetchRaw size headers (presuming those work/exist)... however since it's highly abstracted/integrated into everything in that library it could cause some havoc by breaking importing and other functions that are done i.e. #1705 . This entire routine needs more isolated common routines for I vs O in IO and not combined all into one... plus the "token" or "key" that GH is wanting for that now in #1705 . I thought of the initial authentication key but I don't think that's what it's asking for (i.e. got your email and that's covered in #1705 I think). With the QSP's in place that puts the burden on OUJS and if OUJS goes over the GH DoS limitation then no one can import. If we support a key per user (already have the UI ready but I haven't had the energy to rewrite githubClient and repoManager to read in a new user ghAccessToken because debugging is being picky on my dev station as I mentioned in email) then the burden is on the individual accessToken .

Anyhow long story short... I opted for a quick patch at the expense of fully downloading the file atm for security/stability reasons. Eventually it should be before any buffers come through to speed up CPU and BW... which is why I emailed you because it's not really geared for that currently. Debugging this portion of Code is more in my head, fuzzy, and node isn't cooperating with its debugger last check two months ago. I still have a lot of things on my plate and my current work is one of those vital ones in my area irl so I still don't have a huge amount of time to deal with #1705 . (I'll get burnt out real quick since it's going to take a rather large change from my perspective and too many unanswered questions)