obsproject / obs-browser

CEF-based OBS Studio browser plugin
GNU General Public License v2.0
771 stars 218 forks source link

Special characters seem to affect getScenes and getTransitions #390

Closed juaoose closed 10 months ago

juaoose commented 1 year ago

Operating System Info

Windows 11

Other OS

No response

OBS Studio Version

29.0.1

OBS Studio Version (Other)

No response

OBS Studio Log URL

https://obsproject.com/logs/BdNarNPBbQaO1YOz

OBS Studio Crash Log URL

No response

Expected Behavior

Whenever using window.obsstudio.getScenes or window.obsstudio.getTransitions in OBS, I expect to receive the list of scenes or transitions available.

Current Behavior

If one uses window.obsstudio.getScenes or window.obsstudio.getTransitions whenever a scene with special characters such as ', ", \ the callback passed to the functions is not invoked.

Steps to Reproduce

  1. Start OBS with remote debugging $ ./obs64.exe --remote-debugging-port=9222
  2. In any scene, create a browser source with Page permissions 'Read access to user information'
  3. Check for the list of scenes from the inspector (http://localhost:9222/) by running window.obsstudio.getScenes((a) => {console.log(a)}) for the previously created browser source. The callback will be invoked > ['OverlayTest', 'Webcam', 'Desktop', 'Demo', 'Other']
  4. Create a scene with one of these characters: ' , ", \
  5. Perform step 3 again, this time the callback will not be invoked.

The same happens for transitions getTransitions

Anything else we should know?

No response

WizardCM commented 1 year ago

I did a fair bit of digging on this today.

Our code just runs a janky JSON.parse() on a string (within the V8 context), with a strong assumption that the string contains a valid JSON string. However, this isn't always the case.

It seems the correct way to do this is to parse the data directly, rather than throwing it into V8. Unfortunately, this means converting from a CefValue to a CefV8Value, which there is no built-in utility function for.

I was successfully able to implement the suggestion from this forum post, and handle the data directly in CEF before letting V8 convert it into a JS value properly. I can confirm this code snippet fixes the issue. I'll submit a PR in the coming days.