heroku / docker-registry-client

A Go API client for the v2 Docker Registry API
BSD 3-Clause "New" or "Revised" License
297 stars 225 forks source link

fix(transport): fix request body not send again when not authorized #73

Open Luckyboys opened 5 years ago

Luckyboys commented 5 years ago

Under normal circumstances, the body of req will be read to EOF when the first request is made. If the first request is due to the permission reason and there is no request to complete, it needs to re-request after obtaining the token. But at this time req's body has reached EOF and may even be closed, so when resending the request, you need to reset the body so that the next request can re-read the body.

freeformz commented 5 years ago

Thanks! Do you think you could utilize http.Request.GetBody() instead of copying the body in full ?

Luckyboys commented 5 years ago

Thanks! Do you think you could utilize http.Request.GetBody() instead of copying the body in full ?

What do you mean by changing the read req.Body to read the ReadCloser returned by req.GetBody()?

freeformz commented 5 years ago

I mean: req.Body = req.GetBody()

Luckyboys commented 5 years ago

I mean: req.Body = req.GetBody()

Thank you for reminding me, let me also learn this implementation.

But I found new problem that req.GetBody function is . Because when Registry.UploadBlob function new a http.NewRequest, the content variable is io.Reader. At the net/http/request.go:NewRequestWithContext function, when body not nil , but the body type not in *bytes.Buffer, *bytes.Reader, *strings.Reader, the GetBody will be nil.

So, if we use the os.File like os.Open to get the reader, it will be have some problem.

freeformz commented 5 years ago

Sorry, maybe that was a bad idea. :-(

Maybe test GetBody == nil { // do a copy } else { Body = GetBody() } ?

Luckyboys commented 5 years ago

Sorry, maybe that was a bad idea. :-(

Maybe test GetBody == nil { // do a copy } else { Body = GetBody() } ?

I have new idea. We can check the req.Body != nil and req.GetBody == nil. If satisfy these conditions, then read all content, and make a new GetBody function. When isTokenDemand(resp) return not nil, we can also use req.GetBody to get the reader agian.

freeformz commented 5 years ago

SGTM