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

Disable redirection #260

Closed
wants to merge 2 commits into from
Closed

Disable redirection #260

wants to merge 2 commits into from

Conversation

davidovski
Copy link

Adresses #251 and other minor adjustments:

  • Adds config option "automatic_redirection", defaulting to true to allow their instance to redirect to others at all (defaulting to true in config.php.example)
  • Adds setting option 'Redirect to other instances if this one doesn't work" to settings page (defaults to off)
  • Fixes redirects to happen on any non 200 (success) google response, allowing for easier debugging and a wider scope of redirection conditions.
  • Fixes other settings checkboxes that weren't saving their state when being unchecked.

I believe that the automatic redirection should be completely opt-in by users of an instance, since redirecting to another, potentially unknown, site can be a major security and privacy concern. Imagine if one instance went rouge and was still listed in instances.json, users of a completely unrelated instance may be redirected to this instance unwillingly and this may lead to this user's search results getting compromised, among other things. That's why I have set the defaults as above, allowing the user the choice to redirect to other instances if they want, while also allowing instance maintainers themselves to disable the feature entirely.

As far as "trusted instances" go, I urge any instance maintainers that do wish to keep redirection on to maintain instances.json as they see fit, though unfortunately this is not configurable by the user themselves.

Please test out this PR and let me know if there is anything broken / missing, especially since I am not all that confident in php myself. Thank you

@torunar
Copy link

torunar commented Aug 7, 2023

Great job here! Unfortunately, there is also an issue I mentioned in #251 (comment):
When librex accessed through API, user can end up redirected to the search page, not the api.

@davidovski
Copy link
Author

Thanks for pointing that out. While I was trying to think about how to implement that elegantly using the existing system, I came up with a better solution for the whole redirection system.

Rather than redirecting the user to a different instance, librex itself can fallback to using a different librex instance's API to generate search results. That way if one instance stops being able to access google, then it will still be able to display results to the user and all of these requests to other instances will be proxied by the librex instance itself. Since the api has already existed for a while, it will be backwards compatible so it should prove to give reliable results. Also, since it will just be collecting results from these instances, it should work with both the api and normal results.

There are a few issues with this however, for example, if not enough instances can access google, there is a potential for a loop of inter-instance requests that will cause the user to wait an indefinite amount of time for the results.

I'll try and figure something out, but I'm open to other ideas or suggestions.

@davidovski davidovski mentioned this pull request Aug 7, 2023
5 tasks
@davidovski
Copy link
Author

davidovski commented Aug 7, 2023

Since the other PR (#261) conflicts with this one, I'm going to close this one for now.

@davidovski davidovski closed this Aug 7, 2023
@davidovski davidovski deleted the disable_redirection branch December 18, 2023 01:22
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.

2 participants