I uploaded a patch to update the version, but since it's such a big jump it could possibly introduce breaking changes

Change #1046792 abandoned by Mstyles:

[mediawiki/extensions/3D@master] update three.js version to r165

Reason:

going with r160 version

https://gerrit.wikimedia.org/r/1046792

Change #1047181 had a related patch set uploaded (by Mstyles; author: Mstyles):

[mediawiki/extensions/3D@master] update three.js version to r160

https://gerrit.wikimedia.org/r/1047181

Change #1047181 abandoned by Mstyles:

[mediawiki/extensions/3D@master] update three.js version to r160

Reason:

several other upgrade patches already exist

https://gerrit.wikimedia.org/r/1047181

@bvibber 's patch that we should likely work from going forward: https://gerrit.wikimedia.org/r/1026883

(and an even older attempt: https://gerrit.wikimedia.org/r/955983 by @Reedy)

I'll rework the patch this afternoon to update without breaking, hopefully ;)

Do you have a proof of concept exploit? I'm not sure this is exploitable at all.

First, it's a resource-consumption vulnerability (in that parsing very long rgb color sequences may take a long time) and this would happen either in the browser, after click-through (browser will prompt you to abort the JS if it takes too long, or you can just reload the page) or in node on the thumbnail handler (where it would be isolated to the thumbnail handler process and should eventually get timed out safely).

Further, we only allow STL files and they use a binary format for colors, not the text string parser.

We do not have a proof of concept exploit, but it sounds like the risk would be very low. I still think an upgrade would be worthwhile even if the exploitation risk is low, but if it’s going to be too difficult to rework for an upgrade, we can shelve it for now

Still worth updating it I agree, on general principle I don't like having very old libraries running live. :) I'll see if I can get it cleaned up in a timely fashion...

@bvibber I wanted to check in on this ticket. If you don't have bandwidth for this, that's fine. It would be nice to have some kind of fix even if it's a version update

No activity, but still on my board... Why don't I schedule some tech debt time for first week of September to finish this up. :)

(now on my schedule for Friday September 6 and Friday September 13 all day)

@bvibber any updates on finishing this?

Got this scheduled on my board for today, updates hopefully this afternoon.

Well, this is unfortunate: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/3D/+/1026883/3#message-5a7b353498be52feca8b9937f871c5550d8e178c

Since this extension doesn't have the that many maintainers, and this security issue has been open for almost 6 months, the security team is considering disabling this extension in WMF production.

Any known statement from Structured Data as code stewards (as I cannot find any in this task)?

I have now got this working based on three.js 162, and Donmccurdy has made the light shine again.
Some adjustments had to be made and I will try to apply a patch today.

Why version 162: @donmccurdy
Note that three.js v163 removed support for WebGL 1, but 3d2png depends on headless-gl, which only supports WebGL 1. Assuming 3D and 3D2PNG should use the same three.js versions for consistency, perhaps it's best to stick with three.js v162 in both, for now?
The version could be upgraded past v162 after either (a) headless-gl supports WebGL 2, or (b) three.js WebGPU support is stable and a stable Node.js WebGPU implementation exists.

@OpenDEM when you have the patch up, please post here.

@OpenDEM following up on patch status. To be clear this can go through gerrit.

Change #1098815 had a related patch set uploaded (by OpenDEM; author: OpenDEM):

[mediawiki/extensions/3D@master] update three.js r162

https://gerrit.wikimedia.org/r/1098815

@bvibber waiting on code review from you to get this patch merged

Lighting seems to have regressed a bit:

Before	After

@Esanders can you please add @donmccurdy to this task? He did the lightning system.

@Esanders : Can you please send me the URL of the STL file?

It's done, I've found it: https://de.m.wikipedia.org/wiki/Datei:Asad_Al-Lat.stl

@donmccurdy @OpenDEM any updates on gerrit? It looks like the gerrit patch is much closer to being merged

I am waiting for feedback from @donmccurdy. But I think we should stick with the new standard lighting system, even if it doesn't look quite as aesthetically pleasing in some cases.

@OpenDEM I saw three comments from @donmccurdy that you didn't respond to starting here. Are you waiting for any other feedback?

Yes, I have created a prototype here with the suggested changes: https://opendem.github.io/WikiMediaExtension3D_Test_GLB_Format/stl_shadow.html
But I can't see any difference to before.
I'm also working on a patch for 3d2png (T345953) and realised that the RoomEnvironment lighting system used here doesn't work headless. Therefore, I would like to wait for feedback here.

As the changes have had no visible effect, the standard lighting system is to be retained. Even if this no longer looks as aesthetically pleasing as it used to in some cases. The new lighting system was introduced with three.js r155.

@Esanders: Are there still points that prevent a merge?

@OpenDEM @Esanders any way we can make progress on this?

If I can do something here please let me know.

@OpenDEM For the comments on the patch, you should click the "done" button for the ones you implemented, and hit the reply button for the ones you believe should not be done with a reason why on the patch (And then click the "reply" button on the overall patch to save comments. The UI is a bit confusing). This will mark the comments resolved, which will make the patch show up to potential reviewers as needing to be reviewed.

Thank you. I hope this fits now.

We want to move forward with merging this patch next week if there are no other comments since the security concerns are addressed in this patch

That sounds very good. I have just commited a patch for 3D2PNG: https://gerrit.wikimedia.org/r/c/3d2png/+/1114772

@OpenDEM Do we need to merge https://gerrit.wikimedia.org/r/c/mediawiki/extensions/3D/+/1098815 and https://gerrit.wikimedia.org/r/c/3d2png/+/1114772 at the same time? I don't have merge permissions for 3d2png, so that patch would need to be merged by @bvibber or someone else with perms

That is not a problem. There will only be differences in the display. The new realistic lighting system no longer looks as arty as before.

In T367677#10578524, @OpenDEM wrote:

That is not a problem. There will only be differences in the display. The new realistic lighting system no longer looks as arty as before.

Just to be clear on @Mstyles ' question above: should we merge the 3d2png change set first? i.e. is it necessary to merge the 3d2png change set first? Or can that reasonably be done at some later time?

Excited about this! Carving out a little time each week to push on getting this done. :) Putting the 3D and 3d2png patch review on my plate for Thursday, I'll add some notes and test it and if it's clean enough, merge.

