Issue 87 - Javascript Injection to Browser component

[kuhnchris]

Hey there!

I felt the urge to fix this issue, as I was having some ideas regarding what to display (for example just a certain issue in GitHub) on my stream. So I added this very rudimentary version.
It compiled fine and works just as expected, with following example:
SpiegelEiXXL-WGJ-Creations/WGJ64-DungeonCrawiching#6
+
document.body.innerHTML = document.getElementsByClassName("unminimized-comment")[0].outerHTML;

Closes #87

=>

You DEFINITLY have to watch the console tho, since javascript errors will be reported just there, any disappear in the everlasting log of messages:

If you need anything else regarding this issue, let me know!

[kuhnchris]

For quick tests, here's the tested against DMG for MacOS.
obs-studio-x64-22.0.2-168-g047e586e-modified.dmg.zip

[RytoEX]

Hi! Thanks for submitting your first OBS Project PR. To help out, I've reviewed this PR.

In addition to my inline comments, please squash your commits into a single commit, since the second and third commits are essentially fixup commits to the first. Please also edit your final commit's message to start with a present tense verb and remove the parenthetical phrase. If you feel it's important to reference the GitHub issue in the commit message, I recommend placing that mention in the commit message body rather than its subject.

[kuhnchris]

Thanks for checking my source changes! I made your suggested correction, but I miserably fail at squashing the commit (with rebase and/or merge). Anything I need to watch out?

Thanks!

[RytoEX]

If you're working from the git commandline, and this repo is set as "upstream" and your fork is set as "origin" you'd do something like this:

git checkout master
git fetch upstream
git rebase -i upstream/master

Your default text editor may pop up here with a temporary file. For the first commit, change "pick" to "reword" (the first word of the line). For the following commits, change "pick" to "fixup". Save the file, and close the text editor.

Your default text editor may pop up again here with another temporary file. Edit the commit message to be "Add JavaScript injection". Save the file, and close the text editor.

Git should wrap up its rebase work after this, and then you should be able to force push to your branch with the following:

git push -f origin

Let us know if you have any issues.

[RytoEX]

If you're concerned about making a mistake, you can always make a backup of your branch locally with this before starting your rebase:

git checkout master
git checkout -b javascript-injection
git checkout master

[kuhnchris]

Hello,

thanks so much again for your input - i squashed the commit, so it's 1 commit now.

All the best,
Chris

[DDRBoxman]

Can we make the default injected javascript some commented out warning about pasting random code?

Discord's console has a pretty good warning on it. I just want to make sure that users don't get cookies, etc. stolen

[WizardCM]

I'd also vote to make the JS input disabled by default, and require the user tick a box and accept a warning, if possible.

[kuhnchris]

If that's the case we need to do the same for the "css" aswell, as that one works by injecting javascript to "load" the custom css into the webpage. If that's fine by you guys I will add 2 checkboxes for that ("I understand the risks of injecting (css or javascript) into the website above and know what I am doing").

[kuhnchris]

Added your request(s). Please add a working description to the translation(s) so you guys feel safer. ;-)

[OsirisNL]

If that's the case we need to do the same for the "css" aswell, as that one works by injecting javascript to "load" the custom css into the webpage. If that's fine by you guys I will add 2 checkboxes for that ("I understand the risks of injecting (css or javascript) into the website above and know what I am doing").

The CSS option does not inject Javascript, it executes specific javascript code to create a link tag containg the CSS as base64 data (the actual CSS is injected). So this is not remotely as dangerous as injecting arbitrary javascript code. A checkbox for the CSS field is therefore not needed.

[kuhnchris]

