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

[core] Added IGNORE_CUSTOM_CACHE_TIMEOUT option #894

Merged
merged 7 commits into from
Nov 13, 2018

Conversation

em92
Copy link
Contributor

@em92 em92 commented Oct 29, 2018

Explaining my usecase. For some reason I enabled custom_timeout in my config, but forgot to disable it back. Meanwhile some users already generated feed urls with _cache_timeout parameters. So if I disable custom_timeout back, those feed urls become invalid, which is not good.

So, I added option to ignore _cache_timeout param in url.

@logmanoriginal
Copy link
Contributor

Thanks for the PR 👍

So if I disable custom_timeout back, those feed urls become invalid, which is not good.

I agree, this is a server-side issue.

So, I added option to ignore _cache_timeout param in url.

This doesn't feel right to me and is confusing to new users. Users might also question if the parameter is even applied in their specific situation, which adds a layer of uncertainty.

An alternative to this is replying with the "301 Moved Permanently" header. Feed readers and browsers will automatically follow the location provided in the header. Here is some code for reference:

header('Location: index.php?action=...', true, 301);

Would that work for you?

@em92
Copy link
Contributor Author

em92 commented Nov 11, 2018

Would that work for you?

Yep. Implemented that.

@logmanoriginal
Copy link
Contributor

Almost. Maybe I should have explained myself better.

You can remove ignore_custom_timeout entirely and just assume, that custom_timeout = false implies ignore_custom_timeout = true. That way, you only need to replace this line:

throw new \HttpException('This server doesn\'t support "_cache_timeout"!');

by this:

unset($params['_cache_timeout']);
header('Location: ' . parse_url($_SERVER['REQUEST_URI'], PHP_URL_PATH) . '?' . http_build_query($params), true, 301);
die();

So, if custom_timeout = true, everything works fine.
If custom_timeout = false, everyone gets redirected.

@logmanoriginal logmanoriginal merged commit d951000 into RSS-Bridge:master Nov 13, 2018
@logmanoriginal
Copy link
Contributor

Merged. Thanks for keeping it updated! 👍

@em92 em92 deleted the ignore-cache-timeout branch November 13, 2018 17:04
infominer33 pushed a commit to web-work-tools/rss-bridge that referenced this pull request Apr 17, 2020
…SS-Bridge#894)

Requesting `_cache_timeout` on servers where this option is disabled currently results in the error message 'This server doesn\'t support "_cache_timeout"!'. This commit changes that behavior to redirect to the query without `_cache_timeout`.
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