Factor CloseNotifier use into a new function

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

docs/handlers/blobupload.go

@@ -2,7 +2,6 @@ package handlers
 
 import (
 	"fmt"
-	"io"
 	"net/http"
 	"net/url"
 	"os"
@@ -170,30 +169,8 @@ func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Reque
 
 	// TODO(dmcgowan): support Content-Range header to seek and write range
 
-	// Get a channel that tells us if the client disconnects
+	if err := copyFullPayload(w, r, buh.Upload, buh, "blob PATCH", &buh.Errors); err != nil {
-	var clientClosed <-chan bool
+		// copyFullPayload reports the error if necessary
-	if notifier, ok := w.(http.CloseNotifier); ok {
-		clientClosed = notifier.CloseNotify()
-	} else {
-		panic("the ResponseWriter does not implement CloseNotifier")
-	}
-
-	// Copy the data
-	copied, err := io.Copy(buh.Upload, r.Body)
-	if clientClosed != nil && (err != nil || (r.ContentLength > 0 && copied < r.ContentLength)) {
-		// Didn't recieve as much content as expected. Did the client
-		// disconnect during the request? If so, avoid returning a 400
-		// error to keep the logs cleaner.
-		select {
-		case <-clientClosed:
-			ctxu.GetLogger(buh).Error("client disconnected during blob PATCH")
-			return
-		default:
-		}
-	}
-	if err != nil {
-		ctxu.GetLogger(buh).Errorf("unknown error copying into upload: %v", err)
-		buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
 		return
 	}
 
@@ -231,30 +208,8 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht
 		return
 	}
 
-	// Get a channel that tells us if the client disconnects
+	if err := copyFullPayload(w, r, buh.Upload, buh, "blob PUT", &buh.Errors); err != nil {
-	var clientClosed <-chan bool
+		// copyFullPayload reports the error if necessary
-	if notifier, ok := w.(http.CloseNotifier); ok {
-		clientClosed = notifier.CloseNotify()
-	} else {
-		panic("the ResponseWriter does not implement CloseNotifier")
-	}
-
-	// Read in the data, if any.
-	copied, err := io.Copy(buh.Upload, r.Body)
-	if clientClosed != nil && (err != nil || (r.ContentLength > 0 && copied < r.ContentLength)) {
-		// Didn't recieve as much content as expected. Did the client
-		// disconnect during the request? If so, avoid returning a 400
-		// error to keep the logs cleaner.
-		select {
-		case <-clientClosed:
-			ctxu.GetLogger(buh).Error("client disconnected during blob PUT")
-			return
-		default:
-		}
-	}
-	if err != nil {
-		ctxu.GetLogger(buh).Errorf("unknown error copying into upload: %v", err)
-		buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
 		return
 	}
 

docs/handlers/helpers.go

@@ -1,8 +1,12 @@
 package handlers
 
 import (
+	"errors"
 	"io"
 	"net/http"
+
+	ctxu "github.com/docker/distribution/context"
+	"github.com/docker/distribution/registry/api/errcode"
 )
 
 // closeResources closes all the provided resources after running the target
@@ -15,3 +19,38 @@ func closeResources(handler http.Handler, closers ...io.Closer) http.Handler {
 		handler.ServeHTTP(w, r)
 	})
 }
+
+// copyFullPayload copies the payload of a HTTP request to destWriter. If it
+// receives less content than expected, and the client disconnected during the
+// upload, it avoids sending a 400 error to keep the logs cleaner.
+func copyFullPayload(responseWriter http.ResponseWriter, r *http.Request, destWriter io.Writer, context ctxu.Context, action string, errSlice *errcode.Errors) error {
+	// Get a channel that tells us if the client disconnects
+	var clientClosed <-chan bool
+	if notifier, ok := responseWriter.(http.CloseNotifier); ok {
+		clientClosed = notifier.CloseNotify()
+	} else {
+		panic("the ResponseWriter does not implement CloseNotifier")
+	}
+
+	// Read in the data, if any.
+	copied, err := io.Copy(destWriter, r.Body)
+	if clientClosed != nil && (err != nil || (r.ContentLength > 0 && copied < r.ContentLength)) {
+		// Didn't recieve as much content as expected. Did the client
+		// disconnect during the request? If so, avoid returning a 400
+		// error to keep the logs cleaner.
+		select {
+		case <-clientClosed:
+			ctxu.GetLogger(context).Error("client disconnected during " + action)
+			return errors.New("client disconnected")
+		default:
+		}
+	}
+
+	if err != nil {
+		ctxu.GetLogger(context).Errorf("unknown error reading request payload: %v", err)
+		*errSlice = append(*errSlice, errcode.ErrorCodeUnknown.WithDetail(err))
+		return err
+	}
+
+	return nil
+}

docs/handlers/images.go

@@ -1,9 +1,9 @@
 package handlers
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 	"strings"
 
@@ -113,35 +113,14 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http
 		return
 	}
 
-	// Get a channel that tells us if the client disconnects
+	var jsonBuf bytes.Buffer
-	var clientClosed <-chan bool
+	if err := copyFullPayload(w, r, &jsonBuf, imh, "image manifest PUT", &imh.Errors); err != nil {
-	if notifier, ok := w.(http.CloseNotifier); ok {
+		// copyFullPayload reports the error if necessary
-		clientClosed = notifier.CloseNotify()
-	} else {
-		panic("the ResponseWriter does not implement CloseNotifier")
-	}
-
-	// Copy the data
-	jsonBytes, err := ioutil.ReadAll(r.Body)
-	if clientClosed != nil && (err != nil || (r.ContentLength > 0 && int64(len(jsonBytes)) < r.ContentLength)) {
-		// Didn't recieve as much content as expected. Did the client
-		// disconnect during the request? If so, avoid returning a 400
-		// error to keep the logs cleaner.
-		select {
-		case <-clientClosed:
-			ctxu.GetLogger(imh).Error("client disconnected during image manifest PUT")
-			return
-		default:
-		}
-	}
-	if err != nil {
-		ctxu.GetLogger(imh).Errorf("unknown error reading payload: %v", err)
-		imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
 		return
 	}
 
 	var manifest manifest.SignedManifest
-	if err := json.Unmarshal(jsonBytes, &manifest); err != nil {
+	if err := json.Unmarshal(jsonBuf.Bytes(), &manifest); err != nil {
 		imh.Errors = append(imh.Errors, v2.ErrorCodeManifestInvalid.WithDetail(err))
 		return
 	}