Repository: rclone/rclone
Pull request #7931 (2024-07-01 18:34:29)

fs walk: adding backward compat updates to bwlimit parsing - fixes issue #7595

#### What is the purpose of this change?

The problem is the conflict between rclone's bwlimit timetable syntax, which uses spaces as separators, and Docker's formatting. This leads to frequent parsing issues when configuring rclone within Docker environments.

#### Was the change discussed in an issue or in the forum before?


#7595 


#### Checklist

- [x] I have read the [contribution guidelines](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#submitting-a-new-feature-or-bug-fix).
- [x] I have added tests for all changes in this PR if appropriate.
- [x] I have added documentation for the changes if appropriate.
- [x] All commit messages are in [house style](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#commit-messages).
- [x] I'm done, this Pull Request is ready for review :-)


KDreynolds: Its possible I misunderstood the request in issue #7595, ill amend the commit with test updates and doc updates, thanks!
ncw: Sorry I missed the link to the original commit which is perfectly clear :-)

Please carry on and add some docs / tests and I will merge - thank you :-)
KDreynolds: Ok I have made those additions. Please let me know if anything needs to be addressed @ncw. Thanks!
KDreynolds: Updates made, I think should be good to go now. Thanks again!
KDreynolds: Good now I believe.

Patch:
diff --git a/backend/pikpak/helper.go b/backend/pikpak/helper.go
index 24230312b..b296900a4 100644
--- a/backend/pikpak/helper.go
+++ b/backend/pikpak/helper.go
@@ -8,13 +8,16 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"math/rand"
 	"net/http"
 	"net/url"
 	"os"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/rclone/rclone/backend/pikpak/api"
+	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/lib/rest"
 )
 
@@ -253,6 +256,37 @@ func (f *Fs) requestShare(ctx context.Context, req *api.RequestShare) (info *api
 	return
 }
 
+// getGcid retrieves Gcid cached in API server
+func (f *Fs) getGcid(ctx context.Context, src fs.ObjectInfo) (gcid string, err error) {
+	cid, err := calcCid(ctx, src)
+	if err != nil {
+		return
+	}
+
+	params := url.Values{}
+	params.Set("cid", cid)
+	params.Set("file_size", strconv.FormatInt(src.Size(), 10))
+	opts := rest.Opts{
+		Method:       "GET",
+		Path:         "/drive/v1/resource/cid",
+		Parameters:   params,
+		ExtraHeaders: map[string]string{"x-device-id": f.deviceID},
+	}
+
+	info := struct {
+		Gcid string `json:"gcid,omitempty"`
+	}{}
+	var resp *http.Response
+	err = f.pacer.Call(func() (bool, error) {
+		resp, err = f.rst.CallJSON(ctx, &opts, nil, &info)
+		return f.shouldRetry(ctx, resp, err)
+	})
+	if err != nil {
+		return "", err
+	}
+	return info.Gcid, nil
+}
+
 // Read the gcid of in returning a reader which will read the same contents
 //
 // The cleanup function should be called when out is finished with
@@ -306,6 +340,9 @@ func readGcid(in io.Reader, size, threshold int64) (gcid string, out io.Reader,
 	return
 }
 
