Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fasthttp client errors with io.Pipe() #1408

Closed
lfld287 opened this issue Oct 26, 2022 · 4 comments
Closed

fasthttp client errors with io.Pipe() #1408

lfld287 opened this issue Oct 26, 2022 · 4 comments

Comments

@lfld287
Copy link

lfld287 commented Oct 26, 2022

test code here:

  defer func() {
	  if r := recover(); r != nil {
		  t.Fatal("fatal recover triggered", zap.Any("recover", r))
	  }
  }()

  srcFilePath := "./test/download/test_download.img"
  f, err := os.Open(srcFilePath)
  if err != nil {
	  t.Fatal(err)
  }

  defer func() {
	  _ = f.Close()
  }()

  pr, pw := io.Pipe()
  mw := multipart.NewWriter(pw)

  go func() {
	  defer func() {
		  _ = mw.Close()
		  _ = pw.Close()
		  if err != nil {
			  t.Error("upload file write field failed", zap.Error(err))
		  }
	  }()

	  err = mw.WriteField("user", "test")
	  if err != nil {
		  return
	  }

	  err = mw.WriteField("type", "test")
	  if err != nil {
		  return
	  }

	  err = mw.WriteField("sha1", "")
	  if err != nil {
		  return
	  }

	  var filePart io.Writer
	  filePart, err = mw.CreateFormFile("file", "test.dat")
	  if err != nil {
		  return
	  }

	  _, err = io.Copy(filePart, f)
  }()

  req := fasthttp.AcquireRequest()
  defer fasthttp.ReleaseRequest(req)

  req.Header.SetContentType(mw.FormDataContentType())
  req.Header.SetMethod("POST")
  req.SetBodyStream(pr, -1)
  req.SetRequestURI(fmt.Sprintf("%s/file/upload", "http://127.0.0.1:1234"))

  //ignore response
  err = fasthttp.Do(req, nil)
  if err != nil {
	  t.Fatal(err)
  }

and result:
=== RUN TestPipe
client_test.go:46: fatal recover triggered {recover 15 0 BUG: io.Reader returned 0, nil }
client_test.go:67: upload file write field failed {error 26 0 io: read/write on closed pipe}
--- FAIL: TestPipe (0.00s)

@lfld287
Copy link
Author

lfld287 commented Oct 26, 2022

I found this : golang/go@3d2321f

//
// Implementations of Read are discouraged from returning a
// zero byte count with a nil error, except when len(p) == 0.
// Callers should treat a return of 0 and nil as indicating that
// nothing happened; in particular it does not indicate EOF.
//

maybe should remove this panic?

@erikdubbelboer
Copy link
Collaborator

I'm trying to understand what is causing this. It looks like fasthttp tries to read from pr before the goroutine has written anything to it. But io.Pipe should be blocking on the read in that case.

I don't have the time, but could you debug this a bit more and see why this is happening?

I'm afraid that if we remove the panic it will just keep retrying the Read until it returns an error using up a lot of CPU. And adding a random sleep in this case feels bad as well.

@nick9822
Copy link
Contributor

nick9822 commented Nov 3, 2022

@erikdubbelboer The issue happens where mw writes "" at err = mw.WriteField("sha1", "") which I think write 0 bytes to the pipe. Everything works fine if we ignore writing response and let the loop continue.

Created PR #1417

@erikdubbelboer
Copy link
Collaborator

Fixed in #1417

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

No branches or pull requests

3 participants