Issue 180530043: code review 180530043: x/net/webdav: add OPTIONS
|
Created:
9 years, 5 months ago by dave Modified:
9 years, 4 months ago Reviewers:
CC:
nigeltao, nmvc, golang-codereviews Visibility:
Public. |
Descriptionx/net/webdav: add OPTIONS
Add a basic OPTIONS handler.
Patch Set 1 #Patch Set 2 : diff -r 15d5d0fa7dd8b1ec3f9370cb3082025163881563 https://code.google.com/p/go.net #Patch Set 3 : diff -r 15d5d0fa7dd8b1ec3f9370cb3082025163881563 https://code.google.com/p/go.net #
Total comments: 6
Patch Set 4 : diff -r 15d5d0fa7dd8b1ec3f9370cb3082025163881563 https://code.google.com/p/go.net #MessagesTotal messages: 10
Hello nigeltao@golang.org, nmvc@google.com (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go.net
Sign in to reply to this message.
https://codereview.appspot.com/180530043/diff/40001/webdav/webdav.go File webdav/webdav.go (right): https://codereview.appspot.com/180530043/diff/40001/webdav/webdav.go#newcode253 webdav/webdav.go:253: func (h *Handler) handleOptions(w http.ResponseWriter, r *http.Request) (status int, err error) { So, Nigel/my work is based in-part on a prototype I wrote independently available here, https://github.com/google/go-webdav/blob/master/webdav.go#L254 Could you take a similar approach to OPTIONS there namely appropriately only serving the verbs relevant to a path depending if an item exists? e.g. GET is invalid on unknown paths. https://codereview.appspot.com/180530043/diff/40001/webdav/webdav.go#newcode258 webdav/webdav.go:258: w.Header().Set("DAV", dav) Just inline the "dav" value, also add a comment to // http://www.webdav.org/specs/rfc4918.html#dav.compliance.classes https://codereview.appspot.com/180530043/diff/40001/webdav/webdav.go#newcode259 webdav/webdav.go:259: return http.StatusOK, nil Could you also do: w.Header().Set("MS-Author-Via", "DAV") I found Windows cares about this for some reason.
Sign in to reply to this message.
I'll abandon this, you've got this On Wed, Dec 3, 2014 at 2:44 PM, nmvc via golang-codereviews <golang-codereviews@googlegroups.com> wrote: > > https://codereview.appspot.com/180530043/diff/40001/webdav/webdav.go > File webdav/webdav.go (right): > > https://codereview.appspot.com/180530043/diff/40001/webdav/webdav.go#newcode253 > webdav/webdav.go:253: func (h *Handler) handleOptions(w > http.ResponseWriter, r *http.Request) (status int, err error) { > So, Nigel/my work is based in-part on a prototype I wrote independently > available here, > https://github.com/google/go-webdav/blob/master/webdav.go#L254 > > Could you take a similar approach to OPTIONS there namely appropriately > only serving the verbs relevant to a path depending if an item exists? > e.g. GET is invalid on unknown paths. > > https://codereview.appspot.com/180530043/diff/40001/webdav/webdav.go#newcode258 > webdav/webdav.go:258: w.Header().Set("DAV", dav) > Just inline the "dav" value, also add a comment to // > http://www.webdav.org/specs/rfc4918.html#dav.compliance.classes > > https://codereview.appspot.com/180530043/diff/40001/webdav/webdav.go#newcode259 > webdav/webdav.go:259: return http.StatusOK, nil > Could you also do: > w.Header().Set("MS-Author-Via", "DAV") > > I found Windows cares about this for some reason. > > https://codereview.appspot.com/180530043/ > > -- > You received this message because you are subscribed to the Google Groups > "golang-codereviews" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-codereviews+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.
Sign in to reply to this message.
Hello nigeltao@golang.org, nmvc@google.com (cc: golang-codereviews@googlegroups.com), Please take another look.
Sign in to reply to this message.
https://codereview.appspot.com/180530043/diff/40001/webdav/webdav.go File webdav/webdav.go (right): https://codereview.appspot.com/180530043/diff/40001/webdav/webdav.go#newcode253 webdav/webdav.go:253: func (h *Handler) handleOptions(w http.ResponseWriter, r *http.Request) (status int, err error) { On 2014/12/03 03:44:55, nmvc wrote: > So, Nigel/my work is based in-part on a prototype I wrote independently > available here, https://github.com/google/go-webdav/blob/master/webdav.go#L254 > > Could you take a similar approach to OPTIONS there namely appropriately only > serving the verbs relevant to a path depending if an item exists? e.g. GET is > invalid on unknown paths. > Done. https://codereview.appspot.com/180530043/diff/40001/webdav/webdav.go#newcode258 webdav/webdav.go:258: w.Header().Set("DAV", dav) On 2014/12/03 03:44:55, nmvc wrote: > Just inline the "dav" value, also add a comment to // > http://www.webdav.org/specs/rfc4918.html#dav.compliance.classes Done. https://codereview.appspot.com/180530043/diff/40001/webdav/webdav.go#newcode259 webdav/webdav.go:259: return http.StatusOK, nil On 2014/12/03 03:44:55, nmvc wrote: > Could you also do: > w.Header().Set("MS-Author-Via", "DAV") > > I found Windows cares about this for some reason. Yes, it does :(
Sign in to reply to this message.
On Wed, Dec 3, 2014 at 2:47 PM, Dave Cheney <dave@cheney.net> wrote: > I'll abandon this, you've got this I'm confused. Is this abandoned or should it be reviewed?
Sign in to reply to this message.
Sorry. I changed my mind. Please review this, I think its what nick asked for. On 5 Dec 2014 14:41, "Nigel Tao" <nigeltao@golang.org> wrote: > On Wed, Dec 3, 2014 at 2:47 PM, Dave Cheney <dave@cheney.net> wrote: > > I'll abandon this, you've got this > > I'm confused. Is this abandoned or should it be reviewed? >
Sign in to reply to this message.
On Fri, Dec 5, 2014 at 2:49 PM, Dave Cheney <dave@cheney.net> wrote: > Please review this, I think its what nick asked for. Wait, I just reviewed https://codereview.appspot.com/178930043/ which I think supersedes this one.
Sign in to reply to this message.
Cool. I'm not attached to this cl in particular, I'm just trying to write a simple WebDAV server using this package and needed some things to make cadaver(1) happy. On 5 Dec 2014 14:58, "Nigel Tao" <nigeltao@golang.org> wrote: > On Fri, Dec 5, 2014 at 2:49 PM, Dave Cheney <dave@cheney.net> wrote: > > Please review this, I think its what nick asked for. > > Wait, I just reviewed https://codereview.appspot.com/178930043/ which > I think supersedes this one. >
Sign in to reply to this message.
R=close To the author of this CL: The Go project has moved to Gerrit Code Review. If this CL should be continued, please see the latest version of https://golang.org/doc/contribute.html for instructions on how to set up Git and the Go project's Gerrit codereview plugin, and then create a new change with your current code. If there has been discussion on this CL, please give a link to it (golang.org/cl/180530043 is best) in the description in your new CL. Thanks very much.
Sign in to reply to this message.
|