Well, I did get some problems regarding Cross-Site-Scripting (edit: it's actually the Content Security Policy, sorry for the confusion) when I tried to interfere with the GitHub link, hence my thinking of adding a box there aswell, but, if you do not want to, I can remove that box again, no problemo.

info: obs-browser: Refused to load the stylesheet 'data:text/css;charset=utf-8;base64,Ym9keSB7IGJhY2tncm91bmQtY29sb3I6IHJnYmEoMCwgMCwgMCwgMCk7IG1hcmdpbjogMHB4IGF1dG87IG92ZXJmbG93OiBoaWRkZW47IH0=' because it violates the following Content Security Policy directive: "style-src 'unsafe-inline' assets-cdn.github.com".
 (source: data:text/css;charset=utf-8;base64,Ym9keSB7IGJhY2tncm91bmQtY29sb3I6IHJnYmEoMCwgMCwgMCwgMCk7IG1hcmdpbjogMHB4IGF1dG87IG92ZXJmbG93OiBoaWRkZW47IH0=:1)
[1009/093347.965961:INFO:CONSOLE(1)] "Refused to load the stylesheet 'data:text/css;charset=utf-8;base64,Ym9keSB7IGJhY2tncm91bmQtY29sb3I6IHJnYmEoMCwgMCwgMCwgMCk7IG1hcmdpbjogMHB4IGF1dG87IG92ZXJmbG93OiBoaWRkZW47IH0=' because it violates the following Content Security Policy directive: "style-src 'unsafe-inline' assets-cdn.github.com".
", source: data:text/css;charset=utf-8;base64,Ym9keSB7IGJhY2tncm91bmQtY29sb3I6IHJnYmEoMCwgMCwgMCwgMCk7IG1hcmdpbjogMHB4IGF1dG87IG92ZXJmbG93OiBoaWRkZW47IH0= (1)

[OsirisNL]

That's just Chromium not allowing the css because the site's Content Security Policy doesn't accept the data: scheme as a source for css.
Though that might be circumvented by adding it as a <style> tag, but that's another issue.

[RytoEX]

I've gone over this again. Please see my inline comments. It looks like you're continually changing tabs into spaces. Your IDE may be doing this automatically for you. Please keep an eye out for these whitespace conversions, as they do not follow the project's style preferences.

The commit message still uses past tense verbs. It should be using present tense verbs (see the commit history for examples). It also still refers to "OBS-Browser", but it shouldn't need to since that is this project. It also still refers to GitHub Issue #83 rather than GitHub Issue #87.

This is how the commit message should look:

Add custom JavaScript injection

Add custom JavaScript injection, solving GitHub Issue #87. Requires a 
user to confirm they want to inject JavaScript via a confirmation 
checkbox.

[Enoxs]

What about windows platform?

[OsirisNL]

@Enoxs what do you mean?

[kuhnchris]

You mean a pre-built binary for windows @Enoxs ? If yes, you will have to wait for the next official release and for this commit to be merged into master.
I sadly do not have a Windows developer machine on hand so I cannot compile a version for you to test right now.

[kuhnchris]

@RytoEX thanks again for checking, I checked in the changes you mentioned, checked against vim this time, so the tabbing should be better...
Also changed the commit text. If that's better i'll resquash and use a present tense commit text.
Also, do you need the string(s) added to the translation files?

[RytoEX]

Another review. Please note, I did not mark every single line where an issue occurs. Please apply my requested changes throughout your PR.

Yes, you will need to add appropriate lang strings for the two strings that you've added. You only have to add them for English (en-US). Translations are done via Crowdin.

[kuhnchris]

Once again, thanks! One question: is there any implicit limit to the number of characters for the texts in the plugin configuration dialog? I started out with the following text, but I think the length seems unpracticable.

[Voldi87]

Lame question, but how can I open the Javascript box?

[kuhnchris]

Hey there @Voldi87 - the commit isnt productive yet, so sadly you'd have to compile it yourself. The "open" the settings box you simply configure the browser component

[Voldi87]

Hi, I don't exactly understand this. I've downloaded it and installed it for windows 64bit from releases menu (v1.31). And then what should I do? KuhnChris <[email protected]> ezt írta (időpont: 2019. ápr. 17., Sze 18:26):

…

Hey there @Voldi87 <https://github.com/Voldi87> - the commit isnt productive yet, so sadly you'd have to compile it yourself. The "open" the settings box you simply configure the browser component — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#137 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AvZK3SGEUk-NLqZiTX2o5eG-neQFKo6Lks5vh0szgaJpZM4XLjuC> .

[kuhnchris]

Hi, I don't exactly understand this. I've downloaded it and installed it for windows 64bit from releases menu (v1.31). And then what should I do? KuhnChris [email protected] ezt írta (időpont: 2019. ápr. 17., Sze 18:26):
…
Hey there @Voldi87 https://github.com/Voldi87 - the commit isnt productive yet, so sadly you'd have to compile it yourself. The "open" the settings box you simply configure the browser component — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#137 (comment)>, or mute the thread https://github.com/notifications/unsubscribe-auth/AvZK3SGEUk-NLqZiTX2o5eG-neQFKo6Lks5vh0szgaJpZM4XLjuC .

As I stated: This is not merged yet.
If you downloaded the official release of OBS, then you cannot use this change yet.

[chrisjameschamp]

When is this going to be merged? This would solve a gang load of my problems

[kuhnchris]

@semaj4712 i think I left a question for @RytoEX but I haven't got a reply (regarding the length of the text). Anyways, I am not a member of obs-browser so I sadly cannot tell you anything about a time-line when/how/if this is going to get merged. :-(

@RytoEX if I can help in any way, let me know

[chrisjameschamp]

Would it not be possible implement this as a third party plugin?

[kuhnchris]

Well, you can always fork this and compile it yourself, and replace obs-browser with it, if you need it "quickly", but in my opinion it wouldn't make sense to split a component off just because of an additional functionality.

You can, however, go ahead and do that if you want. The code is up there if you want to try. :-)

[dodgepong]

my streamkit could be improved without me using Fiddler and modifying the code directly

Why would you want to type Javascript into the browser source properties when you have the ability to modify code directly? Why do you need to use Fiddler?

[WizardCM]

Hi there, while DDRBoxman was pushing new commits, it seems the bot that you run on your repo bugged out when rebasing, so it deleted your changes. Unfortunately, this means GitHub thought this PR was merged (because the commit hash matched) and automatically closed the PR.

You'll have to revert your branch, rebase manually, then submit a new PR.

[kuhnchris]

Hi there, yeah I noticed. At this point I however question if I should reopen the PR at all as the original one was dated back in 2018. If we still want to have a discussion about this let me know, else I'll just be done with it for now. (we can also move this to Discord if you want)

And yes, the bot accidently force-pushed so I'd have to rebase my local copy again, I just uninstalled it so this shouldn't happen again.

Thanks,
Chris

[ryantheleach]

Why would you want to type Javascript into the browser source properties when you have the ability to modify code directly? > Why do you need to use Fiddler?

Dodgepong, you are making the assumption that the browser source code is a file that the user controls directly. Injecting javascript is basically what the browser extensions greasemonkey/tampermonkey were built for, so it shows there is a usecase.

[dodgepong]

Dodgepong, you are making the assumption that the browser source code is a file that the user controls directly

The comment I was replying to seems to have been deleted, but if I recall correctly it implied that their code was local, or could have been read that way.

[ryantheleach]

Sorry Dodgepong, I assumed that it was an earlier edit of: #137 (comment)

[Codingale]

The comment I was replying to seems to have been deleted, but if I recall correctly it implied that their code was local, or could have been read that way.

Ah, I see you replied to my comment, I had deleted it since it was basically the exact same as #137 but phrased worse, sorry if I implied I could modify it without using Fiddler or similar MITM proxy.

Didn't mean to delete a comment that someone had replied to, I had woke up after a long day and re-read the comments and saw that someone already had my use case in mind. But didn't reload to see if someone replied.

Basically the code for StreamKit can be modified by users to make it more efficient. I recall a video tutorial that uses multiple browser sources for something that could be done with one JS tweak but no option exists for it. Here's the video

I just want to reaffirm I'd really like this feature as it'd enable more creativity in otherwise limited overlays without having to trust a MITM certificate or some other 3rd party means that could expose you in other ways.

[TeamSteeve]

Went looking for this exact feature. It would be so handy if I could insert some javascript that would fill the log-in page details of my nest cams (which I very much do want to be password protected).

Any chance of this making it into an OBS release?

[schneidermanuel]

Looking for this. Is there a simple way to get this working on Windows?

[willlllllio]

Would be great if this ever got merged.

There are often times where streamers I know use some page as browser source and everything is mostly working except one or two tiny little things being wrong that could easily be patched with 5 lines of JS but is impossible to do within OBS. Some people here recommend local files or servers but that's usually not applicable for modifying 3rd party sites.

Examples:

• an alerts page that had one alert way way louder than others without a way to configure it
• a really great countdown timer page that required manual user interaction to start the timer every time
• a script to block loading avatar images on a leaderboard/stats overlay page for a game that allows users to set their own avatar image, leading to some setting lewd/obscene/racist pictures to catch out streamers and forcing them to not use said overlay at all
• a page that wouldn't load without a chrome only API being there on startup because of a bug in a feature detection function throwing an error on page load when run in CEF

All fixed with a couple lines of JS that you currently have to run in a full browser via a Userscript and capture in OBS instead of just throwing a couple lines of JS in the browser source properties patching those things.

Regarding security concerns, I fully get the possible dangers of it but they seem a bit overstated here. Even all major browsers let everyone open a JS console and just run any code the users pastes with maybe a warning in some of them, and have tons of extensions allowing the customization of loaded pages in various ways. The idea that any amount of hacking around on pages like this is bad™ and should be discouraged and everything being locked down and walled gardened for "user safety" seems kinda saddening to me especially in an open source project.

Regarding implementation, it would be great if it used OnLoadStart for JS injection instead of OnLoadEnd used for CSS as OnLoadStart would more easily allow overwriting browser provided APIs like for example Audio. It doesn't make much difference for CSS but for JS if you only run the code on OnLoadEnd it will often be too late already as the page's scripts are already loaded and ran and many scripts seem to get references for things like browser APIs just once on load and then keep using those forever, seems pretty common especially in bundled SPAs now. SoOnLoadStart seems nicer for injecting JS just to ensure you're running before anything else. Anyone wanting to run after load can still easily do that with the event this way too. I tried it locally and seemed to work without issues, only a couple lines change needed.

I would love seeing this merged eventually if possible. I would also be down to make any changes needed and make a new PR if that's needed to get it merged just and only in case @kuhnchris has no interest in doing that anymore. (I have no intention of stealing credit here, sorry if that comes of like it, would just really like to see this merged eventually and just offering only in case the original author isn't interested anymore).

[Codingale]

Would be great if this ever got merged.

#137 (comment)

Just waiting on a new build of OBS I thought for public release?

[dodgepong]

Nope, that was a Github bug: #137 (comment)

[Codingale]

Ah, didn't get that email and didn't look closer thanks.

[kuhnchris]

Yeah I screwed up merging the recent master into my branch, hence the error.
I talked to the guys over at Discord and there is a very good reason why this is still halted, I do get their basic intention behind not merging it in, tho it takes a very long time and there still isn't a timeline announced regarding the change they are holding this back. Just to address @willlllllio - this would have been just a regular javascript injection into the CEF (Chrome Embedded Framework) - basically executing javascript after the page has been loaded. The implementation still works with the recent implementation of obs-browser, so if you check out the fork and compile the module yourself it works just as intended, if you need any changes on that you can let me know. I just cannot provide binaries (dlls or so) for this.

[willlllllio]

@kuhnchris yeah I did compile it locally and tested it, the problem I had was that the JS gets executed too late to do some things which is what I was referring to.

loading order seems to go:

1. CefLoadHandler::OnLoadStart
2. page loading and main page JS executing already
3. CefLoadHandler::OnLoadEnd

So when injecting the JS in OnLoadEnd the page's own JS will already have ran before the injected code at that point, which makes doing some things with the injected code impossible. For example replacing the browser Audio API with a modified version won't work if the page's code already got a reference to the real Audio object in its initial loading phase.

Injecting the JS earlier in CefLoadHandler::OnLoadStart solved that for me, as it guarantees the injected code runs before any actual page code. Seems overall cleaner and anyone that wants to run their injected JS only after load finished can still easily do that via the documented loaded event just via normal JS.

https://magpcss.org/ceforum/apidocs3/projects/(default)/CefLoadHandler.html

[kuhnchris]

technically the current implementation is ran "OnLoadEnd".
https://github.com/kuhnchris/obs-browser/blob/53cfefe74a2347e9054212bb4c014766e53ee5f4/browser-client.cpp#L442
We could add the other 2 CEF Load Handlers too, shouldn't be that big of an issue - question rather is: should those be separate javascript "boxes" or should you just have 1 box and a radio button group with 2 buttons selecting between Injecting at "Start" or "End"?

[willlllllio]

I don't think there's any need for any special UI for that, always injecting the JS in OnLoadStart should be enough. Anyone that wants to only run their code once loaded can just wrap it in window.addEventListener('load', func) to get the same behavior as OnLoadEnd just via normal JS .

[reversefold]

Was this merged or not? I'm not seeing the Javascript injection option in browser source properties when building from master.

[RytoEX]

Was this merged or not? I'm not seeing the Javascript injection option in browser source properties when building from master.

No. GitHub's UI showing that the PR was merged is a bug. See:
#137 (comment)

[kristoff-it]

Could we now please merge this PR for real, if only just to spite Streamlabs?

I also wouldn't mind it for running my own overlays, but eroding the feature disparity with Streamlabs (which implements this already) would be the cherry on top.

[kuhnchris]

This depends on the timeline of obs-browser. I talked to the guys in Discord - basically I am waiting for an "OK" or "go" from their side to re-implement this change. No idea how far the timeline fell apart with all the virus thing going on. 🤷

[Fenrirthviti]

Most of the discussion regarding our upgrade plan and progress can be found here: obsproject/obs-studio#3853

This is the most pressing task for Browser Source stuff, after which we'll be revisiting proposed changes and other updates.

[lacksfish]

@kuhnchris I'd also be very interested in this feature. I need it to edit names in the Streamkit Discord overlay.

[rogueyoshi]

@lacksfish ghetto css example to replace names. use the person's avatar image as a selector since user-id doesn't seem to be exposed in the frontend. you can get more accurate styles if you copy them from streamkit, i was just lazy.

.avatar ~ .user .name {
    position: absolute;
    visibility: collapse;
}

.avatar ~ .user .name:before {
    position: relative;
    top: -4px;
    left: -6px;
    visibility: visible;
    display: inline;
    background: rgba(30, 33, 36, 0.95);
    padding: 0.2rem 0.4rem;
    border-radius: 0.20rem;
}

.avatar[src="https://cdn.discordapp.com/avatars/157643090700664833/5d733b3f84ae07796fd2c356bbef6e70.jpg"] ~ .user .name:before {
    content: "changeme";
}

[GStefanowich]

since user-id doesn't seem to be exposed in the frontend

User IDs are exposed in the front-end, just sort of convoluted. They're visible in the data-reactid tag, mine is "180106401924775936"

data-reactid=".0.0.0.$180106401924775936/=1$180106401924775936"

Which can be targeted in CSS using the *= selector

.voice-state[data-reactid*="180106401924775936"] {
	some-css-rules: here;
}

[platinumazure]

Is there a successor to this PR? I don't see an obvious link from the comment history but I wanted to double check. Thanks!

(I understand the "merged" status is a glitch in this case.)

Edit: I missed #354, apologies.