-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Infer content type in alloc fs stat endpoint #5907
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,9 @@ import ( | |
"sync" | ||
"time" | ||
|
||
"net/http" | ||
"strings" | ||
|
||
hclog "github.com/hashicorp/go-hclog" | ||
multierror "github.com/hashicorp/go-multierror" | ||
cstructs "github.com/hashicorp/nomad/client/structs" | ||
|
@@ -392,15 +395,41 @@ func (d *AllocDir) Stat(path string) (*cstructs.AllocFileInfo, error) { | |
return nil, err | ||
} | ||
|
||
contentType := detectContentType(info, p) | ||
|
||
return &cstructs.AllocFileInfo{ | ||
Size: info.Size(), | ||
Name: info.Name(), | ||
IsDir: info.IsDir(), | ||
FileMode: info.Mode().String(), | ||
ModTime: info.ModTime(), | ||
Size: info.Size(), | ||
Name: info.Name(), | ||
IsDir: info.IsDir(), | ||
FileMode: info.Mode().String(), | ||
ModTime: info.ModTime(), | ||
ContentType: contentType, | ||
}, nil | ||
} | ||
|
||
// detectContentType tries to infer the file type by reading the first | ||
// 512 bytes of the file. Json file extensions are special cased. | ||
func detectContentType(fileInfo os.FileInfo, path string) string { | ||
contentType := "unknown" | ||
if !fileInfo.IsDir() { | ||
f, err := os.Open(path) | ||
// Best effort content type detection | ||
// We ignore errors because this is optional information | ||
if err == nil { | ||
fileBytes := make([]byte, 512) | ||
_, err := f.Read(fileBytes) | ||
if err == nil { | ||
contentType = http.DetectContentType(fileBytes) | ||
} | ||
} | ||
} | ||
// Special case json files | ||
if strings.HasSuffix(path, ".json") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that this is a cheaper check, I'd consider checking it first. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably an unnecessary caution, but it's possible that a |
||
contentType = "application/json" | ||
} | ||
return contentType | ||
} | ||
|
||
// ReadAt returns a reader for a file at the path relative to the alloc dir | ||
func (d *AllocDir) ReadAt(path string, offset int64) (io.ReadCloser, error) { | ||
if escapes, err := structs.PathEscapesAllocDir("", path); err != nil { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -472,3 +472,18 @@ func TestPathFuncs(t *testing.T) { | |
t.Errorf("%q is not empty. empty=%v error=%v", dir, empty, err) | ||
} | ||
} | ||
|
||
func TestAllocDir_DetectContentType(t *testing.T) { | ||
require := require.New(t) | ||
imgPath := "input/image.png" | ||
fileInfo, err := os.Stat(imgPath) | ||
require.Nil(err) | ||
res := detectContentType(fileInfo, imgPath) | ||
require.Equal("image/png", res) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. more nit picking - seems like a good candidate for table testing - I would test go source files and maybe some |
||
|
||
jsonPath := "input/test.json" | ||
fileInfo, err = os.Stat(jsonPath) | ||
require.Nil(err) | ||
res = detectContentType(fileInfo, jsonPath) | ||
require.Equal("application/json", res) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"test":"test" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would default to
application/octet-stream
and consider returning rather than assigning to variable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to this. It's essentially how I'd interpret unknown anyway, might as well use the proper generic binary type.