Skip to content

CURLOPT parameters per feed#3367

Merged
Alkarex merged 23 commits intoFreshRSS:masterfrom
printfuck:curl_params
Jan 16, 2021
Merged

CURLOPT parameters per feed#3367
Alkarex merged 23 commits intoFreshRSS:masterfrom
printfuck:curl_params

Conversation

@printfuck
Copy link
Contributor

Closes #3271

Changes proposed in this pull request:

  • proxy settings per feed
  • cookie settings per feed

How to test the feature manually:

  1. set a proxy or cookie
  2. check if the result changes

Pull request checklist:

  • clear commit messages
  • code manually tested
  • unit tests written (optional if too hard)
  • documentation updated

@printfuck printfuck changed the title Curl params CURLOPT parameters per feed Jan 13, 2021
@Frenzie
Copy link
Member

Frenzie commented Jan 13, 2021 via email

@Alkarex Alkarex added this to the 1.18.0 milestone Jan 13, 2021
@Alkarex
Copy link
Member

Alkarex commented Jan 14, 2021

Could you please try to resolve the conflicts @printfuck ? Let us know if you face any difficulty

@printfuck
Copy link
Contributor Author

I fixed all the linting hints, but it's still waiting for the Docker builds. Is there something I missed?

@Alkarex
Copy link
Member

Alkarex commented Jan 16, 2021

Don't worry about the Docker builds. It looks good. I will test 👍🏻

@Alkarex
Copy link
Member

Alkarex commented Jan 16, 2021

@printfuck I have made a few changes; please have a look

@printfuck
Copy link
Contributor Author

This looks good to me.

@Alkarex
Copy link
Member

Alkarex commented Jan 16, 2021

Ok, let's merge and please test a bit more if you can

@Alkarex Alkarex merged commit ee175dd into FreshRSS:master Jan 16, 2021
@Alkarex
Copy link
Member

Alkarex commented Jan 24, 2021

I am wandering whether we should allow normal users to use this feature, or only admins.
Furthermore, I forgot that we will have to deal with cache differences between the users, just like we do when providing a username/password to retrieve a feed.

@printfuck
Copy link
Contributor Author

I agree. Unless
(1) the proxy servers are known to the users (e.g. published in a wiki for the given instance of freshrss) and are only available to the instance,
(2) or the users are running the proxy servers themselves, limiting access to the specific freshrss instance,
this feature will only ever be useful for admins.

I don't understand the cache thing as of now. After reading the code multiple times, I didn't get why there is need for a cache.

Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Mar 6, 2021
Alkarex added a commit that referenced this pull request Mar 9, 2021
* SimplePie prevent cache polution
#fix #3367 (comment)
#fix #3494 (comment)

* Fix bug

* Minor improvement

* Update cache filename in FreshRSS (1/2)

* cacheFilename temp

* New SimplePie get_cache_filename()

* Fix typos

* Update lib/SimplePie/SimplePie.php

Typo

* Include user-agent and timeout

* fix array_merge

* Declaration

* force_feed was lost in a commit
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.

Per feed proxy configuration

4 participants