This is a duplicate of T388991.

If I'm understanding this right, this seems rather round-about in relation to the incident. It sounds like the focus is being placed on the particular script that's been loaded, and its specific method of operating, i.e. POST requests on the regular, web UI edit forms. Does a theoretical solution to this task cover a script (which (a) in the first place has to be loaded by a user (b) can spread itself by editing site JS freely if the user is privileged enough) that simply goes for ApiQueryRandom followed by a form submission, or just the edit API?

Somewhat mixed feelings on whether exceptions like this should be logged or not, but I guess it takes one bad actor or an AI crawler at Special:CargoQuery or Special:Drilldown to spam server logs with something that ultimately doesn't reflect on anything actually being broken on the wiki. (Probably. There's always the 'view more results' links.)

I have a feeling that rebuildall doesn't re-parse pages, so doesn't do the same as null edits.

rebuildall's a shortcut for rebuildtextindex, rebuildrecentchanges, and refreshLinks, from which refreshLinks does re-parse pages for derived data updates (like link table changes). This includes category changes.
What null-edits do, is invoke a derived data update and run page save hooks (and this largely differs from regular edits in that a CDN update won't be scheduled).

What commit are you on with 1.45? This would look like touched timestamp updates having been restored on login, which is https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1199878, but that change isn't present on the REL1_45 branch.

Pretty sure yeah.

Sure, but it should be handled either gracefully (i.e. letting the broken character through without breaking any parser behaviour) or prevented (by blocking edits from happening with more than an exception blanking out the page - except this path comes at a trade-off of deterring less technical users from making edits if they encountered a "bug" in a module). IMO it's a stretch to go straight for GIGO when it's a result of the built-in string library, that there's no loud "do not use this unless your input is ASCII-only 100% of the time no matter who comes by" warnings around.

For extra context:

• The Unicode-unaware string Lua library is tearing "—" in half, and the resulting broken sequence is not being sanitised. (In fact, MediaWiki happily outputs the "broken" character if the module's made to return it directly).
• The broken UTF-8 data is causing the parser's regex matching to silently fail.
• Before 975dada200fa3d59f69fee53870612e4f09acc8d (shipped in 1.45), an exception would be thrown with the message "PCRE failure" from Parser::handleExternalLinks.

Well, in general Cargo should not be using exceptions for validation and rather Status objects, but that's a larger rewrite.

I think these three address the issues around mw.ext.cargo.declare, and a possible TypeError on mw.ext.cargo.store. Still, .store could benefit from some type checking improvements (check if literal then type-convert to string) as previously there's been a number of issues resulting from this not being done, so this task shouldn't be closed just yet. (I can submit a patch probably some time during the week). Other decent improvement would be throwing inside of .declare on an error rather than requiring that the user includes the output (which may be a null) in their function's output.

Possible that this patch results in a minor regression with non-wikitext pages no longer saving records to special tables like _pageData. Posting this just to document the difference, I don't think it's really significant.

Will send patches for (2) and (6) in the next 24h, possibly (4) and (5) but I'll need to go through some existing code from other extensions to see how that's handled.

IMO avoid a pattern where you invoke a table declare on each module call. You'll end up with a potentially fairly nasty moment "if" (more on that later) the module gets called on multiple template pages (or a doc page), and in normal circumstances invoking cargo_declare on a non-template page is an error (more on that later in your scenario). If there's an accompanying template (which you sure have given the namespace requirement), it'll be much cleaner if you just call cargo_declare on your own right on the template page.

Doesn't feel like my call really haha, but I'd probably go allow + warning in 1.43-1.45 backports to avoid breaking someone's setup, fail in 1.46 as supporting the --option value syntax only some of the time is quite quirky.

As for the disallow fix, might be as simple as $argv[1] === $script?

Not the most insane oversight to happen since the doccomment for initFromWrapper implies script name's expected at $argv[1], and passing other options before it wasn't anticipated. (This makes the documentation change more interesting, and I suspect that's the source).

Retitled to reflect the issue better. The bug is not necessarily about addArg/getArg, and more so about how the argument line is treated during script setup,. Details below the divider line.

This bug is present in previous releases as well: https://github.com/weirdgloop/mediawiki-extensions-OAuth/commit/6b8f1f87a6dcc8646e73cc88320d90467976f319

Other way (without custom classes) to work around table aliases not being set on the secondary DB handles is using the ForeignDBViaLBRepo file repo class and at minimum assigning the default section in wgLBFactoryConf.

I believe T368206 is related

Looks like the wrong hook being used in AbuseFilter, or a core bug in SpecialMovePage.

Session yes, if they're logged in, or reached Special:CreateAccount/Special:UserLogin. Anons get the ChronologyProtector cookie but that only lasts 10s, and might also often not be configured to exempt from caches on third-party setups.

CargoPopulateTableJob may need the same fix and that should stop the duplicate writes from the job queue... at least in theory.

Not exactly what I meant; it's possible the implicit arg is required to trigger the problem, but without unique the extra row would be getting culled.

Looks like implicit args are missing a trim + an emptiness check that's done for explicit arguments: https://github.com/wikimedia/mediawiki-extensions-Cargo/blob/master/includes/parserfunctions/CargoStore.php#L118 . Possibly explains why the param has to be provided to provoke the bug.

Can only speak here for the 'other information' section:

Hah, can only wish you good luck, sounds like you may need it...

The page is over 100KB large, so If103e2 (T344505) is kicking in for performance reasons. This would also mean T405827 is a duplicate of this ticket.

Can confirm the patch corrects the highlighting :)

Can confirm the patch fixes the problem.

Yeah, wasn't sure how to categorise this as I'm aware a "proper" fix is going to be difficult, but hyphens are probably common enough to call this behaviour change a regression :/

It isn't, but you can say the same thing about any sort of QoL feature.

@lucaweiss Update your Cargo install and revert the revert.

Unsure how helpful this is for diagnosing, but at my workplace we're running a modified version of the TextInput component to work around Vue's composition handling (which is something close to firing key/input events only when composition is finalised).

// at the start of onKeydown and onInput handlers:
if ( event.target.composing ) {
  event.target.composing = false;
  wrappedModel.value = event.data;
}

What virtual keyboard are you trying? Samsung's default one ("Samsung Keyboard") reproduces this issue reliably if predictive text is enabled in settings (default).

AFAIK, cleaning up the handling of <tableA>.<column> is one of those things that requires migrating some edge cases

Thanks for the quick fix! :)

Tagged you directly, MusikAnimal and bhsd, hope that's alright with you!

No, it only covers users who can set those browser- or system-wide preferences that automatic dark mode checks. This task is for users who have to override automatic detection by manually selecting the dark mode in Vector's rail.

Likely related to https://phabricator.wikimedia.org/T295166 as predictive text in the Samsung Keyboard is based on composition events.

$wgRestPath is not relevant here unless you're doing something crazy with your setup where $wgScriptPath is irrelevant to the REST API endpoint.

This is outside of WMF-hosted wikis, correct? Please check whether rest.php routing was configured - using ENWP as reference, the URL to try will be similar to https://en.wikipedia.org/w/rest.php/v1/search/page?q=earth&limit=1 (adjust query as needed).

Doesn't appear to; when I looked into this before submitting the report, it seemed like CodeMirror is clearing the replace phrase internally when the menu is opened (or closed, not sure), and the phrase needs to be 'resubmitted'. But I'm not familiar with CodeMirror or any similar solution, so leaving it to the experts.

thank you!

@ssastry Yup, I found this out when testing a non-WMF extension which the company I work for has in development.

The patch above seems to have (unintentionally?) affected the ApiParseMakeOutputPage hook.
Notably, prior to the patch implementations had access to the page's (though obviously without the transform pipeline's changes) metadata via the OutputPage - an example use-case would be queuing ResourceLoader modules depending on the page's categories to supplement BeforePageDisplay. This is no longer the case. OutputPageBeforeHTML hook is likely also impacted.

