Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decode user JavaScript regardless of the existence of whitespace char in it #3944

Closed
wants to merge 1 commit into from

Conversation

mkobayashime
Copy link
Contributor

@mkobayashime mkobayashime commented Oct 26, 2021

This PR is aimed to make user JavaScripts (aka bookmarklets) more stable.

Background

Bookmarklets often contain some URL-unsafe characters like " or whitespace char. They are normally encoded by Chrome when you save it, but this behavior is not very consistent. AFAIK Chrome tends to partially leave these chars as they are, especially with relatively longer URLs. For instance I have my own bookmarklet like this:

javascript:(() => {const enableSelection=()=>{const e=document.body,t=e.parentNode;t.removeChild(e),t.appendChild(e.cloneNode(!0))},writeLyrics=(e,t=!0)=>{if(e)if(console.log(e),t)window.navigator.clipboard.writeText(e);else{const t=document.createElement("textarea");t.textContent=e,document.body.appendChild(t),t.select(),document.execCommand("copy")}},googleSearch=()=>{const e=document.querySelector("div[data-lyricid]");if(!e)return;const t=e.children[1];if(!t)return;const n=Array.from(t.children);return n&&0!==n.length?n.map((e=>e.innerText)).join("\n\n"):void 0},utaNet=()=>{enableSelection();const e=document.getElementById("kashi_area");if(e)return e.innerText};window.location.href.startsWith("https://www.google.com/search")&&writeLyrics(googleSearch(),!1),window.location.href.startsWith("https://www.uta-net.com/song/")&&writeLyrics(utaNet());})()

After this bookmarklet is saved once, the URL is now like this:

javascript:(() => {const enableSelection=()=>{const e=document.body,t=e.parentNode;t.removeChild(e),t.appendChild(e.cloneNode(!0))},writeLyrics=(e,t=!0)=>{if(e)if(console.log(e),t)window.navigator.clipboard.writeText(e);else{const t=document.createElement("textarea");t.textContent=e,document.body.appendChild(t),t.select(),document.execCommand("copy")}},googleSearch=()=>{const e=document.querySelector("div[data-lyricid]");if(!e)return;const t=e.children[1];if(!t)return;const n=Array.from(t.children);return n&&0!==n.length?n.map((e=%3Ee.innerText)).join(%22\n\n%22):void%200},utaNet=()=%3E{enableSelection();const%20e=document.getElementById(%22kashi_area%22);if(e)return%20e.innerText};window.location.href.startsWith(%22https://www.google.com/search%22)&&writeLyrics(googleSearch(),!1),window.location.href.startsWith(%22https://www.uta-net.com/song/%22)&&writeLyrics(utaNet());})()

You can see untouched whitespace char in javascript:(() => {const enab... and " in ...element("textarea");t.tex... while special chars are somehow correctly encoded in the latter half. This should be a buggy behavior of Chrome, but this is what's going on.

Why that's problematic for Vimium

In Vimium's Vomnibar bookmarklets with both whitespace chars and encoded chars (because of the reason described above) are not decoded at all before the execution, because it checks the non-existence of whitespace chars before decodeURIComponent. I'm guessing this is written to test if the URL has encoded chars, but it's in a wrong way. We can safely get rid of the check because decodeURIComponent is gonna change nothing at all if the original string has no special chars.

@mkobayashime
Copy link
Contributor Author

@philc Could you please review this when you can?

@philc
Copy link
Owner

philc commented Jul 23, 2023

Thanks @mkobayashime. I didn't look at this too closely, but your explanation seems reasonable to me. I've merged this in f8c52aa.

@philc philc closed this Jul 23, 2023
@mkobayashime mkobayashime deleted the decode-user-script branch July 25, 2023 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants