It seems like the reset title cache patch may have broken scribunto tests, at least on the REL1_43 branch.

OSM is using javascript gadget to swap the 24 hr dates to 12 hour ones

We should probably stop linking to sizes we no longer make.

See T393851

I kind of wonder if this is the sort of thing better handled by simply blocking users who do things like this. Even if we banned external links in sigs, nothing is stopping the user from manually writing out their preferred "signature" instead of using ~~~~. Restrictions on signatures more serve to prevent people who don't know better from doing annoying things. Its not really a good security measure against actually malicious users.

I guess so. In the longer term we probably want a better solution though.

Anyways, closing this bug since the file is uploaded.

Looking at logs, it looked like what happened is there were two publish jobs. The second one failed (due to the first one having a lock) which caused an error to be communicated to the client. However the first one was still going and eventually succeeded. Indeed https://commons.wikimedia.org/wiki/File:Cat_Valentine's_TOP_62_Moments_in_Victorious!_-_NickRewind.webm exists

In T393839#10809588, @Trade wrote:

Any suggestions for alternative solutions i can do instead?

In T391473#10726712, @dbenjaminmiller wrote:

Btw, when this goes live (on Commons and on enwiki), can you let me know?

This is kind of a weird situation, because the thing the test is supposed to be testing is that this doesn't happen.

So it looks like the DB use is as follows:

Thanks. I'll investigate further. My gut feeling is that opening the transaction is more due to the mediawiki framework than anything else, and the code isn't really relying on implicit transactions being there, so i suspect that is something fixable without too much trouble (and in any case given we are reconnecting, we dont get the benefits of implicit transactions anyways)

If I was trying to pivot, one thing I thing I'd try to do would be to try and write something to db or cache that might get executed. e.g. Anything still using php unserialize() or Mustache templates. So one thing that might make sense here is to set a different $wgSecretKey between auth and normal (for mustache) [or making a new var just for that], and making sure every instance of unserialize() uses the second argument to limit class types.

I'm pretty sure that master should work with 1.43, but im also happy to backport if that makes things easier.

So my theory for what is happening here so far:

Filed T391755 for increasing the upload by url time limit.

Looking further in the logs, it appears that the assemble job also loses the DB connection, but there is a Wikimedia\Rdbms\Database::handleErroredQuery: lost connection to db1227 with error 2006; reconnected log, so i guess no explicit transaction is open, so there is no issue. Kind of odd that Publish has an open transaction but Assemble does not.

Even if the speed of the connection to IA had been unlimited (or there had been no timeout for that portion), wouldn't the operation still fail at the publication stage?

Just as a note, there are reports of 180 seconds being too small at T391158

In T391473#10734452, @dbenjaminmiller wrote:

@Bawolff To be clear, I first tried async publication, and only later tried a non-async to see if it would work. But async from stash is what I was trying multiple times and it just didn't work. I also tried async from archive.org, too, but (unsurprisingly) that didn't work either.

In T391473#10731330, @bvibber wrote:

In T391473#10731293, @Umherirrender wrote:

Deadlocks are converted to DBQueryError and DBQueryError should not be catched, that mixed up the internal state of rdbms classes.

*nod* do not catch that :D gotcha.

The deletion on uploadstash is based on unique key, so not expecting a lock escalation to table lock. It seems more a gap lock on the unique key.
But the failing query is for a update of an (known?) existing row. Maybe the rows before and after are also "busy" by other running chunked uploads and mysql timeouts to do the changes.

Yeah, my theory is that the initial select for the state fetch is holding a lock in the transaction, and then that times out during file handling before we get to the update query. In which case I think the thing to do is:

• commit transaction after fetching initial state...
• perform the file operation...
• re-check status before updating, and kick out an error in case they don't match (rare, should indicate operator error)

However I'm not quite sure as I don't think it _should_ be holding a lock on the us_key unique index unless we issue a 'for update' in the select, which we're not. So I could still be wrong on this, but the ways of database locks are _very mysterious_ to me. :D

@Umherirrender if you think this sounds reasonable I can update the patch to that and we can see if it helps, but I feel like I'm going to want to test this with real files when I get back from traveling and am on my regular internet connection. :)

For The Cocoanuts.webm, we have the following errors:

So looking at the logstash, it appears the "async" flag was not set for this upload (and hence it was not done via the jobqueue). It is somewhat expected that non-async chunked uploads will fail for uploads of this size.

In T310051#10683519, @jhsoby wrote:

Now that the change has been deployed to ckbwiki, I think there's nothing more to do here. Thanks @Bawolff!

In T156184#10678570, @cscott wrote:

