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

Remove unnecessary header field in ErrorResponse #1066

Merged
merged 1 commit into from Jan 23, 2019

Conversation

vadmeste
Copy link
Member

ErrorResponse.Headers is causing a panic inside golang http package.

The following code can be easily crashed if GetObject API returns an error,
such as ErrSlowDown:

  reader, err := s3Client.GetObject("my-bucketname", "my-source-objectname", minio.GetObjectOptions{})
  if err != nil {
    log.Fatalln(err)
  }
  defer reader.Close()

  stat, err := reader.Stat()
  if err != nil {
    log.Fatalln(err)
  }

  n, err := s3Client.PutObject("my-bucketname", "my-target-objectname", reader, stat.Size, minio.PutObjectOptions{})
  if err != nil {
    log.Fatalln(err)
  }

The reason is that reader is passed to s3Client.PutObject therefore to golang http
via request.Body. Since, reader.Read() can return an ErrorResponse, that error will
be tested by golang http, but since ErrorResponse is uncomparable, then it will cause
a crash with the following stackstrace:

  panic: runtime error: comparing uncomparable type minio.ErrorResponse

  goroutine 20 [running]:
  net/http.(*Request).write(0xc0002c2300, 0x761e60, 0xc000069f80, 0x0, 0x0, 0x0, 0x7628a0, 0xc000518780)
          /home/vadmeste/work/go/src/net/http/request.go:647 +0x74c
  net/http.(*persistConn).writeLoop(0xc0000a17a0)
          /home/vadmeste/work/go/src/net/http/transport.go:1888 +0x1b8
  created by net/http.(*Transport).dialConn
          /home/vadmeste/work/go/src/net/http/transport.go:1339 +0x966
  exit status 2

Hence, removing Headers since it is a map and it is uncomparable.

This will fix minio/mc#2327

ErrorResponse.Headers is causing a panic inside golang http package.

The following code can be easily crashed if GetObject API returns an error,
such as ErrSlowDown:

```
  reader, err := s3Client.GetObject("my-bucketname", "my-source-objectname", minio.GetObjectOptions{})
  if err != nil {
    log.Fatalln(err)
  }
  defer reader.Close()

  stat, err := reader.Stat()
  if err != nil {
    log.Fatalln(err)
  }

  n, err := s3Client.PutObject("my-bucketname", "my-target-objectname", reader, stat.Size, minio.PutObjectOptions{})
  if err != nil {
    log.Fatalln(err)
  }
```

The reason is that `reader` is passed to s3Client.PutObject therefore to golang http
via request.Body.  Since, reader.Read() can return an ErrorResponse, that error will
be tested by golang http, but since ErrorResponse is uncomparable, then it will cause
a crash with the following stackstrace:

```
  panic: runtime error: comparing uncomparable type minio.ErrorResponse

  goroutine 20 [running]:
  net/http.(*Request).write(0xc0002c2300, 0x761e60, 0xc000069f80, 0x0, 0x0, 0x0, 0x7628a0, 0xc000518780)
          /home/vadmeste/work/go/src/net/http/request.go:647 +0x74c
  net/http.(*persistConn).writeLoop(0xc0000a17a0)
          /home/vadmeste/work/go/src/net/http/transport.go:1888 +0x1b8
  created by net/http.(*Transport).dialConn
          /home/vadmeste/work/go/src/net/http/transport.go:1339 +0x966
  exit status 2
```

Hence, removing Headers since it is a map and it is uncomparable.
Copy link
Member

@harshavardhana harshavardhana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW this is a Go implementation bug, but this workaround is perfect for now.

LGTM tested.

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

Successfully merging this pull request may close these issues.

Panic while mirroring corrupted minio storage to openstack swift
4 participants