FreshRSS icon indicating copy to clipboard operation
FreshRSS copied to clipboard
verified publisher icon FreshRSS

Expose CURLOPT_RESOLVE to UI

Open Alwaysin opened this issue 3 years ago • 13 comments

Closes #4560

Changes proposed in this pull request:

  • Exposing CURLOPT_RESOLVE to UI to force IPv4 or IPv6 of a feed

Do note that:

  • I did not apply this change on app/views/subscription/add.phtml as I believe you usually don't know in advance that you need to specifically use IPV4 or IPV6 - what do you think? EDIT: I still have included support in subscriptionController.php, damn! 🤦
  • I did not add translation for "IPv4" and "IPv6" as I believe it will be translated the same in all languages
  • last but not least I am no developer, it is just a kind of "challenge", I will not be offended if this is rejected. In particular, I have high doubts about the true/false I have used, I think this is not the right way in this case but I took example out of ssl_verify, so it was easier for me. But it really seems to be working. I may have a second look if asked for and if I have time/motivation for it!

How to test the feature manually:

  1. Select IPv4 for a feed, get the IP of the server and check traffic with tcpdump ip && host <ip>
  2. Select IPv6 for a feed, get the IP of the server and check traffic with tcpdump ip6 && host <ip>

Option is located in "Advanced" part, at the end, just after "Verify SSL security" ApplicationFrameHost_2022-08-31_23-51-51

Pull request checklist:

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

Additional information can be found in the documentation.

Alwaysin avatar Aug 31 '22 21:08 Alwaysin

It would be good to also add the option when adding a new feed, as it might otherwise fail before the feed is added

Alkarex avatar Sep 01 '22 04:09 Alkarex

I think "automatic" would be more descriptive than "by default", which in context seems to imply the default is already one of those two just set somewhere else.

Frenzie avatar Sep 01 '22 13:09 Frenzie

I think "automatic" would be more descriptive than "by default", which in context seems to imply the default is already one of those two just set somewhere else.

Quick note: We use "By default" at local feed level when the same option can also be set at global level

Alkarex avatar Sep 01 '22 13:09 Alkarex

Well, I'm sorry, it's been hours I'm trying to figure things out but I can't seem to be able to make it to work. That was too much of a task for my skills 😅 Sorry for the noise!

Alwaysin avatar Sep 01 '22 14:09 Alwaysin

@Alkarex

Quick note: We use "By default" at local feed level when the same option can also be set at global level

In that case I misunderstood and that particular phrasing makes sense.

On the screenshot I thought it meant:

  • By default: whatever cURL chooses, probably IPv6 if available, otherwise IPv4
  • IPv4: force IPv4 even if IPv6 is available
  • IPv6: half the internet won't work

But whatever is actually intended, I don't think it's clear.

Frenzie avatar Sep 01 '22 15:09 Frenzie

Well, I'm sorry, it's been hours I'm trying to figure things out but I can't seem to be able to make it to work. That was too much of a task for my skills 😅 Sorry for the noise!

Let's keep it open and I will give it a shot in the coming days

Alkarex avatar Sep 01 '22 17:09 Alkarex

When we restart this PR, we should also add an option for HTTP/1.1, HTTP/2, HTTP/3 https://github.com/FreshRSS/FreshRSS/issues/5438

Alkarex avatar Apr 06 '24 13:04 Alkarex

Related need of cURL options https://github.com/FreshRSS/FreshRSS/issues/3752

Alkarex avatar Apr 06 '24 13:04 Alkarex

Feature needed for reading feeds from websites that may have common misconfigurations like missing intermediate certificates.

benjuade avatar Sep 09 '25 06:09 benjuade

missing intermediate certificates

While waiting for this PR (help welcome), some misconfigured Web sites might work by using the existing option to disable the SSL check.

Alkarex avatar Sep 09 '25 20:09 Alkarex

missing intermediate certificates

While waiting for this PR (help welcome), some misconfigured Web sites might work by using the existing option to disable the SSL check.

Thanks for pointing that. If your believe this PR may be useful, can you share a list of things missing to approve it? 3years passed by its opening, so things may have changed.

Furthermore, it seems a pure frontend change.

benjuade avatar Sep 10 '25 20:09 benjuade

Thanks for pointing that. If your believe this PR may be useful, can you share a list of things missing to approve it? 3years passed by its opening, so things may have changed.

Furthermore, it seems a pure frontend change.

I think this PR needs to be re-done as it branch has been deleted. There are a few review comments higher up to address, and it needs to be tested. Help welcome.

Alkarex avatar Sep 10 '25 21:09 Alkarex

Thanks for pointing that. If your believe this PR may be useful, can you share a list of things missing to approve it? 3years passed by its opening, so things may have changed. Furthermore, it seems a pure frontend change.

I think this PR needs to be re-done as it branch has been deleted. There are a few review comments higher up to address, and it needs to be tested. Help welcome.

Upon better looking at the webapplication, I found that validate SSL is already a curl flag configurable for each feed: immagine

benjuade avatar Sep 21 '25 18:09 benjuade