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

feat(Boost For Reddit): Add Fix /s/ links patch #631

Merged
merged 21 commits into from
Jun 8, 2024

Conversation

OctoNezd
Copy link
Contributor

Multi-part PR:

This PR fixes ReVanced/revanced-patches#478

Code is essentially copy-paste from patch for Sync, but with more exception handling and adding the Authorization header to prevent reddit from giving the client page "log in or provide token or else" without proper Location: header.

@OctoNezd
Copy link
Contributor Author

I really hate the database query part of this - however I couldn't find anything inside context that includes access token or any sort of HTTP client - so its the best solution I managed to come up with :(

My network is censored weirdly - I cant open reddit without VPN, and reddit blocks VPNs

@OctoNezd OctoNezd changed the title [2/2] Add support for /s/ links on boost for reddit. feat(Boost For Reddit - FixSLinks): [2/2] Add support for /s/ links on boost for reddit. May 11, 2024
@oSumAtrIX

This comment was marked as resolved.

import java.net.URL;

public final class FixSLinksPatch {
public static String getUserAccessToken(Context context) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be private

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it still be private? Interfaces don't allow private+default

@OctoNezd
Copy link
Contributor Author

This is related to s-links - to get slink reliably (incl. banned networks) Authorization header with user token must be supplied, else reddit gives 403 without proper Location.

@OctoNezd
Copy link
Contributor Author

Do I add Sync for reddit switch to interface in this PR or should I make a separate one after that one gets merged?

@OctoNezd OctoNezd requested a review from oSumAtrIX May 12, 2024 12:22
@OctoNezd
Copy link
Contributor Author

On note of implementing support for Sync for reddit:

Sync doesn't store access tokens in its DB (unlike boost) - only refresh tokens. The access token in sync seem to be kept inside com.laurencedawson.reddit_sync.singleton.AccountSingleton - but I have no idea how one can even access it from integration. Unless the patch would also patch the AccountSingleton method that sets the Authorization header and somehow transfer between these two.

@OctoNezd OctoNezd marked this pull request as draft May 16, 2024 14:54
@OctoNezd OctoNezd marked this pull request as ready for review May 16, 2024 15:37
@OctoNezd
Copy link
Contributor Author

Tested on Boost 1.12.12 and Sync v23.06.30-13:39.

Resolution no longer halts the UI.

Everything hopefully should be good, gonna run around with boost for a few days and check if everything gonna be okay.

@oSumAtrIX oSumAtrIX changed the title feat(Boost For Reddit - FixSLinks): [2/2] Add support for /s/ links on boost for reddit. feat(Boost For Reddit): Add Fix /s/ links patch May 18, 2024
Copy link
Member

@oSumAtrIX oSumAtrIX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The abstraction can be improved. Theres still some duplicate implementations such as openInAppBrowser or resolveSLink. Instead of copying the implementation, try to reduce duplicates. In the example of openInAppBrowser you could for example add a field of type Activity and then set the field in the constructor of the instance with either activity for Sync and Boost, then the base class implementation could reference it. You would then not need to duplicate the implementation like you do now in the implementing classes.

@OctoNezd
Copy link
Contributor Author

In the example of openInAppBrowser you could for example add a field of type Activity and then set the field in the constructor of the instance with either activity for Sync and Boost, then the base class implementation could reference it

Are you suggesting that both Boost & Sync use the same class, and inside constructor there is some sort of switch-case that determines webview activity based on package name?

Copy link
Member

@oSumAtrIX oSumAtrIX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some refactoring changes and TODOs in the code. Please check on them and test the refactoring changes (same for the patches part). Apart from that lgtm.

@OctoNezd
Copy link
Contributor Author

OctoNezd commented Jun 2, 2024

I am not sure on todo you added:

                    // TODO: Check if this ends in java.lang.ExceptionInInitializerError.
                    Objects.requireNonNull(location, "Location is null");

Wouldnt that be caught by catch (Exception e)?

@oSumAtrIX
Copy link
Member

Wouldnt that be caught by catch (Exception e)?

You originally left a comment claiming that Objects.requireNonNull was causing a RuntimeException, but given that you were not able to reproduce again the question is redundant.

@oSumAtrIX oSumAtrIX merged commit 0c9ad35 into ReVanced:dev Jun 8, 2024
2 checks passed
Copy link

welcome bot commented Jun 8, 2024

Thank you for contributing to ReVanced. Join us on Discord to receive a role for your contribution.

revanced-bot pushed a commit that referenced this pull request Jun 8, 2024
# [1.11.0-dev.3](v1.11.0-dev.2...v1.11.0-dev.3) (2024-06-08)

### Features

* **Boost For Reddit:** Add `Fix /s/ links` patch ([#631](#631)) ([0c9ad35](0c9ad35))
revanced-bot pushed a commit that referenced this pull request Jun 23, 2024
# [1.11.0](v1.10.0...v1.11.0) (2024-06-23)

### Bug Fixes

* **YouTube - Client spoof:** Correctly play more livestreams using Android VR ([#652](#652)) ([58f8172](58f8172))
* **YouTube - Hide description components:** Replace `Hide game section` and `Hide music section` with `Hide attributes section` ([#654](#654)) ([f82dfce](f82dfce))
* **YouTube - Return YouTube Dislike:** Do not replace view count with dislikes ([5f79196](5f79196))
* **YouTube - Spoof client:** Correctly play as background audio only with Android VR ([9adbc66](9adbc66))
* **YouTube - Spoof client:** Correctly play some live streams when using Android VR ([f74fb17](f74fb17))
* **YouTube:** Rename `Minimized playback` to `Remove background playback restrictions` ([#651](#651)) ([84c50c0](84c50c0))

### Features

* **Boost For Reddit:** Add `Fix /s/ links` patch ([#631](#631)) ([0c9ad35](0c9ad35))
* **YouTube - Comments:** Add `Hide 'Create a Short' button` option ([#656](#656)) ([064d8e9](064d8e9))
* **YouTube - Comments:** Add `Hide Thanks button` and `Hide 'Comments by members' header` options ([#653](#653)) ([240e805](240e805))
* **YouTube - Miniplayer:** Rename `Tablet mini player` and allow selecting the style of the in-app miniplayer ([#649](#649)) ([f483af6](f483af6))
E85Addict pushed a commit to E85Addict/revanced-integrations that referenced this pull request Jun 23, 2024
# [1.11.0](v1.10.0...v1.11.0) (2024-06-23)

### Bug Fixes

* **YouTube - Client spoof:** Correctly play more livestreams using Android VR ([ReVanced#652](https://github.com/E85Addict/revanced-integrations/issues/652)) ([58f8172](58f8172))
* **YouTube - Hide description components:** Replace `Hide game section` and `Hide music section` with `Hide attributes section` ([ReVanced#654](https://github.com/E85Addict/revanced-integrations/issues/654)) ([f82dfce](f82dfce))
* **YouTube - Return YouTube Dislike:** Do not replace view count with dislikes ([5f79196](5f79196))
* **YouTube - Spoof client:** Correctly play as background audio only with Android VR ([9adbc66](9adbc66))
* **YouTube - Spoof client:** Correctly play some live streams when using Android VR ([f74fb17](f74fb17))
* **YouTube:** Rename `Minimized playback` to `Remove background playback restrictions` ([ReVanced#651](https://github.com/E85Addict/revanced-integrations/issues/651)) ([84c50c0](84c50c0))

### Features

* **Boost For Reddit:** Add `Fix /s/ links` patch ([ReVanced#631](https://github.com/E85Addict/revanced-integrations/issues/631)) ([0c9ad35](0c9ad35))
* **YouTube - Comments:** Add `Hide 'Create a Short' button` option ([ReVanced#656](https://github.com/E85Addict/revanced-integrations/issues/656)) ([064d8e9](064d8e9))
* **YouTube - Comments:** Add `Hide Thanks button` and `Hide 'Comments by members' header` options ([ReVanced#653](https://github.com/E85Addict/revanced-integrations/issues/653)) ([240e805](240e805))
* **YouTube - Miniplayer:** Rename `Tablet mini player` and allow selecting the style of the in-app miniplayer ([ReVanced#649](https://github.com/E85Addict/revanced-integrations/issues/649)) ([f483af6](f483af6))

### Performance Improvements

* Add upstream sync ([cdb049d](cdb049d))
E85Addict pushed a commit to E85Addict/revanced-integrations that referenced this pull request Jul 7, 2024
# [1.11.0](v1.10.0...v1.11.0) (2024-07-07)

### Bug Fixes

* **YouTube - Client spoof:** Correctly play more livestreams using Android VR ([ReVanced#652](https://github.com/E85Addict/revanced-integrations/issues/652)) ([58f8172](58f8172))
* **YouTube - Hide description components:** Replace `Hide game section` and `Hide music section` with `Hide attributes section` ([ReVanced#654](https://github.com/E85Addict/revanced-integrations/issues/654)) ([f82dfce](f82dfce))
* **YouTube - Return YouTube Dislike:** Do not replace view count with dislikes ([5f79196](5f79196))
* **YouTube - Spoof client:** Correctly play as background audio only with Android VR ([9adbc66](9adbc66))
* **YouTube - Spoof client:** Correctly play some live streams when using Android VR ([f74fb17](f74fb17))
* **YouTube:** Rename `Minimized playback` to `Remove background playback restrictions` ([ReVanced#651](https://github.com/E85Addict/revanced-integrations/issues/651)) ([84c50c0](84c50c0))

### Features

* **Boost For Reddit:** Add `Fix /s/ links` patch ([ReVanced#631](https://github.com/E85Addict/revanced-integrations/issues/631)) ([0c9ad35](0c9ad35))
* **YouTube - Comments:** Add `Hide 'Create a Short' button` option ([ReVanced#656](https://github.com/E85Addict/revanced-integrations/issues/656)) ([064d8e9](064d8e9))
* **YouTube - Comments:** Add `Hide Thanks button` and `Hide 'Comments by members' header` options ([ReVanced#653](https://github.com/E85Addict/revanced-integrations/issues/653)) ([240e805](240e805))
* **YouTube - Miniplayer:** Rename `Tablet mini player` and allow selecting the style of the in-app miniplayer ([ReVanced#649](https://github.com/E85Addict/revanced-integrations/issues/649)) ([f483af6](f483af6))

### Performance Improvements

* Add upstream sync ([4e3b281](4e3b281))
E85Addict pushed a commit to E85Addict/revanced-integrations that referenced this pull request Jul 11, 2024
# [1.11.0](v1.10.0...v1.11.0) (2024-07-11)

### Bug Fixes

* **YouTube - Client spoof:** Correctly play more livestreams using Android VR ([ReVanced#652](https://github.com/E85Addict/revanced-integrations/issues/652)) ([58f8172](58f8172))
* **YouTube - Hide description components:** Replace `Hide game section` and `Hide music section` with `Hide attributes section` ([ReVanced#654](https://github.com/E85Addict/revanced-integrations/issues/654)) ([f82dfce](f82dfce))
* **YouTube - Return YouTube Dislike:** Do not replace view count with dislikes ([5f79196](5f79196))
* **YouTube - Spoof client:** Correctly play as background audio only with Android VR ([9adbc66](9adbc66))
* **YouTube - Spoof client:** Correctly play some live streams when using Android VR ([f74fb17](f74fb17))
* **YouTube:** Rename `Minimized playback` to `Remove background playback restrictions` ([ReVanced#651](https://github.com/E85Addict/revanced-integrations/issues/651)) ([84c50c0](84c50c0))

### Features

* **Boost For Reddit:** Add `Fix /s/ links` patch ([ReVanced#631](https://github.com/E85Addict/revanced-integrations/issues/631)) ([0c9ad35](0c9ad35))
* **YouTube - Comments:** Add `Hide 'Create a Short' button` option ([ReVanced#656](https://github.com/E85Addict/revanced-integrations/issues/656)) ([064d8e9](064d8e9))
* **YouTube - Comments:** Add `Hide Thanks button` and `Hide 'Comments by members' header` options ([ReVanced#653](https://github.com/E85Addict/revanced-integrations/issues/653)) ([240e805](240e805))
* **YouTube - Miniplayer:** Rename `Tablet mini player` and allow selecting the style of the in-app miniplayer ([ReVanced#649](https://github.com/E85Addict/revanced-integrations/issues/649)) ([f483af6](f483af6))

### Performance Improvements

* Add upstream sync ([e9c74d4](e9c74d4))
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