Probably a better solution would be to use a strip marker as the message parameter.

In T387130#10667775, @Reedy wrote:

Thanks for the backports!

Do we know offhand how long this has been around for?

The php bindings do not support tailorings. If they added that it would make a lot of things easier

In T310051#10648028, @Aram wrote:

Just to clarify, with putting vowels under their own header now, are we including وو in a separate header?

No, please. وو should be under و.

With the new changes, the only pairs of characters that are equal weight are: ک & ك and ھ & ه

Exactly!

However, I disagree strongly with the "inline SVG is secure in browsers by default" which is the current POC patch.

Thanks for your input on vowels. Since we've decided to give them their own headers, let's move ێ into its own section.

It might just have been linked somewhere prominent with some sort of embedded google translate.

In T310051#10639115, @Aram wrote:

@Bawolff Thanks for creating the demo and for all your explanations. I tested the demo and noticed the following:

• Sorting numbers numerically works perfectly. However, in the current version, each number seems to have its own header, whereas in the demo, all numbers are grouped under a single header (٠-٩). I don’t see this as a major issue, but if the community prefers the previous format, how can we revert to it?

Huh, weirdly they didn't just forget those characters but icu intentionally put them in the wrong place at the end. https://github.com/unicode-org/cldr/blob/main/common/collation/smn.xml#L27

This sounds like you were running the command as the wrong unix user.

Ok. Based on that description, i think its best we use a custom collation instead of the UCA based one (The custom one gives us more flexibility. The UCA one is more complex and might give better results for characters from other languages but it doesn't allow us to customize it as much. The big difference is that the UCA one allows more options for breaking ties, but i don't think that is super important for ckb. UCA might sort some obscure letters and foreign letters better).

hmm, i also wonder if projects using the identity (case sensitive) collation (like wiktionary) would be interested in this type of sorting, or if they really want every letter.

Just as a reminder (because i didnt see it in the SAL), after changing the setting you must run updateCollation.php (otherwise pre-existing categories will be sorted wrongly and behave weirdly.)

In T384395#10612910, @Amire80 wrote:

In T384395#10607538, @Bawolff wrote:

In addition to that patch, $wgCategoryCollation has to be changed for that wiki prior to running the maintenance script.

Thanks for the clarification! This makes me wonder, however: Why do we have it in local settings? For most languages, this should probably the default in core.

In T387691#10612861, @matthiasmullie wrote:

Not all data attributes need to be converted to data-mw-*; those in e.g. ComponentWidget.js are generated and used by JS only.

I also think we can safely fallback to the original (non data-mw-* prefixed) attributes in cached cases, without having to purge the caches: I narrowed down the selector to target only the nodes in specific places (i.e. those generated by PHP in the expected/predictable spot; not those inserted by users that appear elsewhere on the page)

In T387969#10611001, @John_Cummings wrote:

@Lucas_Werkmeister_WMDE thanks so much, who would be the right person in WMF responsible for UploadWizard to ping to make aware of the bug? I just want the right person to be aware of the issue.

In T310051#10611418, @Aram wrote:

I thought no one cared about this—thank you, everyone! While ckb and fa are somewhat similar, there are still differences. By the way, ckb has the same issue with the letter Heh. For example, the word «ھەور» (cloud) begins with the letter «ھ» but is classified under the letter «ه».

We don't really have the ability to break up groups (or at least, not very easily). We mostly just have the ability to chose which letter represents the group. (Or potentially a string like "ٲ - ئ" if that is better, as long as it starts with one of the letters in question)

This was likely an issue with UTF-8 normalization where the ø was being entered in ISO-8859-1 instead of UTF-8.

This sounds like it can be accomplished by a lua module if desired, or just manually with defaultsort. I would suggest declining this bug

The CKB collation is inheriting from the farsi collation.

Maybe includes/collation/data/first-letters-root.php has to be regenerated for more recent CLDR, or perhaps the script just chooses the wrong representive in that case. ꭓ certainly seems like a better representative of the class than ᶍ,

Looks like CLDR data to this day is still the wrong way - https://github.com/unicode-org/cldr/blob/main/common/collation/lt.xml . In addition to considering Y to be only secondary difference to I, it also considers Į to be secondary difference to I (The Wikipedia article makes it sound like I, Į, and Y should all be primary difference). The CLDR data cites Bronius Piesarkas: Lithuanian-English Dictionary ISBN 9986-465-56-7 as a source.

I'm 99% sure that this is due to the 10MB max attribute size limitation. This means that embedded raster images are not allowed to exceed 10MB (after base64). The files in question appear to have large raster images embedded in them

In T387969#10604884, @Lucas_Werkmeister_WMDE wrote:

The screenshot looks like it’s showing UploadWizard; have you tried Special:Upload? As far as I can tell, MediaWiki should show some additional details with this message, but it’s possible UploadWizard doesn’t show them properly.

In addition to that patch, $wgCategoryCollation has to be changed for that wiki prior to running the maintenance script.

Oh, maybe i misunderstood. https://opensource.contentauthenticity.org/docs/verify-known-cert-list/ says there is a hard coded list.

So if I'm reading the linked pages correctly, this system is based on cameras, photo editing tools, and other tools involved in editing the image to cryptographically sign the file in some way. How would we decide which signatures are valid and which are not?

How does this ensure that this doesn't end up promoting proprietary tools as more "trustworthy" than similar free software tools?

As far as I understand this is an open initiative that is run by a non-profit.

A major question here is do we actually validate the sigs or do we just display the value? If we do validate and its invalid, do we still display it or do we pretend it doesnt exist or show a warning of some kind?

Sorry, my bad. I got confused reading the backscroll and thought it was only the parsoid patches that got deployed.

Can users control these things?

In T387130#10589389, @sbassett wrote:

Backport deployments of c1123486 and c1123488 have been successfully completed. Thanks everyone. I know there are several code-hardening patches to follow, but I believe that should get us mitigated in Wikimedia production.

Perhaps XSS should be split up into i18n-xss and other xss because the two have really different risk profiles so its a bit confusing to group the two.

I'm having trouble finding details of the normalization algorithm, but experimentally we can gain confidence:

In T387130#10585603, @Bawolff wrote:

Thinking about this

The requirements are:

• combining slash is sometimes valid, so we cant outright ban it
• < followed by combining slash will almost always be malicious in output because most keyboards output the precomposed form and we also run NFC on all user input
• we cant generically replace combining slash with entity at the normalization stage as we dont know if the output is html or not.

What if on any output normalization (so excluding normalization of user input in WebRequest), we first count the number of precomposed "not greater than" signs, normalize, and then count again. If the number changes we know an attack is happening since we assume at this point the decomposed not greater than is always malicious. At this point we throw an exception or maybe go back to the unnormalized string, replace all combining slash with unicode replacement and try again (its ok to be a little lossy here since we assume this code path only happens during an attack). Thoughts?

If we do strip these characters, i think it is less confusing to the user to replace them with unicode replacement character then to just silently delete.

Yes, I agree that would be better. What I don't know is why we choose to normalize html (or wikitext) to NFC there. Is that a step we actually need?

Thinking about this

In T387130#10582063, @zoe wrote:

I'm concerned that unicode is a changing specification and we're not entirely guaranteed that there won't be some other combining character added that can interact with >.

In T359470#10559798, @taavi wrote:

In T359470#10080343, @Bawolff wrote:

• https://gerrit.wikimedia.org/r/plugins/gitiles/labs/tools/extjsonuploader/+/refs/heads/master/src/PopularityApp.php#48

I don't see how this code could work as graphite.wikimedia.org is behind an auth wall.

Yes, I agree that would be better. What I don't know is why we choose to normalize html (or wikitext) to NFC there. Is that a step we actually need?

For reference, the exception is:

Another PoC:

This is a really cool vuln.

@sbassett I think everything is merged and deployed at this point. Is it ok to make the task public?

I guess all those are using a different version than wikipedia. I believe wikipedia uses CLDR 37.

The CLDR looks correct to me (But of course I don't speak this language). I'd prefer we use the CLDR collation instead of our own as long as it is correct. You can test the CLDR collation at https://icu4c-demos.unicode.org/icu-bin/collation.html making sure the drop down menu on the top left is set to kk (type=standard): Kazakh (Standard Sort Order)

(probably starting with non-current WMF staff, who may be harder to reach)

I think a good solution might be:

FYI, i do intend to turn this into a blog post once it is public

I'd agree that the main mitigations are now merged.

Part of the reason this feels silly to me, is this dataset should be significantly easier to serve than Wikidata.

If you have any example edits where any of those are being used in practice, then that might be helpful to add to the entry? (or add to any related documentation on MediaWiki-wiki, which we could link to?) (Anytime before next Friday)

The change is aimed at (advanced) template editors who use the TemplateStyles feature. It is primarily an accessibility improvement, allowing editors to adjust templates to display differently depending on the users accessibility preferences.

As an aside, we've had lots of problems in the past related to not storing collation version numbers (different versions of libicu have incompatible output). If we are in the process of rearranging this all, it would be cool if we added a solution to that.

Yes, issue is still present as far as i know

No worries. I'm going to mark this task closed.