From 9d29dbbe5dd8080ba56ea6b1a365a406d2583407 Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Sun, 25 Apr 2021 21:48:17 +0200 Subject: [PATCH 1/3] cmd/syncthing: Don't fail early on api setup error (fixes 7558) (#7591) * cmd/syncthing: Don't fail early on api setup error (fixes 7558) * switch to factory pattern * refactor config command to show help on nothing * wip * wip * already abort in before --- cmd/syncthing/cli/client.go | 77 +++++++++++++++++++++++---- cmd/syncthing/cli/config.go | 87 ++++++++++++++++++++++++++++++ cmd/syncthing/cli/errors.go | 2 +- cmd/syncthing/cli/main.go | 94 ++++----------------------------- cmd/syncthing/cli/operations.go | 5 +- cmd/syncthing/cli/utils.go | 23 ++++++-- 6 files changed, 185 insertions(+), 103 deletions(-) create mode 100644 cmd/syncthing/cli/config.go diff --git a/cmd/syncthing/cli/client.go b/cmd/syncthing/cli/client.go index 393b93215..9bf0ab993 100644 --- a/cmd/syncthing/cli/client.go +++ b/cmd/syncthing/cli/client.go @@ -17,33 +17,88 @@ import ( "strings" "github.com/syncthing/syncthing/lib/config" + "github.com/syncthing/syncthing/lib/events" + "github.com/syncthing/syncthing/lib/locations" + "github.com/syncthing/syncthing/lib/protocol" ) -type APIClient struct { +type APIClient interface { + Get(url string) (*http.Response, error) + Post(url, body string) (*http.Response, error) +} + +type apiClient struct { http.Client cfg config.GUIConfiguration apikey string } -func getClient(cfg config.GUIConfiguration) *APIClient { +type apiClientFactory struct { + cfg config.GUIConfiguration +} + +func (f *apiClientFactory) getClient() (APIClient, error) { + // Now if the API key and address is not provided (we are not connecting to a remote instance), + // try to rip it out of the config. + if f.cfg.RawAddress == "" && f.cfg.APIKey == "" { + var err error + f.cfg, err = loadGUIConfig() + if err != nil { + return nil, err + } + } else if f.cfg.Address() == "" || f.cfg.APIKey == "" { + return nil, errors.New("Both --gui-address and --gui-apikey should be specified") + } + httpClient := http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{ InsecureSkipVerify: true, }, DialContext: func(_ context.Context, _, _ string) (net.Conn, error) { - return net.Dial(cfg.Network(), cfg.Address()) + return net.Dial(f.cfg.Network(), f.cfg.Address()) }, }, } - return &APIClient{ + return &apiClient{ Client: httpClient, - cfg: cfg, - apikey: cfg.APIKey, - } + cfg: f.cfg, + apikey: f.cfg.APIKey, + }, nil } -func (c *APIClient) Endpoint() string { +func loadGUIConfig() (config.GUIConfiguration, error) { + // Load the certs and get the ID + cert, err := tls.LoadX509KeyPair( + locations.Get(locations.CertFile), + locations.Get(locations.KeyFile), + ) + if err != nil { + return config.GUIConfiguration{}, fmt.Errorf("reading device ID: %w", err) + } + + myID := protocol.NewDeviceID(cert.Certificate[0]) + + // Load the config + cfg, _, err := config.Load(locations.Get(locations.ConfigFile), myID, events.NoopLogger) + if err != nil { + return config.GUIConfiguration{}, fmt.Errorf("loading config: %w", err) + } + + guiCfg := cfg.GUI() + + if guiCfg.Address() == "" { + return config.GUIConfiguration{}, errors.New("Could not find GUI Address") + } + + if guiCfg.APIKey == "" { + return config.GUIConfiguration{}, errors.New("Could not find GUI API key") + } + + return guiCfg, nil +} + +func (c *apiClient) Endpoint() string { if c.cfg.Network() == "unix" { return "http://unix/" } @@ -54,7 +109,7 @@ func (c *APIClient) Endpoint() string { return url } -func (c *APIClient) Do(req *http.Request) (*http.Response, error) { +func (c *apiClient) Do(req *http.Request) (*http.Response, error) { req.Header.Set("X-API-Key", c.apikey) resp, err := c.Client.Do(req) if err != nil { @@ -63,7 +118,7 @@ func (c *APIClient) Do(req *http.Request) (*http.Response, error) { return resp, checkResponse(resp) } -func (c *APIClient) Get(url string) (*http.Response, error) { +func (c *apiClient) Get(url string) (*http.Response, error) { request, err := http.NewRequest("GET", c.Endpoint()+"rest/"+url, nil) if err != nil { return nil, err @@ -71,7 +126,7 @@ func (c *APIClient) Get(url string) (*http.Response, error) { return c.Do(request) } -func (c *APIClient) Post(url, body string) (*http.Response, error) { +func (c *apiClient) Post(url, body string) (*http.Response, error) { request, err := http.NewRequest("POST", c.Endpoint()+"rest/"+url, bytes.NewBufferString(body)) if err != nil { return nil, err diff --git a/cmd/syncthing/cli/config.go b/cmd/syncthing/cli/config.go new file mode 100644 index 000000000..36150d539 --- /dev/null +++ b/cmd/syncthing/cli/config.go @@ -0,0 +1,87 @@ +// Copyright (C) 2021 The Syncthing Authors. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at https://mozilla.org/MPL/2.0/. + +package cli + +import ( + "encoding/json" + "fmt" + "reflect" + + "github.com/AudriusButkevicius/recli" + "github.com/pkg/errors" + "github.com/syncthing/syncthing/lib/config" + "github.com/urfave/cli" +) + +type configHandler struct { + original, cfg config.Configuration + client APIClient + err error +} + +func getConfigCommand(f *apiClientFactory) (cli.Command, error) { + h := new(configHandler) + h.client, h.err = f.getClient() + if h.err == nil { + h.cfg, h.err = getConfig(h.client) + } + h.original = h.cfg.Copy() + + // Copy the config and set the default flags + recliCfg := recli.DefaultConfig + recliCfg.IDTag.Name = "xml" + recliCfg.SkipTag.Name = "json" + + commands, err := recli.New(recliCfg).Construct(&h.cfg) + if err != nil { + return cli.Command{}, fmt.Errorf("config reflect: %w", err) + } + + return cli.Command{ + Name: "config", + HideHelp: true, + Usage: "Configuration modification command group", + Subcommands: commands, + Before: h.configBefore, + After: h.configAfter, + }, nil +} + +func (h *configHandler) configBefore(c *cli.Context) error { + for _, arg := range c.Args() { + if arg == "--help" || arg == "-h" { + return nil + } + } + return h.err +} + +func (h *configHandler) configAfter(c *cli.Context) error { + if h.err != nil { + // Error was already returned in configBefore + return nil + } + if reflect.DeepEqual(h.cfg, h.original) { + return nil + } + body, err := json.MarshalIndent(h.cfg, "", " ") + if err != nil { + return err + } + resp, err := h.client.Post("system/config", string(body)) + if err != nil { + return err + } + if resp.StatusCode != 200 { + body, err := responseToBArray(resp) + if err != nil { + return err + } + return errors.New(string(body)) + } + return nil +} diff --git a/cmd/syncthing/cli/errors.go b/cmd/syncthing/cli/errors.go index 6daa5e970..8847ce962 100644 --- a/cmd/syncthing/cli/errors.go +++ b/cmd/syncthing/cli/errors.go @@ -39,7 +39,7 @@ var errorsCommand = cli.Command{ } func errorsPush(c *cli.Context) error { - client := c.App.Metadata["client"].(*APIClient) + client := c.App.Metadata["client"].(APIClient) errStr := strings.Join(c.Args(), " ") response, err := client.Post("system/error", strings.TrimSpace(errStr)) if err != nil { diff --git a/cmd/syncthing/cli/main.go b/cmd/syncthing/cli/main.go index 3bf208cb7..ab4a8a7a3 100644 --- a/cmd/syncthing/cli/main.go +++ b/cmd/syncthing/cli/main.go @@ -8,14 +8,10 @@ package cli import ( "bufio" - "crypto/tls" - "encoding/json" "io/ioutil" "os" - "reflect" "strings" - "github.com/AudriusButkevicius/recli" "github.com/alecthomas/kong" "github.com/flynn-archive/go-shlex" "github.com/mattn/go-isatty" @@ -24,9 +20,6 @@ import ( "github.com/syncthing/syncthing/cmd/syncthing/cmdutil" "github.com/syncthing/syncthing/lib/config" - "github.com/syncthing/syncthing/lib/events" - "github.com/syncthing/syncthing/lib/locations" - "github.com/syncthing/syncthing/lib/protocol" ) type preCli struct { @@ -51,68 +44,16 @@ func Run() error { if err != nil { return errors.Wrap(err, "Command line options:") } - guiCfg := config.GUIConfiguration{ - RawAddress: c.GUIAddress, - APIKey: c.GUIAPIKey, + clientFactory := &apiClientFactory{ + cfg: config.GUIConfiguration{ + RawAddress: c.GUIAddress, + APIKey: c.GUIAPIKey, + }, } - // Now if the API key and address is not provided (we are not connecting to a remote instance), - // try to rip it out of the config. - if guiCfg.RawAddress == "" && guiCfg.APIKey == "" { - // Load the certs and get the ID - cert, err := tls.LoadX509KeyPair( - locations.Get(locations.CertFile), - locations.Get(locations.KeyFile), - ) - if err != nil { - return errors.Wrap(err, "reading device ID") - } - - myID := protocol.NewDeviceID(cert.Certificate[0]) - - // Load the config - cfg, _, err := config.Load(locations.Get(locations.ConfigFile), myID, events.NoopLogger) - if err != nil { - return errors.Wrap(err, "loading config") - } - - guiCfg = cfg.GUI() - } else if guiCfg.Address() == "" || guiCfg.APIKey == "" { - return errors.New("Both --gui-address and --gui-apikey should be specified") - } - - if guiCfg.Address() == "" { - return errors.New("Could not find GUI Address") - } - - if guiCfg.APIKey == "" { - return errors.New("Could not find GUI API key") - } - - client := getClient(guiCfg) - - cfg, cfgErr := getConfig(client) - original := cfg.Copy() - - // Copy the config and set the default flags - recliCfg := recli.DefaultConfig - recliCfg.IDTag.Name = "xml" - recliCfg.SkipTag.Name = "json" - - configCommand := cli.Command{ - Name: "config", - HideHelp: true, - Usage: "Configuration modification command group", - } - if cfgErr != nil { - configCommand.Action = func(*cli.Context) error { - return cfgErr - } - } else { - configCommand.Subcommands, err = recli.New(recliCfg).Construct(&cfg) - if err != nil { - return errors.Wrap(err, "config reflect") - } + configCommand, err := getConfigCommand(clientFactory) + if err != nil { + return err } // Implement the same flags at the upper CLI, but do nothing with them. @@ -144,7 +85,7 @@ func Run() error { app := cli.NewApp() app.Author = "The Syncthing Authors" app.Metadata = map[string]interface{}{ - "client": client, + "clientFactory": clientFactory, } app.Commands = []cli.Command{{ Name: "cli", @@ -187,23 +128,6 @@ func Run() error { } } - if cfgErr == nil && !reflect.DeepEqual(cfg, original) { - body, err := json.MarshalIndent(cfg, "", " ") - if err != nil { - return err - } - resp, err := client.Post("system/config", string(body)) - if err != nil { - return err - } - if resp.StatusCode != 200 { - body, err := responseToBArray(resp) - if err != nil { - return err - } - return errors.New(string(body)) - } - } return nil } diff --git a/cmd/syncthing/cli/operations.go b/cmd/syncthing/cli/operations.go index 5a8a062ce..347a99005 100644 --- a/cmd/syncthing/cli/operations.go +++ b/cmd/syncthing/cli/operations.go @@ -42,7 +42,10 @@ var operationCommand = cli.Command{ } func foldersOverride(c *cli.Context) error { - client := c.App.Metadata["client"].(*APIClient) + client, err := getClientFactory(c).getClient() + if err != nil { + return err + } cfg, err := getConfig(client) if err != nil { return err diff --git a/cmd/syncthing/cli/utils.go b/cmd/syncthing/cli/utils.go index 1f230fcb5..503adfb3b 100644 --- a/cmd/syncthing/cli/utils.go +++ b/cmd/syncthing/cli/utils.go @@ -32,15 +32,21 @@ func responseToBArray(response *http.Response) ([]byte, error) { func emptyPost(url string) cli.ActionFunc { return func(c *cli.Context) error { - client := c.App.Metadata["client"].(*APIClient) - _, err := client.Post(url, "") + client, err := getClientFactory(c).getClient() + if err != nil { + return err + } + _, err = client.Post(url, "") return err } } func indexDumpOutput(url string) cli.ActionFunc { return func(c *cli.Context) error { - client := c.App.Metadata["client"].(*APIClient) + client, err := getClientFactory(c).getClient() + if err != nil { + return err + } response, err := client.Get(url) if err != nil { return err @@ -51,7 +57,10 @@ func indexDumpOutput(url string) cli.ActionFunc { func saveToFile(url string) cli.ActionFunc { return func(c *cli.Context) error { - client := c.App.Metadata["client"].(*APIClient) + client, err := getClientFactory(c).getClient() + if err != nil { + return err + } response, err := client.Get(url) if err != nil { return err @@ -82,7 +91,7 @@ func saveToFile(url string) cli.ActionFunc { } } -func getConfig(c *APIClient) (config.Configuration, error) { +func getConfig(c APIClient) (config.Configuration, error) { cfg := config.Configuration{} response, err := c.Get("system/config") if err != nil { @@ -147,3 +156,7 @@ func nulString(bs []byte) string { func normalizePath(path string) string { return filepath.ToSlash(filepath.Clean(path)) } + +func getClientFactory(c *cli.Context) *apiClientFactory { + return c.App.Metadata["clientFactory"].(*apiClientFactory) +} From 60e8630413cf1609147d83397fbf65b2fe2d9042 Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Mon, 26 Apr 2021 15:35:12 +0200 Subject: [PATCH 2/3] gui: Handle empty path in validation (ref #7379) (#7595) --- gui/default/syncthing/core/pathIsSubDirDirective.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gui/default/syncthing/core/pathIsSubDirDirective.js b/gui/default/syncthing/core/pathIsSubDirDirective.js index 002b1fd20..38674d91f 100644 --- a/gui/default/syncthing/core/pathIsSubDirDirective.js +++ b/gui/default/syncthing/core/pathIsSubDirDirective.js @@ -24,6 +24,9 @@ angular.module('syncthing.core') scope.folderPathErrors.isParent = false; scope.folderPathErrors.otherID = ""; scope.folderPathErrors.otherLabel = ""; + if (!viewValue) { + return true; + } for (var folderID in scope.folders) { if (folderID === scope.currentFolder.id) { continue; From ec86db176edc23ad2375c5d53c9f12967836b42b Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Mon, 26 Apr 2021 15:36:51 +0200 Subject: [PATCH 3/3] lib/model: Handle invalid needed items on send-only (ref #7476) (#7596) --- lib/model/folder_sendonly.go | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/lib/model/folder_sendonly.go b/lib/model/folder_sendonly.go index 861c9440c..752101caa 100644 --- a/lib/model/folder_sendonly.go +++ b/lib/model/folder_sendonly.go @@ -37,8 +37,10 @@ func (f *sendOnlyFolder) PullErrors() []FileError { // pull checks need for files that only differ by metadata (no changes on disk) func (f *sendOnlyFolder) pull() (bool, error) { - batch := make([]protocol.FileInfo, 0, maxBatchSizeFiles) - batchSizeBytes := 0 + batch := newFileInfoBatch(func(files []protocol.FileInfo) error { + f.updateLocalsFromPulling(files) + return nil + }) snap, err := f.dbSnapshot() if err != nil { @@ -46,45 +48,40 @@ func (f *sendOnlyFolder) pull() (bool, error) { } defer snap.Release() snap.WithNeed(protocol.LocalDeviceID, func(intf protocol.FileIntf) bool { - if len(batch) == maxBatchSizeFiles || batchSizeBytes > maxBatchSizeBytes { - f.updateLocalsFromPulling(batch) - batch = batch[:0] - batchSizeBytes = 0 - } + batch.flushIfFull() + + file := intf.(protocol.FileInfo) if f.ignores.ShouldIgnore(intf.FileName()) { - file := intf.(protocol.FileInfo) file.SetIgnored() - batch = append(batch, file) - batchSizeBytes += file.ProtoSize() + batch.append(file) l.Debugln(f, "Handling ignored file", file) return true } curFile, ok := snap.Get(protocol.LocalDeviceID, intf.FileName()) if !ok { - if intf.IsDeleted() { + if intf.IsInvalid() { + // Global invalid file just exists for need accounting + batch.append(file) + } else if intf.IsDeleted() { l.Debugln("Should never get a deleted file as needed when we don't have it") f.evLogger.Log(events.Failure, "got deleted file that doesn't exist locally as needed when pulling on send-only") } return true } - file := intf.(protocol.FileInfo) if !file.IsEquivalentOptional(curFile, f.modTimeWindow, f.IgnorePerms, false, 0) { return true } - batch = append(batch, file) - batchSizeBytes += file.ProtoSize() + batch.append(file) l.Debugln(f, "Merging versions of identical file", file) return true }) - if len(batch) > 0 { - f.updateLocalsFromPulling(batch) - } + batch.flush() return true, nil }