Skip to content

Commit

Permalink
Cleanup properly after hijacking a request
Browse files Browse the repository at this point in the history
The logging handler hijacks the HTTP request, but does
not properly clean up the files left behind by multipart
forms.

The HTTP library normally does this, but if the connection is hijacked,
all responsibility to clean up is on the hijacker.
  • Loading branch information
Matthew Mussomele committed Jul 19, 2019
1 parent 415c481 commit 21f9588
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 0 deletions.
3 changes: 3 additions & 0 deletions logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ func (h loggingHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
url := *req.URL

h.handler.ServeHTTP(logger, req)
if req.MultipartForm != nil {
req.MultipartForm.RemoveAll()
}

params := LogFormatterParams{
Request: req,
Expand Down
73 changes: 73 additions & 0 deletions logging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,16 @@ package handlers

import (
"bytes"
"encoding/base64"
"fmt"
"io/ioutil"
"math/rand"
"mime/multipart"
"net/http"
"net/http/httptest"
"net/url"
"os"
"path/filepath"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -38,6 +45,72 @@ func TestMakeLogger(t *testing.T) {
}
}

func TestLoggerCleanup(t *testing.T) {
rand.Seed(time.Now().UnixNano())

rbuf := make([]byte, 128)
if _, err := rand.Read(rbuf); err != nil {
t.Fatalf("Failed to generate random content: %v", err)
}
contents := base64.StdEncoding.EncodeToString(rbuf)

var body bytes.Buffer
body.WriteString(fmt.Sprintf(`
--boundary
Content-Disposition: form-data; name="buzz"; filename="example.txt"
%s
--boundary--
`, contents))
r := multipart.NewReader(&body, "boundary")
form, err := r.ReadForm(0) // small max memory to force flush to disk
if err != nil {
t.Fatalf("Failed to read multipart form: %v", err)
}

tmpFiles, err := ioutil.ReadDir(os.TempDir())
if err != nil {
t.Fatalf("Failed to list %s: %v", os.TempDir(), err)
}

var tmpFile string
for _, f := range tmpFiles {
if !strings.HasPrefix(f.Name(), "multipart-") {
continue
}

path := filepath.Join(os.TempDir(), f.Name())
switch b, err := ioutil.ReadFile(path); {
case err != nil:
t.Fatalf("Failed to read %s: %v", path, err)
case string(b) != contents:
continue
default:
tmpFile = path
break
}
}

if tmpFile == "" {
t.Fatal("Could not find multipart form tmp file")
}

req := newRequest("GET", "/subdir/asdf")
req.MultipartForm = form

var buf bytes.Buffer
handler := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
req.URL.Path = "/" // simulate http.StripPrefix and friends
w.WriteHeader(200)
})
logger := LoggingHandler(&buf, handler)
logger.ServeHTTP(httptest.NewRecorder(), req)

if _, err := os.Stat(tmpFile); err == nil || !os.IsNotExist(err) {
t.Fatalf("Expected %s to not exist, got %v", tmpFile, err)
}
}

func TestLogPathRewrites(t *testing.T) {
var buf bytes.Buffer

Expand Down

0 comments on commit 21f9588

Please sign in to comment.