This extension hasn't worked correctly for a while as it does not split parser cache for anonymous users. Possibly I'm wrong on this but at least in MW 1.41 there was no way of splitting cache like required here, and doing so on larger sites would be quite sub-optimal.

Yeah this isn't the cleanest report so sorry for the confusion

thank you too!

Additionally, looks like in Chrome and Edge (tested Chrome 134.0.6998.179 + Edge 135.0.3179.54 on Windows 11) if you copy the selected text from the diff section with Ctrl+C or context menu, it's not possible to paste the text into the editor with Ctrl+V until something is typed first (which then unstucks the editor).

You're hitting ->parse() on whichever message is being parsed when you get the error, instead of ->preprocess(); the workaround for parse will look something like this:

Variables no longer uses the InternalParseBeforeSanitize hook (that's why this ticket has been marked as resolved) if you're using the latest version from the master branch, REL1_42 (though this one is missing the template cache fix), or REL1_43.

Unfortunately I'm seeing this issue in one of my 1.41 and 1.43 development sandboxes. Both are running on the same NixOS-based host inside a single Ubuntu 24.04-based Docker container running PHP 8.1.31 (Ondrej's PPA - https://launchpad.net/~ondrej/+archive/ubuntu/php) and NGINX. Doesn't matter whether Opcache or JIT are enabled. It also doesn't matter whether any extensions or skins are installed - as in, I can reproduce the problem on the install without any extensions or skins, as long as I'm logged in and the interface language (prefs or uselang) is set to a language with grammatical gender (tried es, fr, pl, all exhibiting the same issue). This happens regardless of MW's cache settings or the DB used.

The hook was added to the extension's general codebase in da7d2c3, which is a sync with ShoutWiki's codebase, so it must've never been in mainline MediaWiki.