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
Closed

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

sdicker8 opened this issue May 12, 2016 · 15 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@sdicker8
Copy link

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
Copy link
Member

@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
Copy link
Author

sdicker8 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
Copy link
Member

@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
Copy link
Author

@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
Copy link
Member

@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
Copy link
Author

@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
Copy link
Member

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
Copy link
Author

sdicker8 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
Copy link
Member

@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
Copy link
Author

@alexbrainman Will do - thanks

@bradfitz bradfitz added this to the Go1.8 milestone May 20, 2016
@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 20, 2016
@sdicker8
Copy link
Author

@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
Copy link
Member

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

Alex

@alexbrainman
Copy link
Member

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
Copy link
Contributor

rsc 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
Copy link

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

@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.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants