Update transport.go , to fix ntlm 407 challenge httpresponse containing extra httpbody issue #119
Conversation
ziyiwang
requested review from
camh-,
juliaogris,
marcelocantos and
samuong
as code owners
…ddress instead of nonloopback address
| // No packets are ever sent, because UDP is "connectionless". | ||
| // The IP address chosen is in one of the "documentation" prefixes that are guaranteed not to exist on the | ||
| // internet, and is therefore "safe" to use. It will generally follow the default route set in the OS. | ||
| conn, err := net.Dial("udp", "192.0.2.1:53") // RFC5737 TEST-NET-1 |
There was a problem hiding this comment.
It makes sense to do a UDP dial and take the local IP address, but it's not really clear to me which network interface the OS will choose to dial 192.0.2.1. RFC5737 says that this address "SHOULD" be unroutable, but maybe it's possible for people to, for example, set this up as a local test network which would quietly break Alpaca in unexpected ways?
Before we settle on an approach, I'd like to test a few scenarios myself first (including what happens if I try to make a UDP connection to this IP). I'll get back to you on that later this week, but let's make this myIpAddress() fix in a separate PR? It seems unrelated to the other change.
There was a problem hiding this comment.
@ziyiwang I took a look at this, and yes Alpaca does give the "wrong" IP address back - in my case, it returns my WiFi adapter's IP address, rather than the VPN address, so similar to what you were seeing.
I've written up the problem here: #122.
I personally don't need to use the myIpAddress() function, so thanks for reporting this, it's not something I would've noticed myself.
| var resp2 *http.Response | ||
| var err2 error | ||
|
|
||
| resp2,err2 = http.ReadResponse(t.reader, req) |
There was a problem hiding this comment.
I'm not sure it's correct to read another response here. This function gets called by ProxyHandler.proxyRequest(), and the body of the response gets closed later in that file. Since this is done in a defer statement, it might not happen until after the next request is sent, which I think is the bug that you're running into.
If that's the case, then I think #120 would fix it - any chance you can give that branch a try and let me know if it fixes your issue?
There was a problem hiding this comment.
sure let me test it out and come back to you, most likely removing defer will also fix the issue
There was a problem hiding this comment.
Sounds good, let's continue the discussion in that PR.
to fix ntlm 407 challenge httpresponse containing extra httpbody issue issue #118
extra httpBody in httpResponse with 407 , is not read and stuck in stream, the body needs to be read and discarded before next HTTP response comes in (HTTP 200) , otherwise the stuck body in stream will cause HTTP response error