Skip to content

Use fs.FS interface #167

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use fs.FS interface #167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 27 additions & 44 deletions dlna/dms/cds.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import (
"encoding/json"
"encoding/xml"
"fmt"
"io/fs"
"io/ioutil"
"net/http"
"net/url"
"os"
"path"
"path/filepath"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -73,7 +73,7 @@ func readDynamicStream(metadataPath string) (*dmsDynamicMediaItem, error) {
return &re, nil
}

func (me *contentDirectoryService) cdsObjectDynamicStreamToUpnpavObject(cdsObject object, fileInfo os.FileInfo, host, userAgent string) (ret interface{}, err error) {
func (me *contentDirectoryService) cdsObjectDynamicStreamToUpnpavObject(cdsObject object, fileInfo fs.FileInfo, host, userAgent string) (ret interface{}, err error) {
// at this point we know that entryFilePath points to a .dms.json file; slurp and parse
dmsMediaItem, err := readDynamicStream(cdsObject.FilePath())
if err != nil {
Expand All @@ -100,12 +100,12 @@ func (me *contentDirectoryService) cdsObjectDynamicStreamToUpnpavObject(cdsObjec
obj.AlbumArtURI = iconURI

switch dmsMediaItem.Type {
case "video":
obj.Class = "object.item.videoItem"
case "audio":
obj.Class = "object.item.audioItem"
default:
obj.Class = "object.item.videoItem"
case "video":
obj.Class = "object.item.videoItem"
case "audio":
obj.Class = "object.item.audioItem"
default:
obj.Class = "object.item.videoItem"
}

obj.Title = dmsMediaItem.Title
Expand Down Expand Up @@ -170,7 +170,7 @@ func (me *contentDirectoryService) cdsObjectDynamicStreamToUpnpavObject(cdsObjec
// returned if the entry is not of interest.
func (me *contentDirectoryService) cdsObjectToUpnpavObject(
cdsObject object,
fileInfo os.FileInfo,
fileInfo fs.FileInfo,
host, userAgent string,
) (ret interface{}, err error) {
entryFilePath := cdsObject.FilePath()
Expand Down Expand Up @@ -204,7 +204,7 @@ func (me *contentDirectoryService) cdsObjectToUpnpavObject(
me.Logger.Printf("%s ignored: non-regular file", cdsObject.FilePath())
return
}
mimeType, err := MimeTypeByPath(entryFilePath)
mimeType, err := MimeTypeByPath(me.FS, entryFilePath)
if err != nil {
return
}
Expand Down Expand Up @@ -332,7 +332,7 @@ func (me *contentDirectoryService) readContainer(
// TODO(anacrolix): Dig up why this special cast was added.
FoldersLast: strings.Contains(userAgent, `AwoX/1.1`),
}
sfis.fileInfoSlice, err = o.readDir()
sfis.fileInfoSlice, err = o.readDir(me.FS)
if err != nil {
return
}
Expand Down Expand Up @@ -366,13 +366,9 @@ func (me *contentDirectoryService) objectFromID(id string) (o object, err error)
return
}
if o.Path == "0" {
o.Path = "/"
o.Path = "."
}
o.Path = path.Clean(o.Path)
if !path.IsAbs(o.Path) {
err = fmt.Errorf("bad ObjectID %v", o.Path)
return
}
o.RootObjectPath = me.RootObjectPath
return
}
Expand Down Expand Up @@ -434,8 +430,8 @@ func (me *contentDirectoryService) Handle(action string, argsXML []byte, r *http
var ret interface{}
var err error
if me.OnBrowseMetadata == nil {
var fileInfo os.FileInfo
fileInfo, err = os.Stat(obj.FilePath())
var fileInfo fs.FileInfo
fileInfo, err = fs.Stat(me.FS, obj.FilePath())
if err != nil {
if os.IsNotExist(err) {
return nil, &upnp.Error{
Expand Down Expand Up @@ -502,7 +498,7 @@ type object struct {

func (me *contentDirectoryService) isOfInterest(
cdsObject object,
fileInfo os.FileInfo,
fileInfo fs.FileInfo,
) (ret bool, err error) {
entryFilePath := cdsObject.FilePath()
ignored, err := me.IgnorePath(entryFilePath)
Expand All @@ -526,7 +522,7 @@ func (me *contentDirectoryService) isOfInterest(
return
}

mimeType, err := MimeTypeByPath(entryFilePath)
mimeType, err := MimeTypeByPath(me.FS, entryFilePath)
if err != nil {
return
}
Expand All @@ -539,7 +535,7 @@ func (me *contentDirectoryService) isOfInterest(

// Returns the number of children this object has, such as for a container.
func (cds *contentDirectoryService) objectChildCount(me object) (count int) {
fileInfoSlice, err := me.readDir()
fileInfoSlice, err := me.readDir(cds.FS)
if err != nil {
return
}
Expand All @@ -562,13 +558,13 @@ func (cds *contentDirectoryService) objectChildCount(me object) (count int) {
// directory succeeds. Returns true on first hit.
func (me *contentDirectoryService) objectHasChildren(
cdsObject object,
fileInfo os.FileInfo,
fileInfo fs.FileInfo,
) (ret bool, err error) {
if !fileInfo.IsDir() {
panic("Expected directory")
}

files, err := cdsObject.readDir()
files, err := cdsObject.readDir(me.FS)
if err != nil {
return
}
Expand All @@ -588,22 +584,19 @@ func (me *contentDirectoryService) objectHasChildren(

// Returns the actual local filesystem path for the object.
func (o *object) FilePath() string {
return filepath.Join(o.RootObjectPath, filepath.FromSlash(o.Path))
return path.Join(o.RootObjectPath, path.Clean(o.Path))
}

// Returns the ObjectID for the object. This is used in various ContentDirectory actions.
func (o object) ID() string {
if !path.IsAbs(o.Path) {
log.Panicf("Relative object path: %s", o.Path)
}
if len(o.Path) == 1 {
return "0"
}
return url.QueryEscape(o.Path)
}

func (o *object) IsRoot() bool {
return o.Path == "/"
return o.Path == "."
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about changing this part? Same with the other usage. Can you make / rooted in whatever subtree fs.FS is?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The . (dot) in the context of fs.FS generally refers to the root of the file system represented by that specific fs.FS instance. It is not necessarily the current working directory of the operating system, but rather the base or starting point of the abstract file system being exposed by the fs.FS implementation.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, the interface I was going for was to expose / as the root of the FS exposed to the device. I understand . would be used on the server side that makes sense.

}

// Returns the object's parent ObjectID. Fortunately it can be deduced from the
Expand All @@ -618,31 +611,21 @@ func (o object) ParentID() string {

// This function exists rather than just calling os.(*File).Readdir because I
// want to stat(), not lstat() each entry.
func (o *object) readDir() (fis []os.FileInfo, err error) {
dirPath := o.FilePath()
dirFile, err := os.Open(dirPath)
func (o *object) readDir(fsys fs.FS) (fis []fs.FileInfo, err error) {
dirFile, err := fs.ReadDir(fsys, o.Path)
if err != nil {
return
}
defer dirFile.Close()
var dirContent []string
dirContent, err = dirFile.Readdirnames(-1)
if err != nil {
return
}
fis = make([]os.FileInfo, 0, len(dirContent))
for _, file := range dirContent {
fi, err := os.Stat(filepath.Join(dirPath, file))
if err != nil {
continue
}
fis = make([]fs.FileInfo, 0, len(dirFile))
for _, file := range dirFile {
fi, _ := file.Info()
fis = append(fis, fi)
}
return
}

type sortableFileInfoSlice struct {
fileInfoSlice []os.FileInfo
fileInfoSlice []fs.FileInfo
FoldersLast bool
}

Expand Down
41 changes: 20 additions & 21 deletions dlna/dms/dms.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"io/ioutil"
"math/rand"
"net"
Expand Down Expand Up @@ -280,6 +281,7 @@ type Server struct {
TranscodeLogPattern string
Logger log.Logger
eventingLogger log.Logger
FS fs.FS
}

// UPnP SOAP service.
Expand Down Expand Up @@ -638,7 +640,7 @@ func (me *Server) serviceControlHandler(w http.ResponseWriter, r *http.Request)
}

func safeFilePath(root, given string) string {
return filepath.Join(root, filepath.FromSlash(path.Clean("/" + given))[1:])
return path.Join(root, path.Clean(given))
}

func (s *Server) filePath(_path string) string {
Expand Down Expand Up @@ -864,15 +866,15 @@ func (server *Server) initMux(mux *http.ServeMux) {
} else {
k = r.URL.Query().Get("transcode")
}
mimeType, err := MimeTypeByPath(filePath)
mimeType, err := MimeTypeByPath(server.FS, filePath)
if k == "" || mimeType.IsImage() {
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
w.Header().Set("Content-Type", string(mimeType))
w.Header().Set("Content-Disposition", "attachment; filename="+strconv.Quote(path.Base(filePath)))
http.ServeFile(w, r, filePath)
http.ServeFileFS(w, r, server.FS, filePath)
return
}
if server.NoTranscode {
Expand Down Expand Up @@ -939,6 +941,11 @@ func (s *Server) initServices() (err error) {
}

func (srv *Server) Init() (err error) {
if srv.FS == nil {
fsys := os.DirFS(srv.RootObjectPath)
srv.FS = fsys
}
srv.RootObjectPath = "."
srv.eventingLogger = srv.Logger.WithNames("eventing")
srv.eventingLogger.Levelf(log.Debug, "hello %v", "world")
if err = srv.initServices(); err != nil {
Expand Down Expand Up @@ -1071,19 +1078,15 @@ func (me *Server) location(ip net.IP) string {

// Can return nil info with nil err if an earlier Probe gave an error.
func (srv *Server) ffmpegProbe(path string) (info *ffprobe.Info, err error) {
// We don't want relative paths in the cache.
path, err = filepath.Abs(path)
if err != nil {
return
}
fi, err := os.Stat(path)
fi, err := fs.Stat(srv.FS, path)
if err != nil {
return
}
key := ffmpegInfoCacheKey{path, fi.ModTime().UnixNano()}
value, ok := srv.FFProbeCache.Get(key)
if !ok {
info, err = ffprobe.Run(path)
uri := fmt.Sprintf("http://127.0.0.1:%d%s?path=%s", srv.httpPort(), resPath, path)
info, err = ffprobe.Run(uri)
err = suppressFFmpegProbeDataErrors(err)
srv.FFProbeCache.Set(key, info)
return
Expand All @@ -1094,19 +1097,16 @@ func (srv *Server) ffmpegProbe(path string) (info *ffprobe.Info, err error) {

// IgnorePath detects if a file/directory should be ignored.
func (server *Server) IgnorePath(path string) (bool, error) {
if !filepath.IsAbs(path) {
return false, fmt.Errorf("Path must be absolute: %s", path)
}
if server.IgnoreHidden {
if hidden, err := isHiddenPath(path); err != nil {
if hidden, err := isHiddenPath(server.FS, path); err != nil {
return false, err
} else if hidden {
log.Print(path, " ignored: hidden")
return true, nil
}
}
if server.IgnoreUnreadable {
if readable, err := isReadablePath(path); err != nil {
if readable, err := isReadablePath(server.FS, path); err != nil {
return false, err
} else if !readable {
log.Print(path, " ignored: unreadable")
Expand All @@ -1124,13 +1124,12 @@ func (server *Server) IgnorePath(path string) (bool, error) {
return false, nil
}

func tryToOpenPath(path string) (bool, error) {
func isReadablePath(fsys fs.FS, path string) (bool, error) {
// Ugly but portable way to check if we can open a file/directory
if fh, err := os.Open(path); err == nil {
fh.Close()
return true, nil
} else if !os.IsPermission(err) {
f, err := fsys.Open(path)
if err != nil {
return false, err
}
return false, nil
f.Close()
return true, nil
}
24 changes: 10 additions & 14 deletions dlna/dms/dms_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,19 @@
package dms

import (
"io/fs"
"path/filepath"
"strings"

"golang.org/x/sys/unix"
)

func isHiddenPath(path string) (bool, error) {
return strings.Contains(path, "/."), nil
}

func isReadablePath(path string) (bool, error) {
err := unix.Access(path, unix.R_OK)
switch err {
case nil:
return true, nil
case unix.EACCES:
func isHiddenPath(fsys fs.FS, path string) (bool, error) {
if path == "." {
return false, nil
default:
return false, err
}
base := filepath.Base(path)
if strings.HasPrefix(base, ".") {
return true, nil
}

return isHiddenPath(fsys, filepath.Dir(path))
}
16 changes: 8 additions & 8 deletions dlna/dms/dms_unix_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,17 @@ import "testing"

func TestIsHiddenPath(t *testing.T) {
data := map[string]bool{
"/some/path": false,
"/some/foo.bar": false,
"/some/path/.hidden": true,
"/some/.hidden/path": true,
"/.hidden/path": true,
"some/path": false,
"some/foo.bar": false,
"some/path/.hidden": true,
"some/.hidden/path": true,
".hidden/path": true,
}
for path, expected := range data {
if actual, err := isHiddenPath(path); err != nil {
t.Errorf("isHiddenPath(%v) returned unexpected error: %s", path, err)
if actual, err := isHiddenPath(nil, path); err != nil {
t.Errorf("isHiddenPath(nil, %v) returned unexpected error: %s", path, err)
} else if expected != actual {
t.Errorf("isHiddenPath(%v), expected %v, got %v", path, expected, actual)
t.Errorf("isHiddenPath(nil, %v), expected %v, got %v", path, expected, actual)
}
}
}
Loading