feat(Spotify): Add Sanitize sharing links patch

[drobotk]

Removes user tracking data from shared urls.

For example, https://open.spotify.com/track/0gJvyZ5a4d2UQd2ZFWbiGE?si=YCFUrazkSNqfK76YgbpxpA

Then becomes:
https://open.spotify.com/track/0gJvyZ5a4d2UQd2ZFWbiGE

[kitadai31]

Actually, @anddea had already added a similar patch to his RVX fork.
anddea/revanced-patches@dfd7461
This might be helpful.

Your patch is cleaner in the patches side, but about the extensions code, anddea version is better.
His implementation only uses String, so it will never fail.
I think you should borrow his extensions code.

I think the patch description is also anddea version is better.
It clearly explains what the patch does as a result.

(I wonder why he added the Spotify patches to his RVX fork instead of PRing to official ReVanced alongside with the official ReVanced's Unlock Premium patch 🤔)

[oSumAtrIX]

The extension code from this PR follows URL specs. This is correct. Arbitrary string manipulation is not recommended

[oSumAtrIX]

I wrote this on discord already, but there's a couple of sanitize links patched that may be abstractable into a generic sanitize link patch for code deduplication

[Brosssh]

Actually, @anddea had already added a similar patch to his RVX fork. anddea/revanced-patches@dfd7461 This might be helpful.

Your patch is cleaner in the patches side, but about the extensions code, anddea version is better. His implementation only uses String, so it will never fail. I think you should borrow his extensions code.

I think the patch description is also anddea version is better. It clearly explains what the patch does.

(I wonder why he added the Spotify patches to his RVX fork instead of PRing to official ReVanced alongside with the official ReVanced's Unlock Premium patch 🤔)

The extension method done by anddea is basically wrong tho.. it's simply returning everything before ?si=. The problem is Spotify sometimes actually add additional context to a shareable link (for instance in which playlist is the song that's being shared). That should not be removed from the shareable URL (in my opinion at least).

[kitadai31]

The problem is Spotify sometimes actually add additional context to a shareable link

I didn't know, sorry
That was a serious issue... Thank you

[anddea]

it's simply returning everything before ?si=

That's true, I hadn't considered that aspect. But I've never come across links like that. Do you have any examples?

patch is cleaner

I can simplify my patch. I overcomplicated the fingerprint and invokeDirectIndex during testing to match more cases and ensure I hit the right spot on the first try. I just never simplified it afterward because there wasn’t really a reason to, but yeah, it can be drastically simplified. I even have both the fingerprint and the patch in a single file, which might be considered as not the best practice by some people.

But why are there two instructions, though? I’ve been using just one, and I haven’t run into any issues so far. Am I missing something?

(I wonder why he added the Spotify patches to his RVX fork instead of PRing to official ReVanced alongside with the official ReVanced's Unlock Premium patch 🤔)

I don't mind if anyone moves all my patches here or even rewrites it in a better way. I just don't want to make the same thing twice, I experiment in my repo where I can push immediately.

[drobotk]

Can this be merged for now, or should I convert this to a generic patch still in this same PR?

[LisoUseInAIKyrios]

What's here is ok.

[Brosssh]

it's simply returning everything before ?si=

That's true, I hadn't considered that aspect. But I've never come across links like that. Do you have any examples?

https://open.spotify.com/track/6H6UoUKz8OVaLXqQbZgbPK?si=WHmwIx3tTVOrwfeWGFn6EQ&context=spotify%3Aalbum%3A3bwpDqNK4LGNW9ZkfltZ6I
This for example. I just pressed share for a song inside its album.

[anddea]

&context=spotify%3Aalbum%3A3bwpDqNK4LGNW9ZkfltZ6I

Not sure if this part is really necessary, spamming the URL just makes it even longer (since a Spotify link already opens the album directly). But I'll merge these changes into my repo anyway.

Genuine question though, why inject two separate calls to sanitizeUrl? Even though the end user won't notice any big impact, creating the Uri, Uri.Builder, and other objects is already less performant than raw string manipulation, and here it's doing all that twice. Seems like overhead that should be avoided if possible.

[Nuckyz]

&context=spotify%3Aalbum%3A3bwpDqNK4LGNW9ZkfltZ6I

Not sure if this part is really necessary, spamming the URL just makes it even longer (since a Spotify link already opens the album directly). But I'll merge these changes into my repo anyway.

Genuine question though, why inject two separate calls to sanitizeUrl? Even though the end user won't notice any big impact, creating the Uri, Builder, and other objects is already less performant than raw string manipulation, and here it's doing all that twice. Seems like overhead that should be avoided if possible.

That seems like a huge micro optimization which shouldn't be considered. The user will click little times in the share button, that won't have any issues.

[anddea]

huge micro optimization
The user will click little times in the share button, that won't have any issues.

Sure (why huge though?)

the end user won't notice any big impact

Still, with one injection in my repo, there were no issues (if I'm not missing anything, never got any reports). So if possible, then why?

[drobotk]

I wasn't sure whether fullUrl should be sanitized too, so I figured its better to be safe than sorry

[LisoUseInAIKyrios]

It appears fullUrl is used for remote logging. shareUrl is also remotely logged as well.
I think it's more long term stable to instead modify the url passed to the share intent, rather than broadcast modified urls to Spotify that are missing data.

Search for string "android.intent.extra.TEXT" and see the method invokeSuspend(Object).
The url can be modified before it's passed into intent.putExtra(), and the remote logging remains untouched.

[LisoUseInAIKyrios]

Modifying the url passed to the intent works, but only if the Android share sheet is used (the quick links first shown are still unpatched).

[LisoUseInAIKyrios]

I meant to push to my own branch, but the commit here shows the what I intended.

There's a few places left to patch (X and TikTok), but they are simple and it would not touch remote logging functionality. (Edit: X uses the Android share sheet, and it's already patched with the code in the commit above).

The downside is the patch will require half a dozen fingerprints, and if quick share targets are added they won't be modified until the patch is updated.

[LisoUseInAIKyrios]

It looks like the TikTok share code is dead legacy code, or at least it's not functional with Android 15. I installed TikTok but it does not show as a quick share action in Spotify.

So the commit above may be all that's needed.

[anddea]

I think it's more long term stable to instead modify the url passed to the share intent, rather than broadcast modified urls to Spotify that are missing data.

This is quite complicated, there are many targets involved: Copy Link, SMS, WhatsApp, Instagram, X, other apps, and "More" (which opens the Android share sheet for additional apps).

In the end, does it really matter if Spotify receives these modified URLs? If they notice and decide to change the sharing logic, they could just as easily check whatever method is being used here and update their code in a future release.

if quick share targets are added they won't be modified until the patch is updated

With the current implementation (at least in my patch), URLs are already being sanitized for all share targets, and this comes at a relatively low cost. Even if Spotify breaks this patch by altering the output format or whatever, the impact is less severe compared to the effort required to implement a more complex solution that could still be broken just as easily.

[LisoUseInAIKyrios]

I have it working with only 2 changes. Copy url, and share sheet. The share sheet is used for all sharing except copy url.

I agree it's more complicated, but I think it's worth it to make the app behave closer to unmodified. It will remove the chance of user accounts acting strange because the reported share data is missing key information.

[Nuckyz]

Nitpick to match all the other patches in Spotify, which all include new lines at the end of the files