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

Close response body before moving on to next request/response #120

Merged
merged 4 commits into from
May 28, 2024

Conversation

samuong
Copy link
Owner

@samuong samuong commented May 26, 2024

Fixes #118

@ziyiwang
Copy link

sure let me test it out and come back to you, most likely removing defer will also fix the issue

@samuong
Copy link
Owner Author

samuong commented May 27, 2024

I tested this today and confirmed that 1) the response body was not being closed properly, which meant that Alpaca was trying to parse the response body as if it contained the header, and 2) the fix that I suggested does work. I want to rework this a little bit to add a bit more error checking, and test it for a bit longer. Hoping to merge this PR some time this week and cut another release of Alpaca with the fix for this (and the other bug) included.

Again, thanks for reporting this issue. NTLM auth has been gradually phased out where I am, and so I'm not sure when this regression slipped in.

@ziyiwang
Copy link

ziyiwang commented May 27, 2024

No problem, thanks, yup agree with proper handling close body, should resolve issue, i tested with a few users on our end with the io stream fix, also confirms fix issue as body from 407 is not really needed, bit i leave it in your capable hands, thanks mate, great project, really address a gap we have with cntlm

@samuong samuong merged commit e81c962 into master May 28, 2024
16 checks passed
@samuong samuong deleted the issue-118 branch May 28, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants