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

Fix Payload dropped from POST when using proxy with NTLM authentication #3669

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
2 participants
@Neurones67
Copy link
Contributor

commented Mar 11, 2019

Hello,

There's an attempt to fix the bug (#2431) that prevents sending payload when authentication with NTLM is done before sending credentials.

I noticed that the check that prevent payload from sending in case of authentication doesn't check properly if the authentication is done or not.

They're cases where the proxy respond "200 OK" before sending authentication challenge. This change thake care of that cases. I tested it and it works in my cases.

Can you tell me what do you think about it?

Thank you

@bagder bagder changed the title Attempt to fix issue https://github.com/curl/curl/issues/1261 Fix HTTP POST with Negotiate Mar 11, 2019

@bagder

This comment has been minimized.

Copy link
Member

commented Mar 11, 2019

FYI, #1975 is another ongoing work to fix that same issue.

@Neurones67 Neurones67 changed the title Fix HTTP POST with Negotiate Attempt to fix issue https://github.com/curl/curl/issues/2431 Mar 11, 2019

@Neurones67

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2019

I made a mistake, it's the bug #2431 I talk about.

@bagder

This comment has been minimized.

Copy link
Member

commented Mar 11, 2019

But please spell out what the PR does in the title! The fixed issue can be mentioned in the description.

@Neurones67 Neurones67 changed the title Attempt to fix issue https://github.com/curl/curl/issues/2431 Fix Payload dropped from POST when using proxy with NTLM authentication Mar 11, 2019

@bagder

This comment has been minimized.

Copy link
Member

commented Mar 11, 2019

Test 1097 fails with this PR applied, but to me that seems right so the test needs to be updated accordingly. It seems to behave correctly with this patch as now the post data is actually seen being sent there.

@Neurones67

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

The unit test is now fixed.

Some tests on some environments failed but I don't think it's related to this PR.

@bagder

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

Agreed!

@bagder

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

Thanks!

@bagder bagder closed this in dd8a19f Mar 13, 2019

@lock lock bot locked as resolved and limited conversation to collaborators Jun 11, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.