+// calcGcid calculates Gcid from reader
+//
+// Gcid is a custom hash to index a file contents
 func calcGcid(r io.Reader, size int64) (string, error) {
 	calcBlockSize := func(j int64) int64 {
 		var psize int64 = 0x40000
@@ -330,3 +367,64 @@ func calcGcid(r io.Reader, size int64) (string, error) {
 	}
 	return hex.EncodeToString(totalHash.Sum(nil)), nil
 }
+
+// calcCid calculates Cid from source
+//
+// Cid is a simplified version of Gcid
+func calcCid(ctx context.Context, src fs.ObjectInfo) (cid string, err error) {
+	srcObj := fs.UnWrapObjectInfo(src)
+	if srcObj == nil {
+		return "", fmt.Errorf("failed to unwrap object from src: %s", src)
+	}
+
+	size := src.Size()
+	hash := sha1.New()
+	var rc io.ReadCloser
+
+	readHash := func(start, length int64) (err error) {
+		end := start + length - 1
+		if rc, err = srcObj.Open(ctx, &fs.RangeOption{Start: start, End: end}); err != nil {
+			return fmt.Errorf("failed to open src with range (%d, %d): %w", start, end, err)
+		}
+		defer fs.CheckClose(rc, &err)
+		_, err = io.Copy(hash, rc)
+		return err
+	}
+
+	if size <= 0xF000 { // 61440 = 60KB
+		err = readHash(0, size)
+	} else { // 20KB from three different parts
+		for _, start := range []int64{0, size / 3, size - 0x5000} {
+			err = readHash(start, 0x5000)
+			if err != nil {
+				break
+			}
+		}
+	}
+	if err != nil {
+		return "", fmt.Errorf("failed to hash: %w", err)
+	}
+	cid = strings.ToUpper(hex.EncodeToString(hash.Sum(nil)))
+	return
+}
+
+// randomly generates device id used for request header 'x-device-id'
+//
+// original javascript implementation
+//
+//	return "xxxxxxxxxxxx4xxxyxxxxxxxxxxxxxxx".replace(/[xy]/g, (e) => {
+//	    const t = (16 * Math.random()) | 0;
+//	    return ("x" == e ? t : (3 & t) | 8).toString(16);
+//	});
+func genDeviceID() string {
+	base := []byte("xxxxxxxxxxxx4xxxyxxxxxxxxxxxxxxx")
+	for i, char := range base {
+		switch char {
+		case 'x':
+			base[i] = fmt.Sprintf("%x", rand.Intn(16))[0]
+		case 'y':
+			base[i] = fmt.Sprintf("%x", rand.Intn(16)&3|8)[0]
+		}
+	}
+	return string(base)
+}
diff --git a/backend/pikpak/pikpak.go b/backend/pikpak/pikpak.go
index 81645c13c..3d666867f 100644
--- a/backend/pikpak/pikpak.go
+++ b/backend/pikpak/pikpak.go
@@ -274,6 +274,7 @@ type Fs struct {
 	dirCache     *dircache.DirCache // Map of directory path to directory id
 	pacer        *fs.Pacer          // pacer for API calls
 	rootFolderID string             // the id of the root folder
+	deviceID     string             // device id used for api requests
 	client       *http.Client       // authorized client
 	m            configmap.Mapper
 	tokenMu      *sync.Mutex // when renewing tokens
@@ -489,6 +490,7 @@ func newFs(ctx context.Context, name, path string, m configmap.Mapper) (*Fs, err
 		CanHaveEmptyDirectories: true, // can have empty directories
 		NoMultiThreading:        true, // can't have multiple threads downloading
 	}).Fill(ctx, f)
+	f.deviceID = genDeviceID()
 
 	if err := f.newClientWithPacer(ctx); err != nil {
 		return nil, err
@@ -1694,32 +1696,36 @@ func (o *Object) upload(ctx context.Context, in io.Reader, src fs.ObjectInfo, wi
 	}
 
 	// Calculate gcid; grabbed from package jottacloud
-	var gcid string
-	if srcObj := fs.UnWrapObjectInfo(src); srcObj != nil && srcObj.Fs().Features().IsLocal {
-		// No buffering; directly calculate gcid from source
-		rc, err := srcObj.Open(ctx)
-		if err != nil {
-			return fmt.Errorf("failed to open src: %w", err)
-		}
-		defer fs.CheckClose(rc, &err)
+	gcid, err := o.fs.getGcid(ctx, src)
+	if err != nil || gcid == "" {
+		fs.Debugf(o, "calculating gcid: %v", err)
+		if srcObj := fs.UnWrapObjectInfo(src); srcObj != nil && srcObj.Fs().Features().IsLocal {
+			// No buffering; directly calculate gcid from source
+			rc, err := srcObj.Open(ctx)
+			if err != nil {
+				return fmt.Errorf("failed to open src: %w", err)
+			}
+			defer fs.CheckClose(rc, &err)
 
-		if gcid, err = calcGcid(rc, srcObj.Size()); err != nil {
-			return fmt.Errorf("failed to calculate gcid: %w", err)
-		}
-	} else {
-		// unwrap the accounting from the input, we use wrap to put it
-		// back on after the buffering
-		var wrap accounting.WrapFn
-		in, wrap = accounting.UnWrap(in)
-		var cleanup func()
-		gcid, in, cleanup, err = readGcid(in, size, int64(o.fs.opt.HashMemoryThreshold))
-		defer cleanup()
-		if err != nil {
-			return fmt.Errorf("failed to calculate gcid: %w", err)
+			if gcid, err = calcGcid(rc, srcObj.Size()); err != nil {
+				return fmt.Errorf("failed to calculate gcid: %w", err)
+			}
+		} else {
+			// unwrap the accounting from the input, we use wrap to put it
+			// back on after the buffering
+			var wrap accounting.WrapFn
+			in, wrap = accounting.UnWrap(in)
+			var cleanup func()
+			gcid, in, cleanup, err = readGcid(in, size, int64(o.fs.opt.HashMemoryThreshold))
+			defer cleanup()
+			if err != nil {
+				return fmt.Errorf("failed to calculate gcid: %w", err)
+			}
+			// Wrap the accounting back onto the stream
+			in = wrap(in)
 		}
-		// Wrap the accounting back onto the stream
-		in = wrap(in)
 	}
+	fs.Debugf(o, "gcid = %s", gcid)
 
 	if !withTemp {
 		info, err := o.fs.upload(ctx, in, leaf, dirID, gcid, size, options...)
diff --git a/docs/content/commands/rclone.md b/docs/content/commands/rclone.md
index 4cf7350d2..c73ff6f5e 100644
--- a/docs/content/commands/rclone.md
+++ b/docs/content/commands/rclone.md
@@ -123,7 +123,7 @@ rclone [flags]
       --box-token-url string                                Token server url
       --box-upload-cutoff SizeSuffix                        Cutoff for switching to multipart upload (>= 50 MiB) (default 50Mi)
       --buffer-size SizeSuffix                              In memory buffer size when reading files for each --transfer (default 16Mi)
-      --bwlimit BwTimetable                                 Bandwidth limit in KiB/s, or use suffix B|K|M|G|T|P or a full timetable
+      --bwlimit BwTimetable                                 Bandwidth limit in KiB/s, or use suffix B|K|M|G|T|P or a full timetable.
       --bwlimit-file BwTimetable                            Bandwidth limit per file in KiB/s, or use suffix B|K|M|G|T|P or a full timetable
       --ca-cert stringArray                                 CA certificate used to verify servers
       --cache-chunk-clean-interval Duration                 How often should the cache perform cleanups of the chunk storage (default 1m0s)
diff --git a/docs/content/docs.md b/docs/content/docs.md
index 49180b0ed..c4ef41783 100644
--- a/docs/content/docs.md
+++ b/docs/content/docs.md
@@ -724,12 +724,20 @@ for upload:download, e.g.`10M:1M`.
   characters. It is optional.
 - `HH:MM` is an hour from 00:00 to 23:59.
 
+Entries can be separated by spaces or semicolons.
+
+**Note:** Semicolons can be used as separators instead of spaces to avoid parsing issues in environments like Docker.
+
 An example of a typical timetable to avoid link saturation during daytime
 working hours could be:
 
+Using spaces as separators:
 `--bwlimit "08:00,512k 12:00,10M 13:00,512k 18:00,30M 23:00,off"`
 
-In this example, the transfer bandwidth will be set to 512 KiB/s
+Using semicolons as separators:
+`--bwlimit "08:00,512k;12:00,10M;13:00,512k;18:00,30M;23:00,off"`
+
+In these examples, the transfer bandwidth will be set to 512 KiB/s
 at 8am every day. At noon, it will rise to 10 MiB/s, and drop back
 to 512 KiB/sec at 1pm. At 6pm, the bandwidth limit will be set to
 30 MiB/s, and at 11pm it will be completely disabled (full speed).
@@ -737,8 +745,12 @@ Anything between 11pm and 8am will remain unlimited.
 
 An example of timetable with `WEEKDAY` could be:
 
+Using spaces as separators:
 `--bwlimit "Mon-00:00,512 Fri-23:59,10M Sat-10:00,1M Sun-20:00,off"`
 
+Using semicolons as separators:
+`--bwlimit "Mon-00:00,512;Fri-23:59,10M;Sat-10:00,1M;Sun-20:00,off"`
+
 It means that, the transfer bandwidth will be set to 512 KiB/s on
 Monday. It will rise to 10 MiB/s before the end of Friday. At 10:00
 on Saturday it will be set to 1 MiB/s. From 20:00 on Sunday it will
diff --git a/fs/bwtimetable.go b/fs/bwtimetable.go
index 3361fd61f..eb01a8d76 100644
--- a/fs/bwtimetable.go
+++ b/fs/bwtimetable.go
@@ -150,7 +150,10 @@ func (x *BwTimetable) Set(s string) error {
 		return nil
 	}
 
-	for _, tok := range strings.Split(s, " ") {
+	// Split the timetable string by both spaces and semicolons
+	for _, tok := range strings.FieldsFunc(s, func(r rune) bool {
+		return r == ' ' || r == ';'
+	}) {
 		tv := strings.Split(tok, ",")
 
 		// Format must be dayOfWeek-HH:MM,BW


Test patch:
diff --git a/fs/bwtimetable_test.go b/fs/bwtimetable_test.go
index 67f4057a3..b1d45f567 100644
--- a/fs/bwtimetable_test.go
+++ b/fs/bwtimetable_test.go
@@ -256,6 +256,53 @@ func TestBwTimetableSet(t *testing.T) {
 			false,
 			"Mon-00:00,512Ki Sun-12:00,1Mi Mon-12:00,1Mi Tue-12:00,1Mi Wed-12:00,1Mi Thu-12:00,1Mi Fri-12:00,1Mi Sat-12:00,1Mi Sun-20:00,off",
 		},
+		{
+			"11:00,333;13:40,666;23:50,10M;23:59,off",
+			BwTimetable{
+				BwTimeSlot{DayOfTheWeek: 0, HHMM: 1100, Bandwidth: BwPair{Tx: 333 * 1024, Rx: 333 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 1, HHMM: 1100, Bandwidth: BwPair{Tx: 333 * 1024, Rx: 333 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 2, HHMM: 1100, Bandwidth: BwPair{Tx: 333 * 1024, Rx: 333 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 3, HHMM: 1100, Bandwidth: BwPair{Tx: 333 * 1024, Rx: 333 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 4, HHMM: 1100, Bandwidth: BwPair{Tx: 333 * 1024, Rx: 333 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 5, HHMM: 1100, Bandwidth: BwPair{Tx: 333 * 1024, Rx: 333 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 6, HHMM: 1100, Bandwidth: BwPair{Tx: 333 * 1024, Rx: 333 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 0, HHMM: 1340, Bandwidth: BwPair{Tx: 666 * 1024, Rx: 666 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 1, HHMM: 1340, Bandwidth: BwPair{Tx: 666 * 1024, Rx: 666 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 2, HHMM: 1340, Bandwidth: BwPair{Tx: 666 * 1024, Rx: 666 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 3, HHMM: 1340, Bandwidth: BwPair{Tx: 666 * 1024, Rx: 666 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 4, HHMM: 1340, Bandwidth: BwPair{Tx: 666 * 1024, Rx: 666 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 5, HHMM: 1340, Bandwidth: BwPair{Tx: 666 * 1024, Rx: 666 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 6, HHMM: 1340, Bandwidth: BwPair{Tx: 666 * 1024, Rx: 666 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 0, HHMM: 2350, Bandwidth: BwPair{Tx: 10 * 1024 * 1024, Rx: 10 * 1024 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 1, HHMM: 2350, Bandwidth: BwPair{Tx: 10 * 1024 * 1024, Rx: 10 * 1024 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 2, HHMM: 2350, Bandwidth: BwPair{Tx: 10 * 1024 * 1024, Rx: 10 * 1024 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 3, HHMM: 2350, Bandwidth: BwPair{Tx: 10 * 1024 * 1024, Rx: 10 * 1024 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 4, HHMM: 2350, Bandwidth: BwPair{Tx: 10 * 1024 * 1024, Rx: 10 * 1024 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 5, HHMM: 2350, Bandwidth: BwPair{Tx: 10 * 1024 * 1024, Rx: 10 * 1024 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 6, HHMM: 2350, Bandwidth: BwPair{Tx: 10 * 1024 * 1024, Rx: 10 * 1024 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 0, HHMM: 2359, Bandwidth: BwPair{Tx: -1, Rx: -1}},
+				BwTimeSlot{DayOfTheWeek: 1, HHMM: 2359, Bandwidth: BwPair{Tx: -1, Rx: -1}},
+				BwTimeSlot{DayOfTheWeek: 2, HHMM: 2359, Bandwidth: BwPair{Tx: -1, Rx: -1}},
+				BwTimeSlot{DayOfTheWeek: 3, HHMM: 2359, Bandwidth: BwPair{Tx: -1, Rx: -1}},
+				BwTimeSlot{DayOfTheWeek: 4, HHMM: 2359, Bandwidth: BwPair{Tx: -1, Rx: -1}},
+				BwTimeSlot{DayOfTheWeek: 5, HHMM: 2359, Bandwidth: BwPair{Tx: -1, Rx: -1}},
+				BwTimeSlot{DayOfTheWeek: 6, HHMM: 2359, Bandwidth: BwPair{Tx: -1, Rx: -1}},
+			},
+			false,
+			"Sun-11:00,333Ki Mon-11:00,333Ki Tue-11:00,333Ki Wed-11:00,333Ki Thu-11:00,333Ki Fri-11:00,333Ki Sat-11:00,333Ki Sun-13:40,666Ki Mon-13:40,666Ki Tue-13:40,666Ki Wed-13:40,666Ki Thu-13:40,666Ki Fri-13:40,666Ki Sat-13:40,666Ki Sun-23:50,10Mi Mon-23:50,10Mi Tue-23:50,10Mi Wed-23:50,10Mi Thu-23:50,10Mi Fri-23:50,10Mi Sat-23:50,10Mi Sun-23:59,off Mon-23:59,off Tue-23:59,off Wed-23:59,off Thu-23:59,off Fri-23:59,off Sat-23:59,off",
+		},
+		{
+			"Mon-11:00,333;Tue-13:40,666:333;Fri-00:00,10M;Sat-10:00,off;Sun-23:00,666",
+			BwTimetable{
+				BwTimeSlot{DayOfTheWeek: 1, HHMM: 1100, Bandwidth: BwPair{Tx: 333 * 1024, Rx: 333 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 2, HHMM: 1340, Bandwidth: BwPair{Tx: 666 * 1024, Rx: 333 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 5, HHMM: 0000, Bandwidth: BwPair{Tx: 10 * 1024 * 1024, Rx: 10 * 1024 * 1024}},
+				BwTimeSlot{DayOfTheWeek: 6, HHMM: 1000, Bandwidth: BwPair{Tx: -1, Rx: -1}},
+				BwTimeSlot{DayOfTheWeek: 0, HHMM: 2300, Bandwidth: BwPair{Tx: 666 * 1024, Rx: 666 * 1024}},
+			},
+			false,
+			"Mon-11:00,333Ki Tue-13:40,666Ki:333Ki Fri-00:00,10Mi Sat-10:00,off Sun-23:00,666Ki",
+		},
 	} {
 		tt := BwTimetable{}
 		err := tt.Set(test.in)