From 9d3c6bf52e83a9bbe7a89395b912fb2dedbd3372 Mon Sep 17 00:00:00 2001 From: Rhea Ghosh Date: Mon, 16 Oct 2023 14:35:11 -0500 Subject: [PATCH] ipn/ipnlocal/peerapi: refactoring taildrop to just one endpoint (#9832) Updates #14772 Signed-off-by: Rhea Ghosh --- ipn/ipnlocal/peerapi.go | 159 ++++++++++++++--------------------- ipn/ipnlocal/peerapi_test.go | 4 +- ipn/localapi/localapi.go | 2 +- 3 files changed, 68 insertions(+), 97 deletions(-) diff --git a/ipn/ipnlocal/peerapi.go b/ipn/ipnlocal/peerapi.go index 994d34b23..73b82d57d 100644 --- a/ipn/ipnlocal/peerapi.go +++ b/ipn/ipnlocal/peerapi.go @@ -305,12 +305,10 @@ func (h *peerAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.Header().Set("X-Frame-Options", "DENY") w.Header().Set("X-Content-Type-Options", "nosniff") } - if strings.HasPrefix(r.URL.Path, "/v0/partial-files/") { - h.handlePartialFileGet(w, r) - return - } if strings.HasPrefix(r.URL.Path, "/v0/put/") { - metricPutCalls.Add(1) + if r.Method == "PUT" { + metricPutCalls.Add(1) + } h.handlePeerPut(w, r) return } @@ -631,112 +629,85 @@ func (h *peerAPIHandler) peerHasCap(wantCap tailcfg.PeerCapability) bool { return h.ps.b.PeerCaps(h.remoteAddr.Addr()).HasCapability(wantCap) } -var errMisconfiguredInternals = errors.New("misconfigured internals") - -func (h *peerAPIHandler) extractBaseName(rawPath, prefix string) (ret string, err error) { - prefix, ok := strings.CutPrefix(rawPath, prefix) - if !ok { - return "", errMisconfiguredInternals - } - if prefix == "" { - return "", taildrop.ErrInvalidFileName - } - if strings.Contains(prefix, "/") { - return "", taildrop.ErrInvalidFileName - } - baseName, err := url.PathUnescape(prefix) - if err == errMisconfiguredInternals { - return "", errMisconfiguredInternals - } else if err != nil { - return "", taildrop.ErrInvalidFileName - } - return baseName, nil -} - -func (h *peerAPIHandler) handlePartialFileGet(w http.ResponseWriter, r *http.Request) { - if !h.ps.b.hasCapFileSharing() { - http.Error(w, taildrop.ErrNoTaildrop.Error(), http.StatusForbidden) - return - } - if r.Method != "GET" { - http.Error(w, "expected method GET", http.StatusMethodNotAllowed) - return - } - var resp any - var err error - id := taildrop.ClientID(h.peerNode.StableID()) - - if r.URL.Path == "/v0/partial-files/" { - resp, err = h.ps.taildrop.PartialFiles(id) - } else { - baseName, _ := h.extractBaseName(r.URL.EscapedPath(), "/v0/partial-files/") - ranges, ok := httphdr.ParseRange(r.Header.Get("Range")) - if !ok || len(ranges) != 1 || ranges[0].Length < 0 { - http.Error(w, "invalid Range header", http.StatusBadRequest) - return - } - offset := ranges[0].Start - length := ranges[0].Length - if length == 0 { - length = -1 // httphdr.Range.Length == 0 implies reading the rest of file - } - resp, err = h.ps.taildrop.HashPartialFile(id, baseName, offset, length) - } - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - if err := json.NewEncoder(w).Encode(resp); err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - -} - func (h *peerAPIHandler) handlePeerPut(w http.ResponseWriter, r *http.Request) { if !h.canPutFile() { http.Error(w, taildrop.ErrNoTaildrop.Error(), http.StatusForbidden) return } if !h.ps.b.hasCapFileSharing() { - http.Error(w, "file sharing not enabled by Tailscale admin", http.StatusForbidden) + http.Error(w, taildrop.ErrNoTaildrop.Error(), http.StatusForbidden) return } - if r.Method != "PUT" { - http.Error(w, "expected method PUT", http.StatusMethodNotAllowed) + rawPath := r.URL.EscapedPath() + prefix, ok := strings.CutPrefix(rawPath, "/v0/put/") + if !ok { + http.Error(w, "misconfigured internals", http.StatusForbidden) return } - baseName, err := h.extractBaseName(r.URL.EscapedPath(), "/v0/put/") + baseName, err := url.PathUnescape(prefix) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + http.Error(w, taildrop.ErrInvalidFileName.Error(), http.StatusBadRequest) return } - t0 := h.ps.b.clock.Now() - id := taildrop.ClientID(h.peerNode.StableID()) + switch r.Method { + case "GET": + var resp any + var err error + id := taildrop.ClientID(h.peerNode.StableID()) - var offset int64 - if rangeHdr := r.Header.Get("Range"); rangeHdr != "" { - ranges, ok := httphdr.ParseRange(rangeHdr) - if !ok || len(ranges) != 1 || ranges[0].Length != 0 { - http.Error(w, "invalid Range header", http.StatusBadRequest) + if r.URL.Path == "/v0/put/"+baseName { + resp, err = h.ps.taildrop.PartialFiles(id) + } else { + ranges, ok := httphdr.ParseRange(r.Header.Get("Range")) + if !ok || len(ranges) != 1 || ranges[0].Length < 0 { + http.Error(w, "invalid Range header", http.StatusBadRequest) + return + } + offset := ranges[0].Start + length := ranges[0].Length + if length == 0 { + length = -1 // httphdr.Range.Length == 0 implies reading the rest of file + } + resp, err = h.ps.taildrop.HashPartialFile(id, baseName, offset, length) + } + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + if err := json.NewEncoder(w).Encode(resp); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) return } - offset = ranges[0].Start - } - n, err := h.ps.taildrop.PutFile(taildrop.ClientID(fmt.Sprint(id)), baseName, r.Body, offset, r.ContentLength) - switch err { - case nil: - d := h.ps.b.clock.Since(t0).Round(time.Second / 10) - h.logf("got put of %s in %v from %v/%v", approxSize(n), d, h.remoteAddr.Addr(), h.peerNode.ComputedName) - io.WriteString(w, "{}\n") - case taildrop.ErrNoTaildrop: - http.Error(w, err.Error(), http.StatusForbidden) - case taildrop.ErrInvalidFileName: - http.Error(w, err.Error(), http.StatusBadRequest) - case taildrop.ErrFileExists: - http.Error(w, err.Error(), http.StatusConflict) + case "PUT": + t0 := h.ps.b.clock.Now() + id := taildrop.ClientID(h.peerNode.StableID()) + + var offset int64 + if rangeHdr := r.Header.Get("Range"); rangeHdr != "" { + ranges, ok := httphdr.ParseRange(rangeHdr) + if !ok || len(ranges) != 1 || ranges[0].Length != 0 { + http.Error(w, "invalid Range header", http.StatusBadRequest) + return + } + offset = ranges[0].Start + } + n, err := h.ps.taildrop.PutFile(taildrop.ClientID(fmt.Sprint(id)), baseName, r.Body, offset, r.ContentLength) + switch err { + case nil: + d := h.ps.b.clock.Since(t0).Round(time.Second / 10) + h.logf("got put of %s in %v from %v/%v", approxSize(n), d, h.remoteAddr.Addr(), h.peerNode.ComputedName) + io.WriteString(w, "{}\n") + case taildrop.ErrNoTaildrop: + http.Error(w, err.Error(), http.StatusForbidden) + case taildrop.ErrInvalidFileName: + http.Error(w, err.Error(), http.StatusBadRequest) + case taildrop.ErrFileExists: + http.Error(w, err.Error(), http.StatusConflict) + default: + http.Error(w, err.Error(), http.StatusInternalServerError) + } default: - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, "expected method GET or PUT", http.StatusMethodNotAllowed) } } diff --git a/ipn/ipnlocal/peerapi_test.go b/ipn/ipnlocal/peerapi_test.go index f412c8cd4..f1f6b2265 100644 --- a/ipn/ipnlocal/peerapi_test.go +++ b/ipn/ipnlocal/peerapi_test.go @@ -183,7 +183,7 @@ func TestHandlePeerAPI(t *testing.T) { reqs: []*http.Request{httptest.NewRequest("PUT", "/v0/put/foo", nil)}, checks: checks( httpStatus(http.StatusForbidden), - bodyContains("file sharing not enabled by Tailscale admin"), + bodyContains("Taildrop disabled"), ), }, { @@ -204,7 +204,7 @@ func TestHandlePeerAPI(t *testing.T) { reqs: []*http.Request{httptest.NewRequest("POST", "/v0/put/foo", nil)}, checks: checks( httpStatus(405), - bodyContains("expected method PUT"), + bodyContains("expected method GET or PUT"), ), }, { diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index 1b4df8488..832a4e871 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -1325,7 +1325,7 @@ func (h *Handler) serveFilePut(w http.ResponseWriter, r *http.Request) { Transport: h.b.Dialer().PeerAPITransport(), Timeout: 10 * time.Second, } - req, err := http.NewRequestWithContext(r.Context(), "GET", "http://peer/v0/partial-files/"+filenameEscaped, nil) + req, err := http.NewRequestWithContext(r.Context(), "GET", "http://peer/v0/put/"+filenameEscaped, nil) if err != nil { return taildrop.FileChecksums{}, err }