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

Feature: Add NTLM support for CURB #144

Merged
merged 3 commits into from
Jun 18, 2015

Conversation

plainprogrammer
Copy link

My changes now only raise the not supported error if the available libcurl library used by CURB doesn't have NTLM support. New versions of libcurl should allow for NTLM to be used with CURB, which is a big win for HTTPI.

@@ -59,10 +59,11 @@ def do_request
def setup_client
basic_setup

if @request.auth.ntlm?
if @request.auth.ntlm? && !Curl.ntlm?
raise NotSupportedError, "curb does not support NTLM authentication"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be great to tell on this message what curb's version support it, just to help the developer.

@rogerleite
Copy link
Member

Hi @plainprogrammer!
Thanks for the help. I think we can merge after this improvement on not supported error message.

@tjarratt after the merge we update the changelog? We have a kind of process for this?

The support issue is with the underlying version of libcurl, not Curb itself. This messaging makes that more clear.
@coveralls
Copy link

Coverage Status

Coverage increased (+24.89%) to 96.48% when pulling 91352dc on plainprogrammer:feature-curb_ntlm_support into be7c740 on savonrb:master.

@tjarratt
Copy link
Contributor

tjarratt commented Mar 2, 2015

@rogerleite that sounds right. I'm not opposed to having contributors update the changelog, but I try to either

  • update the changelog when I merge a PR
  • update the changelog when I'm cutting a new release

@rogerleite
Copy link
Member

cURL, 7.10.6 - July 28 2003, adds support to NTLM authentication.
Current version: curl 7.41.0, Released on the 25th of February 2015.

http://curl.haxx.se/changes.html

I'll merge and update error message and changelog.

rogerleite added a commit that referenced this pull request Jun 18, 2015
@rogerleite rogerleite merged commit 38cd9fb into savonrb:master Jun 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants