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

net/http: Filename from FormFile header does not contain slashes #15664

Closed
sdicker8 opened this issue May 12, 2016 · 15 comments

Comments

Projects
None yet
6 participants
@sdicker8
Copy link

commented May 12, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    1.6.2 window/amd64
  2. What operating system and processor architecture are you using (go env)?
    windows 7 amd64
  3. What did you do?
    If possible, provide a recipe for reproducing the error.
    A complete runnable program is good.
    A link on play.golang.org is best.
    package main

import (
"fmt"
"net/http"
"os/exec"
)

func validate(w http.ResponseWriter, r *http.Request) {
file, header, err := r.FormFile("file")
if err != nil {
fmt.Fprintf(w, "%s\n", err)
fmt.Println(err)
return
}
defer file.Close()
fmt.Fprintf(w, "%s\n", header.Header)
fmt.Fprintf(w, "%s\n", header.Filename)
fmt.Println(header.Header)
fmt.Println(header.Filename)
}

func index(w http.ResponseWriter, r *http.Request) {
fmt.Fprintf(w, html)
}

func main() {
http.HandleFunc("/", index)
http.HandleFunc("/validate", validate)
go exec.Command("rundll32", "url.dll,FileProtocolHandler",
"http://localhost:8090/").Start()
http.ListenAndServe(":8090", nil)
}

var html = `

<title>Validation</title> <style type="text/css"> body{font-family:arial;margin-top:4em;margin-left:4em} </style>

Validation

Select and submit a file to validate.

` 1. What did you expect to see? map[Content-Disposition:[form-data; name="file"; filename="C:\Users\sdr\Desktop\test.csv"] Content-Type:[application/vnd.ms-excel]] C:\Users\sdr\Desktop\test.csv 2. What did you see instead? map[Content-Disposition:[form-data; name="file"; filename="C:\Users\sdr\Desktop\test.csv"] Content-Type:[application/vnd.ms-excel]] C:UserssdrDesktoptest.csv
@odeke-em

This comment has been minimized.

Copy link
Member

commented May 13, 2016

@sdicker8 I improved your code to produce a working sample that could be used for others to reproduce your bug run in the form of a client and server at https://github.com/odeke-em/bugs/tree/master/golang/15664 or in one place https://gist.github.com/odeke-em/46a8deba3ded6bb4f2169e2e80928442, or inlined below.
To run the server, just run

$ go run server.go

Then for the client

$ go run client.go <paths....>

However, I get filename to be contain the proper slashes when run on *NIX since I don't have access to Windows machines.
screen shot 2016-05-12 at 8 29 07 pm

Maybe that's a Windows thing?

Code inlined

  • client.go
package main

import (
    "bytes"
    "fmt"
    "io"
    "mime/multipart"
    "net/http"
    "os"
)

func exitIfErr(err error) {
    if err == nil {
        return
    }
    fmt.Fprintf(os.Stderr, "%v\n", err)
    os.Exit(-1)
}

func main() {
    if len(os.Args) < 2 {
        exitIfErr(fmt.Errorf("expecting atleast one arg"))
    }

    rest := os.Args[1:]
    for _, filename := range rest {
        f, err := os.Open(filename)
        exitIfErr(err)

        fields := map[string]string{
            "filename": filename,
        }
        res, err := multipartUpload("http://localhost:8090/validate", f, fields)
        _ = f.Close()
        exitIfErr(err)

        io.Copy(os.Stdout, res.Body)
        _ = res.Body.Close()
    }
}

func createFormFile(mw *multipart.Writer, filename string) (io.Writer, error) {
    return mw.CreateFormFile("file", filename)
}

func multipartUpload(destURL string, f io.Reader, fields map[string]string) (*http.Response, error) {
    if f == nil {
        return nil, fmt.Errorf("bodySource cannot be nil")
    }
    body := &bytes.Buffer{}
    writer := multipart.NewWriter(body)
    fw, err := createFormFile(writer, fields["filename"])
    if err != nil {
        return nil, fmt.Errorf("createFormFile %v", err)
    }

    n, err := io.Copy(fw, f)
    if err != nil && n < 1 {
        return nil, fmt.Errorf("copying fileWriter %v", err)
    }

    for k, v := range fields {
        _ = writer.WriteField(k, v)
    }

    err = writer.Close()
    if err != nil {
        return nil, fmt.Errorf("writerClose: %v", err)
    }

    req, err := http.NewRequest("POST", destURL, body)
    if err != nil {
        return nil, err
    }

    req.Header.Set("Content-Type", writer.FormDataContentType())

    if req.Close && req.Body != nil {
        defer req.Body.Close()
    }

    return http.DefaultClient.Do(req)
}
  • server.go
package main

import (
    "fmt"
    "log"
    "net/http"
    "os/exec"
)

const html = `
<html>
  Validation
  <form method="POST" action="/validate" enctype="multipart/form-data">
    <input type="file" name="file" />
    <br />
    <input type="submit" value="Send" />
  </form>
</html>
`

func validate(w http.ResponseWriter, r *http.Request) {
    if err := r.ParseMultipartForm(1 << 20); err != nil {
        fmt.Fprintf(w, "parseForm: %v\n", err)
        return
    }
    file, header, err := r.FormFile("file")
    if err != nil {
        fmt.Fprintf(w, "formFile retrieval %s\n", err)
        fmt.Println(err)
        return
    }
    defer file.Close()
    fmt.Fprintf(w, "%s\n", header.Header)
    fmt.Fprintf(w, "%s\n", header.Filename)
    fmt.Println(header.Header)
    fmt.Println(header.Filename)
}

func index(w http.ResponseWriter, r *http.Request) {
    fmt.Fprintf(w, html)
}

func main() {
    addr := ":8090"
    http.HandleFunc("/", index)
    http.HandleFunc("/validate", validate)
    if false { // This is your specific command, not present on *NIX
        go exec.Command("rundll32", "url.dll,FileProtocolHandler",
            fmt.Sprintf("http://localhost%s", addr)).Start()
    }
    if err := http.ListenAndServe(addr, nil); err != nil {
        log.Fatal(err)
    }
}
@sdicker8

This comment has been minimized.

Copy link
Author

commented May 13, 2016

Thanks odekm-em - I have actually used very similar code (to my original) on windows and Linux with go 1.5.2 and not had any problems.

@alexbrainman

This comment has been minimized.

Copy link
Member

commented May 14, 2016

@sdicker8 I cannot reproduce your problem with test created by @odeke-em (thank you for the test @odeke-em). But when I try to use browser as a client, the browser always sends just a file name, never full path. Please, let me know what I am doing wrong. Thank you.

Alex

@sdicker8

This comment has been minimized.

Copy link
Author

commented May 15, 2016

@alexbrainman Here is my code in a form that you should be able to just copy and run on windows - thanks.

Scott

package main

import (
    "fmt"
    "net/http"
    "os/exec"
)

func validate(w http.ResponseWriter, r *http.Request) {
    file, header, err := r.FormFile("file")
    if err != nil {
        fmt.Fprintf(w, "validate: %s\n", err)
        fmt.Println(err)
        return
    }
    defer file.Close()
    fmt.Fprintf(w, "%s\n", header.Header)
    fmt.Fprintf(w, "%s\n", header.Filename)
    fmt.Println(header.Header)
    fmt.Println(header.Filename)
}

func index(w http.ResponseWriter, r *http.Request) {
    fmt.Fprintf(w, html)
}

func main() {
    http.HandleFunc("/", index)
    http.HandleFunc("/validate", validate)
    go exec.Command("rundll32", "url.dll,FileProtocolHandler",
        "http://localhost:8090/").Start()
    http.ListenAndServe(":8090", nil)
}

var html = `<!DOCTYPE html>
<html>
<head>
<charset="utf-8">
<title>Validation</title>
<style type="text/css">
body{font-family:arial,sans-serif;margin-top:4em;margin-left:4em}
</style>
</head>
<body>
<h2>Validation</h2>
<p>Select and submit a file to validate.</p>
<form enctype="multipart/form-data" action="validate" method="post">
<input type="file" name="file" />
&nbsp;
<input type="submit" value="Submit"/>
</form>
</body>
</html>
`
@alexbrainman

This comment has been minimized.

Copy link
Member

commented May 16, 2016

@sdicker8 thank you for the code, but you're not telling me what is the problem with your code. I can compile your code, and run that program. But it outputs nothing. Do you expect your program to output something? What?

Also you're calling url.dll,FileProtocolHandler from your program. Is that related to your problem? Does your problem goes away if you remove call into url.dll,FileProtocolHandler?

Alex

@sdicker8

This comment has been minimized.

Copy link
Author

commented May 16, 2016

@alexbrainman When I run this code it opens a webpage with a form. When I then submit a file, which invokes FormFile, I get a filepath from Filename that does not contain slashes (e.g. C:UserssdrDesktoptest.csv). I am using 1.6.2 on windows.

I use the call to exec.Command(...).Start() as a convenience so that the initial page opens automatically. Removing it does not change the problem.

As an aside, I've been using Go for a couple of years now and I think it's awesome. Thank you very much for the time and effort you put into it.

Scott

@alexbrainman

This comment has been minimized.

Copy link
Member

commented May 18, 2016

When I run this code it opens a webpage with a form.

It does that here.

When I then submit a file, ...

I have this:

C:\dev\src\issues\issue15664>go version
go version go1.6.2 windows/386

C:\dev\src\issues\issue15664>type main.go
package main

import (
    "fmt"
    "net/http"
    "os/exec"
)

func validate(w http.ResponseWriter, r *http.Request) {
    file, header, err := r.FormFile("file")
    if err != nil {
        fmt.Fprintf(w, "validate: %s\n", err)
        fmt.Println(err)
        return
    }
    defer file.Close()
    fmt.Fprintf(w, "%s\n", header.Header)
    fmt.Fprintf(w, "%s\n", header.Filename)
    fmt.Println(header.Header)
    fmt.Println(header.Filename)
}

func index(w http.ResponseWriter, r *http.Request) {
    fmt.Fprintf(w, html)
}

func main() {
    http.HandleFunc("/", index)
    http.HandleFunc("/validate", validate)
    go exec.Command("rundll32", "url.dll,FileProtocolHandler",
        "http://localhost:8090/").Start()
    http.ListenAndServe(":8090", nil)
}

var html = `<!DOCTYPE html>
<html>
<head>
<charset="utf-8">
<title>Validation</title>
<style type="text/css">
body{font-family:arial,sans-serif;margin-top:4em;margin-left:4em}
</style>
</head>
<body>
<h2>Validation</h2>
<p>Select and submit a file to validate.</p>
<form enctype="multipart/form-data" action="validate" method="post">
<input type="file" name="file" />
&nbsp;
<input type="submit" value="Submit"/>
</form>
</body>
</html>
`
C:\dev\src\issues\issue15664>

.. which invokes FormFile, I get a filepath from Filename that does not contain slashes (e.g. C:UserssdrDesktoptest.csv). ...

I run your program. It opens my browser. I click "Choose File" button. That opens "Open file" dialogue. I navigate into c:\tmp directory and select A.TXT file in that directory. Now I am back to my broser, and it says "A.TXT" next to "Choose File" button. Next I click Submit button, and browser displays map[Content-Disposition:[form-data; name="file"; filename="AOM.TXT"] Content-Type:[text/plain]] AOM.TXT.

My program says this:

C:\dev\src\issues\issue15664>go run main.go
map[Content-Disposition:[form-data; name="file"; filename="AOM.TXT"] Content-Typ
e:[text/plain]]
AOM.TXT

... I am using 1.6.2 on windows.

Me too.

I use the call to exec.Command(...).Start() as a convenience so that the initial page opens automatically. Removing it does not change the problem.

That is fine. I kept your code as is, but I still don't see how I can make form filename include full path.

Wha am I doing wrong?

Alex

@sdicker8

This comment has been minimized.

Copy link
Author

commented May 18, 2016

@alexbrainman @odeke-em Argh - this appears to be a browser specific problem. When I use IE11 (in a corporate environment) I get 'filepaths' with the slashes problem. When I switch to Firefox I get just filenames like you Alex.

I just tested this on Linux/amd64(centos) - go1.6.2 - Firefox and got filenames. Emmanuel, on some flavor of *NIX and browser, got filepaths.

Thanks
Scott

edit: Emmanuel didn't use a browser - sorry.

@alexbrainman

This comment has been minimized.

Copy link
Member

commented May 19, 2016

@sdicker8 I can reproduce your problem with IE11. Unfortunately I don't have time to debug this - I will be travelling until mid June. But you can try it yourself. All you need to do is insert fmt.Printf into standard library code (and rebuild it) to find the culprit. Then we could decide what to do about it. I suspect the problem is in mime package, but you should start from the start net/http.

Alex

@sdicker8

This comment has been minimized.

Copy link
Author

commented May 19, 2016

@alexbrainman Will do - thanks

@bradfitz bradfitz added this to the Go1.8 milestone May 20, 2016

@sdicker8

This comment has been minimized.

Copy link
Author

commented May 22, 2016

@alexbrainman @bradfitz The backslashes in the filepath are ultimately removed in mime.consumeValue. I can provide the full function call path from request.FormFile if needed.

When IE is run in the 'Internet Zone', by using 127.0.0.1 instead of localhost, only the filename is returned, as expected, and not the (modified) filepath.

So, it appears we have a bit of a corner case where developers who are working with, or intend to use, localhost on IE may get sidetracked a bit. A note in the docs might suffice - but you guys are in a better position to make that call. Thank you for your time.

Scott

@alexbrainman

This comment has been minimized.

Copy link
Member

commented May 22, 2016

Thank you for investigating. I will look into it when I get back from my travelling.

Alex

@alexbrainman

This comment has been minimized.

Copy link
Member

commented Oct 3, 2016

I tried debugging this. If I apply

diff --git a/src/mime/mediatype.go b/src/mime/mediatype.go
index 1845401..fa599ca 100644
--- a/src/mime/mediatype.go
+++ b/src/mime/mediatype.go
@@ -102,6 +102,7 @@ func checkMediaTypeDisposition(s string) error {
 // The returned map, params, maps from the lowercase
 // attribute to the attribute value with its case preserved.
 func ParseMediaType(v string) (mediatype string, params map[string]string, err error) {
+   fmt.Printf("ParseMediaType(%q)\n", v)
    i := strings.Index(v, ";")
    if i == -1 {
        i = len(v)
@@ -127,6 +128,7 @@ func ParseMediaType(v string) (mediatype string, params map[string]string, err e
            break
        }
        key, value, rest := consumeMediaParam(v)
+       fmt.Printf("key=%q value=%q\n", key, value)
        if key == "" {
            if strings.TrimSpace(rest) == ";" {
                // Ignore trailing semicolons.

against db82cf4, the program will output

C:\tmp>u:\test
ParseMediaType("multipart/form-data; boundary=---------------------------7e0f124280332")
key="boundary" value="---------------------------7e0f124280332"
ParseMediaType("multipart/form-data; boundary=---------------------------7e0f124280332")
key="boundary" value="---------------------------7e0f124280332"
ParseMediaType("form-data; name=\"file\"; filename=\"C:\\dev\\go\\robots.txt\"")
key="name" value="file"
key="filename" value="C:devgorobots.txt"
map[Content-Disposition:[form-data; name="file"; filename="C:\dev\go\robots.txt"
] Content-Type:[text/plain]]
C:devgorobots.txt

So the problem is in mime/consumeValue. IE does not escape \ in the filename. And, unlike all other browsers I tried, IE sends full path - which is, probably, not secure.

I googled for solutions:
https://java.net/jira/si/jira.issueviews:issue-html/JERSEY-759/JERSEY-759.html
mscdex/busboy#24
http://jersey.576304.n2.nabble.com/Jersey-truncating-the-slashes-from-the-uploaded-file-name-td5984041.html

Maybe we should try and return the last element of filename path. I am not sure.

I will let @bradfitz decide here.

Alex

@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2016

It seems OK to me to try to accommodate MSIE here. What MSIE is sending is pretty easy to distinguish from what other clients sends. That is, there's not really ambiguity here: nothing would go out of its way to put unnecessary backslashes into the encoding of "c:devgorobots.txt" to produce "c:\dev\go\robots.txt". The intention behind the latter seems quite unambiguous.

I'll send a CL and we'll see.

@gopherbot

This comment has been minimized.

Copy link

commented Oct 26, 2016

CL https://golang.org/cl/32175 mentions this issue.

@gopherbot gopherbot closed this in eac5950 Oct 27, 2016

@golang golang locked and limited conversation to collaborators Oct 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.