Skip to content
This repository has been archived by the owner on Mar 16, 2024. It is now read-only.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Luckyboys
Copy link

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
Copy link

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

@Luckyboys
Copy link
Author

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
Copy link

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

@Luckyboys Luckyboys force-pushed the hotfix-put-not-authorized branch 2 times, most recently from 1ebd5fc to 0806579 Compare October 9, 2019 02:42
@Luckyboys
Copy link
Author

Luckyboys commented Oct 9, 2019

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
Copy link

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

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

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.
@Luckyboys Luckyboys force-pushed the hotfix-put-not-authorized branch from 0806579 to e3e0b31 Compare October 10, 2019 12:52
@Luckyboys
Copy link
Author

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
Copy link

SGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants