Don't reuse the existing request for digest based auth. #12
Conversation
In some cases, such as when handling digest auth, we might need to retry the request. We shouldn't be using the original request object however because the Body will have been read by the first request. To fix this a new argument has been added to Do() so that the original body can be passed along to the next request. Note, the original body could be copied. But this seemed like a better implementation because it saves a couple of allocations and potentially memory too depending on the body size.
|
@andygrunwald, FYI this is still a work in progress but I'm hoping to finish it in the next couple of days. For the record, the existing APIs seem to work in most cases. However this code fails complaining about differences in the body versus the content length: package main
import (
"github.com/andygrunwald/go-gerrit"
log "github.com/Sirupsen/logrus"
)
func main() {
client, err := gerrit.NewClient("http://localhost:8081/", nil)
if err != nil {
panic(err)
}
user := ""
password := ""
client.Authentication.SetDigestAuth(user, password)
input := &gerrit.ReviewInput{
Message: "FOOBAR",
Drafts: "PUBLISH_ALL_REVISIONS",
}
info, response, err := client.Changes.SetReview(
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
input,
)
log.Info(info)
log.Info(response)
log.Info(err)
if err != nil {
panic(err)
}
}Right now the changes are causing |
Without this bit of code retrying the request with the previous uri results in a/ being double prepended.
|
Right now i am on vacation. Will Be back in 10 days. Ok? Am Freitag, 9. September 2016 schrieb Coveralls :
|
|
Not a problem, still working out a few issues anyway. Enjoy your vacation! |
|
Thank you. Will do. Croatia is amazing so far. Don't work to much / hard :) Am Freitag, 9. September 2016 schrieb opalmer :
|
For digest based auth. we were reusing the request
anytime we received StatusUnauthorized from the
server. The first attempt to fix this was to
reconstruct the request which worked but was
more complicated. This commit improves on the
previous approach by:
* Removing the special digest only code in Do()
* Removing the need to modify all of go-gerrit to
pass in the request input to Do()
* Sending a request to the target URI without a
body. This reduces the amount of traffic to the server
and also help to allow the http client to reuse the
connection in some cases.
opalmer
changed the title
|
@andygrunwald, this ready for a look when you get back I think. I've also opened up #13 which lead to the discovery of this bug. Unfortunately, this PR only partially fixes #13 which I'm still working on but that should probably be addressed in another PR. |
|
Merged manually. Thank you @opalmer |
For digest based auth. we were reusing the request anytime we received
StatusUnauthorizedfrom the server. The first attempt to fix this was to reconstruct the request which worked but was more complicated. This PR improves on the previous approach by:Do()The above was mostly sourced from commit 30db148.