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

Add CURLOPT_COOKIELIST support #455

Merged
merged 8 commits into from
Oct 11, 2024
Merged

Add CURLOPT_COOKIELIST support #455

merged 8 commits into from
Oct 11, 2024

Conversation

uvlad7
Copy link
Contributor

@uvlad7 uvlad7 commented Sep 5, 2024

Add interface to call curl_easy_setopt - Easy::setopt - with CURLOPT_COOKIELIST.

It's not something like #cookielist=, because I believe this method should be more Rubyish, like add constants for special values, support array argument, or even use CURB_OBJECT_HSETTER instead of simply passing it to libcurl and keep it consistent with other options (like you do this with headers=/CURLOPT_HTTPHEADER, for example, and set enable_cookies flag accordingly). All of this requires much more effort, and I'm happy with this simple raw interface.

@taf2
Copy link
Owner

taf2 commented Sep 9, 2024

Thanks for adding this and agreed with approach would probably like more things to just be handled through the simpler set/get approach. Do you think it would be possible to add to this a test case?

@taf2 taf2 mentioned this pull request Sep 9, 2024
@uvlad7
Copy link
Contributor Author

uvlad7 commented Sep 9, 2024

add to this a test case

Sure, gonna take some time, though. Are you OK wit httpbin usage in the test?

@taf2
Copy link
Owner

taf2 commented Sep 10, 2024

httpbin looks interesting, but we have TestServlet already in the test_helper I assume for testing if cookies are set you could add a new endpoint:

  def do_GET(req,res)
    if req.path.match(/redirect$/)
      res.status = 302
      res['Location'] = '/foo'
    elsif req.path.match(/not_here$/)
      res.status = 404
    elsif req.path.match(/error$/)
      res.status = 500
    end
    respond_with("GET#{req.query_string}",req,res)
  end

Maybe something like:

elsif req.path.match(/test_cookies/)

@uvlad7
Copy link
Contributor Author

uvlad7 commented Oct 2, 2024

Added tests. Looks like tests/bug_issue277.rb file is missing and was removed after rake package.

Can you please release a new version after this is merged?

@taf2
Copy link
Owner

taf2 commented Oct 11, 2024

Thanks this is great !

@taf2 taf2 merged commit 96b874f into taf2:master Oct 11, 2024
4 checks passed
@uvlad7
Copy link
Contributor Author

uvlad7 commented Oct 18, 2024

@taf2 can you please release a new version?

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