I think if we're just cleaning up the library versioning and lighting model it doesn't super matter about pushing both at the same time; we'll have old renderings cached I imagine for a while in either case. 3d2png will handle renderings done via thumbnails, and 3D will handle on-web renderings, and as long as they look similar I think we're good.

As I understand, my current plan is:

• review these partial updates to three.js in 3D and 3d2png
• once i'm happy with them, deploy them so we've got a clean slate on the old CVE
• i'll provide further advice and review on gltf2png and addition of gltf support as community work happens
• note that community wishlist is a good opportunity to look for getting additional development resources like dev contractor time to help push these things towards production!

@bvibber sounds like a plan, I'll follow up on Friday. It would be great to have development resources for this extension in the future.

@bvibber Thanks!

• note that community wishlist is a good opportunity to look for getting additional development resources like dev contractor time to help push these things towards production!

We had already placed a wish there in the november, which now needs to be revised:
https://meta.wikimedia.org/wiki/Community_Wishlist/Wishes/Support_full_colour_3D_models_on_Wikimedia_projects

Finally got a chance to poke over the patch and get it running in my test environment. Good news: it's close! Bad news: slight regression on File: page which is easy to fix (yay!) and there _something_ weird about the lighting setup I'm still not happy with. I seem to end up with objects overexposed from the environment map, and no light source so no shadows.

This is probably something I can work out with a couple hours poking at the three.js docs from here -- I'm going to finish this up during travel downtime next week as I'll have plenty of extra time and my work laptop. :D

Should be able to merge it after a lighting adjustment & fixing the File: page bit.

thanks for the update @bvibber, looking forward to the changes

If I restore the original lighting and simply up the light brightnesses to match the new behavior I end up with some odd funkiness with weird corruption with the shadow map:

Not quite sure why yet; I may just remove the shadow and adjust the lighting to match the old view better. :) *sigh*

The artifact above is usually resolved by configuring shadow bias. A small value, usually negative, tends to work best. Like:

light.shadow.bias = -0.0001;

Not uncommon with this shadow mapping technique, though I'm not sure why it would have changed with a three.js version bump if the lighting is otherwise the same.

In T367677#10730620, @donmccurdy wrote:

The artifact above is usually resolved by configuring shadow bias. A small value, usually negative, tends to work best. Like:

light.shadow.bias = -0.0001;

Not uncommon with this shadow mapping technique, though I'm not sure why it would have changed with a three.js version bump if the lighting is otherwise the same.

woot! I suspect something in the units handling of lighting must've changed it. :) Looks like we're back in business with that tweaked. :)

Woohoo! Latest rev of the patch now approximately matches the old behavior, with slight 'headlight' casting no shadow and brighter left/up light which does cast shadow. I'm happy with this and expect to merge shortly.

Change #1098815 merged by jenkins-bot:

[mediawiki/extensions/3D@master] update three.js r162

https://gerrit.wikimedia.org/r/1098815

Latest patch is merged! Approximately reproduces the previous lighting but updated to three.js r162, and more future-proof to further upgrades. It should go out with the MediaWiki update train.... I think it might be too late for next week but the week after definitely.

Huge thanks to @OpenDEM and @donmccurdy for helping us finally get this fixed up and get our scene setup modernized for the rendering changes. :) I hope we can make the next steps a little faster and get moving towards further improvements over the next months!

Change #1135821 had a related patch set uploaded (by Reedy; author: OpenDEM):

[mediawiki/extensions/3D@REL1_43] update three.js r162

https://gerrit.wikimedia.org/r/1135821

Change #1135822 had a related patch set uploaded (by Reedy; author: OpenDEM):

[mediawiki/extensions/3D@REL1_42] update three.js r162

https://gerrit.wikimedia.org/r/1135822

Change #1135823 had a related patch set uploaded (by Reedy; author: OpenDEM):

[mediawiki/extensions/3D@REL1_39] update three.js r162

https://gerrit.wikimedia.org/r/1135823

Excellent @bvibber thank you so much! Can we call the work on this done now?

Thanks a lot @bvibber!

Thanks so much @bvibber! :D

Now would be a great time to do T355822: Add a foreign-resources.yaml file to 3D...

Couple of other things.. Beyond my comment above.

Why when we've upgraded to a specific version, have we got a semver constraint against the version? https://gerrit.wikimedia.org/r/c/mediawiki/extensions/3D/+/1098815/6/package.json ^0.162.0 should probably be 0.162.0

I've started backports... REL1_43 (important as it is an LTS) just needs the package-lock.json regenerating.

As per https://www.mediawiki.org/wiki/Version_lifecycle, REL1_42 is due to be EOL in ~2.5 months, so effort to upgrade that may not be worthwhile...

REL1_39 has got till the end November 2025... Would be nice to have, but the effort again may not be worthwhile.

Change #1136128 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/3D@master] package.json: Explicitly pin three to 0.162.0 (remove ^)

https://gerrit.wikimedia.org/r/1136128

Change #1136129 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/3D@REL1_43] package.json: Explicitly pin three to 0.162.0 (remove ^)

https://gerrit.wikimedia.org/r/1136129

Change #1135821 merged by jenkins-bot:

[mediawiki/extensions/3D@REL1_43] update three.js r162

https://gerrit.wikimedia.org/r/1135821

Change #1136128 merged by jenkins-bot:

[mediawiki/extensions/3D@master] package.json: Explicitly pin three to 0.162.0 (remove ^)

https://gerrit.wikimedia.org/r/1136128

Change #1136129 merged by jenkins-bot:

[mediawiki/extensions/3D@REL1_43] package.json: Explicitly pin three to 0.162.0 (remove ^)

https://gerrit.wikimedia.org/r/1136129

Once we get the backport merged for 1_39, we want to make this ticket public. Any objections?

Still working on backports for 1.39 and 1.42

In T367677#10762034, @Mstyles wrote:

Still working on backports for 1.39 and 1.42

1.42 should be ready now. 1.39 still has some testing issues, see comment here.

Change #1135822 merged by jenkins-bot:

[mediawiki/extensions/3D@REL1_42] update three.js r162

https://gerrit.wikimedia.org/r/1135822

@Reedy for the testing issues with 1.39, would you recommend just merging as is with the tests failing or taking a different approach?

Change #1135823 abandoned by SBassett:

[mediawiki/extensions/3D@REL1_39] update three.js r162

Reason:

The test issues are proving tricky to the point of perhaps not being feasible to resolve. And REL1_39 will be unsupported in 4 months. I'd also guess the number of operators of REL1_39 MediaWiki _with_ ext:3D are countable on, at most, one hand.

https://gerrit.wikimedia.org/r/1135823