Skip to content
This repository was 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
Luckyboys wants to merge 1 commit into
heroku:masterfrom
kpaas-io:hotfix-put-not-authorized
Open

Luckyboys wants to merge 1 commit into
heroku:masterfrom
kpaas-io:hotfix-put-not-authorized

Conversation

@Luckyboys

Copy link
Copy Markdown

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

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

@Luckyboys

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

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

Luckyboys commented Oct 9, 2019

Copy link
Copy Markdown
Author

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

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

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