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

fix sasl_sspi digest authentication bugs #525

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@tvbuehler
Contributor

tvbuehler commented Nov 12, 2015

  • fixed unicode build for digest authentication
  • fix identity memory leak in digest authentication

@captain-caveman2k captain-caveman2k self-assigned this Nov 12, 2015

@captain-caveman2k

This comment has been minimized.

Member

captain-caveman2k commented Nov 12, 2015

Many thanks Stefan,

I'll take a further look tonight, as I've got some of my own patches to push, but both commits look pretty good from my initial review - apart from one query (see below) and a very minor code style typo with pointers ;-)

Do you think the failure check of Curl_convert_UTF8_to_tchar() should return CURL_OUT_OF_MEMORY rather than CURL_LOGIN_DENIED like we do in other calls to Curl_convert_UTF8_to_tchar() as well as memory allocs/string dups in other authentication message functions?

@tvbuehler

This comment has been minimized.

Contributor

tvbuehler commented Nov 12, 2015

I ran checksrc.pl and it didn't complain...

The CURLE_LOGIN_DENIED was a copy&paste mistake, I changed it to CURLE_OUT_OF_MEMORY, which should be appropriate afaics in this situation.

@captain-caveman2k

This comment has been minimized.

captain-caveman2k commented on d4e31c0 Nov 12, 2015

Pushed as commit b6baa10. I simplified the cast around Curl_convert_UTF8_to_tchar() as we have done in other calls to this function when we have a const char pointer.

@captain-caveman2k

This comment has been minimized.

captain-caveman2k commented on 51f9352 Nov 12, 2015

Pushed as commit 077fd8f but moved a couple of the calls to after we have freed the credentials handle - to be consistent with other code that does the same.

@tvbuehler tvbuehler deleted the tvbuehler:fix-sasl-digest branch Nov 13, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment