modrinth / code

The Modrinth monorepo containing all code which powers Modrinth!
https://modrinth.com
Other
904 stars 167 forks source link

Ad script breaks Vue's behaviour and parts of the website #2427

Open brawaru opened 3 weeks ago

brawaru commented 3 weeks ago

Please confirm the following.

What browsers are you seeing the problem on?

Chrome (including Arc, Brave, Opera, Vivaldi), Microsoft Edge, Firefox, Safari

Describe the bug

An ad script from cadmus.script.ac that Modrinth loads overrides the definition of Function in globalThis with custom obfuscation function, which seems to break any Vue components that have property definitions of Function type.

This is because Function is not yet overridden at the moment all the component scripts are loaded in and initialised.

defineComponent({
  props: {
    test: {
      type: Function, // << refers to globalThis.Function
      default: () => alert('Hello, world!'),
    },
  },
})

However, when Vue app starts to hydrate, the Function is already overridden with a custom function e5 from the cadmus script. This doesn't cause any immediate harm, but once a component that has property of Function type being initialised, Vue's property definition type check fails here:

https://github.com/vuejs/core/blob/e075dfad5c7649c6045e3711687ec888e7aa1a39/packages/runtime-core/src/componentProps.ts#L462

Because of this failure, Vue goes into a standard behaviour for resolving defaults, which is to check if the default is a function, and if it is, call it with raw properties to resolve the default value. Thus, Vue completely skips the special behaviour for Function properties, which is to assume the function in default is the default value.

With the example above, the ‘Hello, world!’ alert will be shown immediately when Vue attempts to hydrate the component instead of when it's actually called by the component, which is an unexpected behaviour.

One of the components affected by this is vue-multiselect, which is used in many places throughout the site, the first one — in the projects search to select sorting method. Its default function for customLabel property is supposed to be used in the component's code, not during the component initialisation.

Called during initialisation, that default function returns nonsensical value, which causes an error when the component gets rendered because retuned value is not a function. This leaves only a shell generated by the server, and nothing on a re-render:

Screenshot of ‘Sort by’ select element that is only a shell of unopened ‘Relevance’ select rendered by the server. Next to it ‘Show per page’ select is completely unrendered

Another component affected by this are modals, because they have the onHide property, which default value is an empty function. This function gets called as well, returning undefined, which is then used as the default value, and causes an error when popup close button is clicked, because undefined is not a function (duh). #2423

Steps to reproduce

  1. Don't have an ad blocker, don't have Modrinth Plus, be in region that is served ads.
  2. Open Modrinth website.
  3. Navigate to Mods tab.

Expected behavior

This is pure negligence/malice on the ad script developer's side to override JavaScript built-ins like that. This should never be done, and Function, as well as all other built-ins, should remain intact, preserving the expected environment of the web platform.

Additional context

Supersedes https://github.com/modrinth/code/issues/2205 Clarifies https://github.com/modrinth/code/pull/2358

brawaru commented 3 weeks ago

This issue specifically can probably be worked around by running the following code before the ad script is loaded, which effectively prevents it from overriding globalThis.Function. Kinda cursed.

if (import.meta.client) {
  const f = globalThis.Function
  Object.defineProperty(globalThis, 'Function', {
    configurable: false,
    writeable: false,
    get() { return f },
  })
}
burdoto commented 1 day ago

i'm seeing a similar issue on firefox exclusively, with and without adblock enabled; where since the ui overhaul, basically almost the entire site is completely unresponsive. sometimes a single click works, and then nothing else ever again until i reload the page, rinse and repeat.

this cannot be reproduced in chromium, but is bound to firefox.

brawaru commented 1 day ago

You are likely having completely unrelated issue to this. Clear your browser cache and see if that fixes the issue for you. Otherwise, if that does not help, check the Console tab in Developer Tools (Ctrl+Shift+I or F12) for errors.