Skip to content

Fix digest credentials for proxy in windows#4104

Merged
ethomson merged 2 commits intolibgit2:masterfrom
cbargren:fix/windows-digest-proxy
Feb 9, 2017
Merged

Fix digest credentials for proxy in windows#4104
ethomson merged 2 commits intolibgit2:masterfrom
cbargren:fix/windows-digest-proxy

Conversation

@cbargren
Copy link
Copy Markdown
Contributor

@cbargren cbargren commented Feb 1, 2017

Using a proxy with digest credentials was not working in Windows. It was simply always using basic auth whenever a plaintext credential mechanism was specified. Detecting whether the plaintext credential mechanism was digest or basic and applying that mechanism to WinHttpSetCredentials fixed the problem.

Comment thread src/transports/winhttp.c
if (t->owner->proxy.credentials) {
int cred_error = 1;
cred_error = t->owner->proxy.credentials(&t->proxy_cred, t->owner->proxy.url, NULL, allowed_types, NULL);
cred_error = t->owner->proxy.credentials(&t->proxy_cred, t->owner->proxy.url, NULL, allowed_types, t->owner->proxy.payload);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't quite understand where this change is coming from yet. I don't immediately see anything that's using a payload with regards to the digest changes... Is this a related change, or should it be split out into its own commit?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without the payload, the credentials call was segfaulting in https://github.com/nodegit/nodegit. It isn't directly related to digest credentials, but without it, the credentials callback isn't working. I'll go ahead and split it into its own commit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ethomson Alright, that's taken care of. Let me know if there's anything else that needs changing. Thanks!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Got it. Thanks for clarifying; I do appreciate that you split this out into its own commit.

@cbargren cbargren force-pushed the fix/windows-digest-proxy branch from c7e667f to 1e929eb Compare February 6, 2017 18:01
@ethomson ethomson merged commit b4bd5e8 into libgit2:master Feb 9, 2017
@cbargren cbargren deleted the fix/windows-digest-proxy branch February 9, 2017 22:30
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