diff --git a/webdav/file.go b/webdav/file.go index b4af8781..31b1e9d0 100644 --- a/webdav/file.go +++ b/webdav/file.go @@ -5,6 +5,7 @@ package webdav import ( + "encoding/xml" "io" "net/http" "os" @@ -44,6 +45,9 @@ type FileSystem interface { // A File is returned by a FileSystem's OpenFile method and can be served by a // Handler. +// +// A File may optionally implement the DeadPropsHolder interface, if it can +// load and save dead properties. type File interface { http.File io.Writer @@ -401,10 +405,11 @@ type memFSNode struct { // children is protected by memFS.mu. children map[string]*memFSNode - mu sync.Mutex - data []byte - mode os.FileMode - modTime time.Time + mu sync.Mutex + data []byte + mode os.FileMode + modTime time.Time + deadProps map[xml.Name]Property } func (n *memFSNode) stat(name string) *memFileInfo { @@ -418,6 +423,39 @@ func (n *memFSNode) stat(name string) *memFileInfo { } } +func (n *memFSNode) DeadProps() map[xml.Name]Property { + n.mu.Lock() + defer n.mu.Unlock() + if len(n.deadProps) == 0 { + return nil + } + ret := make(map[xml.Name]Property, len(n.deadProps)) + for k, v := range n.deadProps { + ret[k] = v + } + return ret +} + +func (n *memFSNode) Patch(patches []Proppatch) ([]Propstat, error) { + n.mu.Lock() + defer n.mu.Unlock() + pstat := Propstat{Status: http.StatusOK} + for _, patch := range patches { + for _, p := range patch.Props { + pstat.Props = append(pstat.Props, Property{XMLName: p.XMLName}) + if patch.Remove { + delete(n.deadProps, p.XMLName) + continue + } + if n.deadProps == nil { + n.deadProps = map[xml.Name]Property{} + } + n.deadProps[p.XMLName] = p + } + } + return []Propstat{pstat}, nil +} + type memFileInfo struct { name string size int64 @@ -443,6 +481,12 @@ type memFile struct { pos int } +// A *memFile implements the optional DeadPropsHolder interface. +var _ DeadPropsHolder = (*memFile)(nil) + +func (f *memFile) DeadProps() map[xml.Name]Property { return f.n.DeadProps() } +func (f *memFile) Patch(patches []Proppatch) ([]Propstat, error) { return f.n.Patch(patches) } + func (f *memFile) Close() error { return nil } diff --git a/webdav/litmus_test_server.go b/webdav/litmus_test_server.go index 439d0ccc..8aea9995 100644 --- a/webdav/litmus_test_server.go +++ b/webdav/litmus_test_server.go @@ -37,7 +37,7 @@ func main() { http.Handle("/", &webdav.Handler{ FileSystem: fs, LockSystem: ls, - PropSystem: webdav.NewMemPS(fs, ls, webdav.ReadWrite), + PropSystem: webdav.NewMemPS(fs, ls), Logger: func(r *http.Request, err error) { litmus := r.Header.Get("X-Litmus") if len(litmus) > 19 { diff --git a/webdav/prop.go b/webdav/prop.go index 9601e76d..2887008f 100644 --- a/webdav/prop.go +++ b/webdav/prop.go @@ -13,9 +13,12 @@ import ( "os" "path/filepath" "strconv" - "sync" ) +// TODO(nigeltao): eliminate the concept of a configurable PropSystem, and the +// MemPS implementation. Properties are now the responsibility of a File +// implementation, not a PropSystem implementation. + // PropSystem manages the properties of named resources. It allows finding // and setting properties as defined in RFC 4918. // @@ -54,8 +57,6 @@ type PropSystem interface { // // Note that the WebDAV RFC requires either all patches to succeed or none. Patch(name string, patches []Proppatch) ([]Propstat, error) - - // TODO(rost) COPY/MOVE/DELETE. } // Proppatch describes a property update instruction as defined in RFC 4918. @@ -91,46 +92,66 @@ type Propstat struct { ResponseDescription string } +// makePropstats returns a slice containing those of x and y whose Props slice +// is non-empty. If both are empty, it returns a slice containing an otherwise +// zero Propstat whose HTTP status code is 200 OK. +func makePropstats(x, y Propstat) []Propstat { + pstats := make([]Propstat, 0, 2) + if len(x.Props) != 0 { + pstats = append(pstats, x) + } + if len(y.Props) != 0 { + pstats = append(pstats, y) + } + if len(pstats) == 0 { + pstats = append(pstats, Propstat{ + Status: http.StatusOK, + }) + } + return pstats +} + +// DeadPropsHolder holds the dead properties of a resource. +// +// Dead properties are those properties that are explicitly defined. In +// comparison, live properties, such as DAV:getcontentlength, are implicitly +// defined by the underlying resource, and cannot be explicitly overridden or +// removed. See the Terminology section of +// http://www.webdav.org/specs/rfc4918.html#rfc.section.3 +// +// There is a whitelist of the names of live properties. This package handles +// all live properties, and will only pass non-whitelisted names to the Patch +// method of DeadPropsHolder implementations. +type DeadPropsHolder interface { + // DeadProps returns a copy of the dead properties held. + DeadProps() map[xml.Name]Property + + // Patch patches the dead properties held. + // + // Patching is atomic; either all or no patches succeed. It returns (nil, + // non-nil) if an internal server error occurred, otherwise the Propstats + // collectively contain one Property for each proposed patch Property. If + // all patches succeed, Patch returns a slice of length one and a Propstat + // element with a 200 OK HTTP status code. If none succeed, for reasons + // other than an internal server error, no Propstat has status 200 OK. + // + // For more details on when various HTTP status codes apply, see + // http://www.webdav.org/specs/rfc4918.html#PROPPATCH-status + Patch([]Proppatch) ([]Propstat, error) +} + // memPS implements an in-memory PropSystem. It supports all of the mandatory // live properties of RFC 4918. type memPS struct { fs FileSystem ls LockSystem - m Mutability - - mu sync.RWMutex - nodes map[string]*memPSNode } -// memPSNode stores the dead properties of a resource. -type memPSNode struct { - mu sync.RWMutex - deadProps map[xml.Name]Property -} - -// BUG(rost): In this development version, the in-memory property system does -// not handle COPY/MOVE/DELETE requests. As a result, dead properties are not -// released if the according DAV resource is deleted or moved. It is not -// recommended to use a read-writeable property system in production. - -// Mutability indicates the mutability of a property system. -type Mutability bool - -const ( - ReadOnly = Mutability(false) - ReadWrite = Mutability(true) -) - -// NewMemPS returns a new in-memory PropSystem implementation. A read-only -// property system rejects all patches. A read-writeable property system -// stores arbitrary properties but refuses to change any DAV: property -// specified in RFC 4918. It imposes no limit on the size of property values. -func NewMemPS(fs FileSystem, ls LockSystem, m Mutability) PropSystem { +// NewMemPS returns a new in-memory PropSystem implementation. +func NewMemPS(fs FileSystem, ls LockSystem) PropSystem { return &memPS{ - fs: fs, - ls: ls, - m: m, - nodes: make(map[string]*memPSNode), + fs: fs, + ls: ls, } } @@ -185,79 +206,75 @@ var liveProps = map[xml.Name]struct { } func (ps *memPS) Find(name string, propnames []xml.Name) ([]Propstat, error) { - ps.mu.RLock() - defer ps.mu.RUnlock() - - fi, err := ps.fs.Stat(name) + f, err := ps.fs.OpenFile(name, os.O_RDONLY, 0) if err != nil { return nil, err } + defer f.Close() + fi, err := f.Stat() + if err != nil { + return nil, err + } + isDir := fi.IsDir() - // Lookup the dead properties of this resource. It's OK if there are none. - n, ok := ps.nodes[name] - if ok { - n.mu.RLock() - defer n.mu.RUnlock() + var deadProps map[xml.Name]Property + if dph, ok := f.(DeadPropsHolder); ok { + deadProps = dph.DeadProps() } - pm := make(map[int]Propstat) + pstatOK := Propstat{Status: http.StatusOK} + pstatNotFound := Propstat{Status: http.StatusNotFound} for _, pn := range propnames { - // If this node has dead properties, check if they contain pn. - if n != nil { - if dp, ok := n.deadProps[pn]; ok { - pstat := pm[http.StatusOK] - pstat.Props = append(pstat.Props, dp) - pm[http.StatusOK] = pstat - continue - } + // If this file has dead properties, check if they contain pn. + if dp, ok := deadProps[pn]; ok { + pstatOK.Props = append(pstatOK.Props, dp) + continue } // Otherwise, it must either be a live property or we don't know it. - p := Property{XMLName: pn} - s := http.StatusNotFound - if prop := liveProps[pn]; prop.findFn != nil && (prop.dir || !fi.IsDir()) { - xmlvalue, err := prop.findFn(ps, name, fi) + if prop := liveProps[pn]; prop.findFn != nil && (prop.dir || !isDir) { + innerXML, err := prop.findFn(ps, name, fi) if err != nil { return nil, err } - s = http.StatusOK - p.InnerXML = []byte(xmlvalue) + pstatOK.Props = append(pstatOK.Props, Property{ + XMLName: pn, + InnerXML: []byte(innerXML), + }) + } else { + pstatNotFound.Props = append(pstatNotFound.Props, Property{ + XMLName: pn, + }) } - pstat := pm[s] - pstat.Props = append(pstat.Props, p) - pm[s] = pstat } - - pstats := make([]Propstat, 0, len(pm)) - for s, pstat := range pm { - pstat.Status = s - pstats = append(pstats, pstat) - } - return pstats, nil + return makePropstats(pstatOK, pstatNotFound), nil } func (ps *memPS) Propnames(name string) ([]xml.Name, error) { - fi, err := ps.fs.Stat(name) + f, err := ps.fs.OpenFile(name, os.O_RDONLY, 0) if err != nil { return nil, err } + defer f.Close() + fi, err := f.Stat() + if err != nil { + return nil, err + } + isDir := fi.IsDir() - propnames := make([]xml.Name, 0, len(liveProps)) + var deadProps map[xml.Name]Property + if dph, ok := f.(DeadPropsHolder); ok { + deadProps = dph.DeadProps() + } + + propnames := make([]xml.Name, 0, len(liveProps)+len(deadProps)) for pn, prop := range liveProps { - if prop.findFn != nil && (prop.dir || !fi.IsDir()) { + if prop.findFn != nil && (prop.dir || !isDir) { propnames = append(propnames, pn) } } - - ps.mu.RLock() - defer ps.mu.RUnlock() - if n, ok := ps.nodes[name]; ok { - n.mu.RLock() - defer n.mu.RUnlock() - for pn := range n.deadProps { - propnames = append(propnames, pn) - } + for pn := range deadProps { + propnames = append(propnames, pn) } - return propnames, nil } @@ -280,61 +297,65 @@ func (ps *memPS) Allprop(name string, include []xml.Name) ([]Propstat, error) { } func (ps *memPS) Patch(name string, patches []Proppatch) ([]Propstat, error) { - // A DELETE/COPY/MOVE might fly in, so we need to keep all nodes locked until - // the end of this PROPPATCH. - ps.mu.Lock() - defer ps.mu.Unlock() - n, ok := ps.nodes[name] - if !ok { - n = &memPSNode{deadProps: make(map[xml.Name]Property)} + conflict := false +loop: + for _, patch := range patches { + for _, p := range patch.Props { + if _, ok := liveProps[p.XMLName]; ok { + conflict = true + break loop + } + } + } + if conflict { + pstatForbidden := Propstat{ + Status: http.StatusForbidden, + XMLError: ``, + } + pstatFailedDep := Propstat{ + Status: StatusFailedDependency, + } + for _, patch := range patches { + for _, p := range patch.Props { + if _, ok := liveProps[p.XMLName]; ok { + pstatForbidden.Props = append(pstatForbidden.Props, Property{XMLName: p.XMLName}) + } else { + pstatFailedDep.Props = append(pstatFailedDep.Props, Property{XMLName: p.XMLName}) + } + } + } + return makePropstats(pstatForbidden, pstatFailedDep), nil } - n.mu.Lock() - defer n.mu.Unlock() - _, err := ps.fs.Stat(name) + f, err := ps.fs.OpenFile(name, os.O_RDWR, 0) if err != nil { return nil, err } - - // Perform a dry-run to identify any patch conflicts. A read-only property - // system always fails at this stage. - pm := make(map[int]Propstat) + defer f.Close() + if dph, ok := f.(DeadPropsHolder); ok { + ret, err := dph.Patch(patches) + if err != nil { + return nil, err + } + // http://www.webdav.org/specs/rfc4918.html#ELEMENT_propstat says that + // "The contents of the prop XML element must only list the names of + // properties to which the result in the status element applies." + for _, pstat := range ret { + for i, p := range pstat.Props { + pstat.Props[i] = Property{XMLName: p.XMLName} + } + } + return ret, nil + } + // The file doesn't implement the optional DeadPropsHolder interface, so + // all patches are forbidden. + pstat := Propstat{Status: http.StatusForbidden} for _, patch := range patches { for _, p := range patch.Props { - s := http.StatusOK - if _, ok := liveProps[p.XMLName]; ok || ps.m == ReadOnly { - s = http.StatusForbidden - } - pstat := pm[s] pstat.Props = append(pstat.Props, Property{XMLName: p.XMLName}) - pm[s] = pstat } } - // Based on the dry-run, either apply the patches or handle conflicts. - if _, ok = pm[http.StatusOK]; ok { - if len(pm) == 1 { - for _, patch := range patches { - for _, p := range patch.Props { - if patch.Remove { - delete(n.deadProps, p.XMLName) - } else { - n.deadProps[p.XMLName] = p - } - } - } - ps.nodes[name] = n - } else { - pm[StatusFailedDependency] = pm[http.StatusOK] - delete(pm, http.StatusOK) - } - } - - pstats := make([]Propstat, 0, len(pm)) - for s, pstat := range pm { - pstat.Status = s - pstats = append(pstats, pstat) - } - return pstats, nil + return []Propstat{pstat}, nil } func (ps *memPS) findResourceType(name string, fi os.FileInfo) (string, error) { diff --git a/webdav/prop_test.go b/webdav/prop_test.go index bd284e0e..eaca7c81 100644 --- a/webdav/prop_test.go +++ b/webdav/prop_test.go @@ -8,6 +8,7 @@ import ( "encoding/xml" "fmt" "net/http" + "os" "reflect" "sort" "testing" @@ -49,10 +50,10 @@ func TestMemPS(t *testing.T) { } testCases := []struct { - desc string - mutability Mutability - buildfs []string - propOp []propOp + desc string + noDeadProps bool + buildfs []string + propOp []propOp }{{ desc: "propname", buildfs: []string{"mkdir /dir", "touch /file"}, @@ -238,9 +239,9 @@ func TestMemPS(t *testing.T) { }}, }}, }, { - desc: "proppatch property on read-only property system", - buildfs: []string{"mkdir /dir"}, - mutability: ReadOnly, + desc: "proppatch property on no-dead-properties file system", + buildfs: []string{"mkdir /dir"}, + noDeadProps: true, propOp: []propOp{{ op: "proppatch", name: "/dir", @@ -264,16 +265,16 @@ func TestMemPS(t *testing.T) { }}, }}, wantPropstats: []Propstat{{ - Status: http.StatusForbidden, + Status: http.StatusForbidden, + XMLError: ``, Props: []Property{{ XMLName: xml.Name{Space: "DAV:", Local: "getetag"}, }}, }}, }}, }, { - desc: "proppatch dead property", - buildfs: []string{"mkdir /dir"}, - mutability: ReadWrite, + desc: "proppatch dead property", + buildfs: []string{"mkdir /dir"}, propOp: []propOp{{ op: "proppatch", name: "/dir", @@ -302,9 +303,8 @@ func TestMemPS(t *testing.T) { }}, }}, }, { - desc: "proppatch dead property with failed dependency", - buildfs: []string{"mkdir /dir"}, - mutability: ReadWrite, + desc: "proppatch dead property with failed dependency", + buildfs: []string{"mkdir /dir"}, propOp: []propOp{{ op: "proppatch", name: "/dir", @@ -320,7 +320,8 @@ func TestMemPS(t *testing.T) { }}, }}, wantPropstats: []Propstat{{ - Status: http.StatusForbidden, + Status: http.StatusForbidden, + XMLError: ``, Props: []Property{{ XMLName: xml.Name{Space: "DAV:", Local: "displayname"}, }}, @@ -342,9 +343,8 @@ func TestMemPS(t *testing.T) { }}, }}, }, { - desc: "proppatch remove dead property", - buildfs: []string{"mkdir /dir"}, - mutability: ReadWrite, + desc: "proppatch remove dead property", + buildfs: []string{"mkdir /dir"}, propOp: []propOp{{ op: "proppatch", name: "/dir", @@ -418,9 +418,8 @@ func TestMemPS(t *testing.T) { }}, }}, }, { - desc: "propname with dead property", - buildfs: []string{"touch /file"}, - mutability: ReadWrite, + desc: "propname with dead property", + buildfs: []string{"touch /file"}, propOp: []propOp{{ op: "proppatch", name: "/file", @@ -450,9 +449,8 @@ func TestMemPS(t *testing.T) { }, }}, }, { - desc: "proppatch remove unknown dead property", - buildfs: []string{"mkdir /dir"}, - mutability: ReadWrite, + desc: "proppatch remove unknown dead property", + buildfs: []string{"mkdir /dir"}, propOp: []propOp{{ op: "proppatch", name: "/dir", @@ -490,8 +488,11 @@ func TestMemPS(t *testing.T) { if err != nil { t.Fatalf("%s: cannot create test filesystem: %v", tc.desc, err) } + if tc.noDeadProps { + fs = noDeadPropsFS{fs} + } ls := NewMemLS() - ps := NewMemPS(fs, ls, tc.mutability) + ps := NewMemPS(fs, ls) for _, op := range tc.propOp { desc := fmt.Sprintf("%s: %s %s", tc.desc, op.op, op.name) if err = calcProps(op.name, fs, op.wantPropstats); err != nil { @@ -551,36 +552,43 @@ func cmpXMLName(a, b xml.Name) bool { type byXMLName []xml.Name -func (b byXMLName) Len() int { - return len(b) -} -func (b byXMLName) Swap(i, j int) { - b[i], b[j] = b[j], b[i] -} -func (b byXMLName) Less(i, j int) bool { - return cmpXMLName(b[i], b[j]) -} +func (b byXMLName) Len() int { return len(b) } +func (b byXMLName) Swap(i, j int) { b[i], b[j] = b[j], b[i] } +func (b byXMLName) Less(i, j int) bool { return cmpXMLName(b[i], b[j]) } type byPropname []Property -func (b byPropname) Len() int { - return len(b) -} -func (b byPropname) Swap(i, j int) { - b[i], b[j] = b[j], b[i] -} -func (b byPropname) Less(i, j int) bool { - return cmpXMLName(b[i].XMLName, b[j].XMLName) -} +func (b byPropname) Len() int { return len(b) } +func (b byPropname) Swap(i, j int) { b[i], b[j] = b[j], b[i] } +func (b byPropname) Less(i, j int) bool { return cmpXMLName(b[i].XMLName, b[j].XMLName) } type byStatus []Propstat -func (b byStatus) Len() int { - return len(b) +func (b byStatus) Len() int { return len(b) } +func (b byStatus) Swap(i, j int) { b[i], b[j] = b[j], b[i] } +func (b byStatus) Less(i, j int) bool { return b[i].Status < b[j].Status } + +type noDeadPropsFS struct { + FileSystem } -func (b byStatus) Swap(i, j int) { - b[i], b[j] = b[j], b[i] + +func (fs noDeadPropsFS) OpenFile(name string, flag int, perm os.FileMode) (File, error) { + f, err := fs.FileSystem.OpenFile(name, flag, perm) + if err != nil { + return nil, err + } + return noDeadPropsFile{f}, nil } -func (b byStatus) Less(i, j int) bool { - return b[i].Status < b[j].Status + +// noDeadPropsFile wraps a File but strips any optional DeadPropsHolder methods +// provided by the underlying File implementation. +type noDeadPropsFile struct { + f File } + +func (f noDeadPropsFile) Close() error { return f.f.Close() } +func (f noDeadPropsFile) Read(p []byte) (int, error) { return f.f.Read(p) } +func (f noDeadPropsFile) Readdir(count int) ([]os.FileInfo, error) { return f.f.Readdir(count) } +func (f noDeadPropsFile) Seek(off int64, whence int) (int64, error) { return f.f.Seek(off, whence) } +func (f noDeadPropsFile) Stat() (os.FileInfo, error) { return f.f.Stat() } +func (f noDeadPropsFile) Write(p []byte) (int, error) { return f.f.Write(p) }