Repository: rclone/rclone
Pull request #7699 (2024-03-28 08:52:38)

Remove the temporary Uloz.to filesize limit
<!--
Thank you very much for contributing code or documentation to rclone! Please
fill out the following questions to make it easier for us to review your
changes.

You do not need to check all the boxes below all at once, feel free to take
your time and add more commits. If you're done and ready for review, please
check the last box.
-->

#### What is the purpose of this change?

<!--
Describe the changes here
-->
Remove the workarounds for uloz.to's file size. 

The combination of large files, absence of `Content-Length` header, and chunked encoding triggers the issue. 

- files under 2.5G are uploaded fine as demonstrated. 
- setting the `Content-Length` either fixes the issue (non-negative value), or triggers a 400 if the file size is not known (-1). 
- removing chunked encoding does nothing (if `Content-Length` is set), or triggers a 415 (if `Content-Length` is not specified)

This currently means we still run into the issue with large files if rclone doesn't know their size beforehand. Such scenario is presumably very rare though, and explicitly setting the size to -1 isn't handled properly in rclone's multipart generation code. 

#### Was the change discussed in an issue or in the forum before?

https://github.com/rclone/rclone/pull/7552

<!--
Link issues and relevant forum posts here.
-->

#### 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 :-)



Patch:
diff --git a/backend/ulozto/ulozto.go b/backend/ulozto/ulozto.go
index b5033107a..44810f310 100644
--- a/backend/ulozto/ulozto.go
+++ b/backend/ulozto/ulozto.go
@@ -38,8 +38,6 @@ const (
 	maxSleep      = 2 * time.Second
 	decayConstant = 2 // bigger for slower decay, exponential
 	rootURL       = "https://apis.uloz.to"
-	// TODO temporary limitation, remove with chunked upload impl
-	maxFileSizeBytes = 2500 * 1024 * 1024
 )
 
 // Options defines the configuration for this backend
@@ -194,9 +192,7 @@ func errorHandler(resp *http.Response) error {
 // retryErrorCodes is a slice of error codes that we will retry
 var retryErrorCodes = []int{
 	429, // Too Many Requests.
-	// TODO: random 500s should be retried but the error code corresponds to a known issue with uploading large files,
-	//   leading to numerous (slow & resource consuming) retries. Don't retry them until the root cause is addressed.
-	// 500, // Internal Server Error
+	500, // Internal Server Error
 	502, // Bad Gateway
 	503, // Service Unavailable
 	504, // Gateway Timeout
@@ -346,6 +342,11 @@ func (f *Fs) uploadUnchecked(ctx context.Context, name, parentSlug string, info
 		MultipartFileName:    encodedName,
 		Parameters:           url.Values{},
 	}
+	if info.Size() > 0 {
+		size := info.Size()
+		opts.ContentLength = &size
+	}
+
 	var uploadResponse api.SendFilePayloadResponse
 
 	err = f.pacer.CallNoRetry(func() (bool, error) {
@@ -442,10 +443,6 @@ func (f *Fs) uploadUnchecked(ctx context.Context, name, parentSlug string, info
 
 // Put implements the mandatory method fs.Fs.Put.
 func (f *Fs) Put(ctx context.Context, in io.Reader, src fs.ObjectInfo, options ...fs.OpenOption) (fs.Object, error) {
-	// TODO: workaround for uloz.to's bug. Remove when chunked upload support is implemented.
-	if src.Size() > maxFileSizeBytes {
-		return nil, errors.New("file size over the supported max threshold")
-	}
 	existingObj, err := f.NewObject(ctx, src.Remote())
 
 	switch {
diff --git a/docs/content/local.md b/docs/content/local.md
index d45fefb89..bef94be15 100644
--- a/docs/content/local.md
+++ b/docs/content/local.md
@@ -237,7 +237,7 @@ $ tree /tmp/a
 Copying the entire directory with '-l'
 
 ```
-$ rclone copyto -l /tmp/a/file1 remote:/tmp/a/
+$ rclone copy -l /tmp/a/ remote:/tmp/a/
 ```
 
 The remote files are created with a '.rclonelink' suffix
@@ -261,7 +261,7 @@ $ rclone cat remote:/tmp/a/file2.rclonelink
 Copying them back with '-l'
 
 ```
-$ rclone copyto -l remote:/tmp/a/ /tmp/b/
+$ rclone copy -l remote:/tmp/a/ /tmp/b/
 
 $ tree /tmp/b
 /tmp/b
@@ -280,6 +280,16 @@ $ tree /tmp/b
 └── file2.rclonelink
 ````
 
+If you want to copy a single file with `-l` then you must use the `.rclonelink` suffix.
+
+```
+$ rclone copy -l remote:/tmp/a/file1.rclonelink /tmp/c
+
+$ tree /tmp/c
+/tmp/c
+└── file1 -> ./file4
+```
+
 Note that this flag is incompatible with `-copy-links` / `-L`.
 
 ### Restricting filesystems with --one-file-system
diff --git a/lib/http/middleware.go b/lib/http/middleware.go
index 3ee5c0cdc..06b82278b 100644
--- a/lib/http/middleware.go
+++ b/lib/http/middleware.go
@@ -195,6 +195,14 @@ func MiddlewareCORS(allowOrigin string) Middleware {
 // MiddlewareStripPrefix instantiates middleware that removes the BaseURL from the path
 func MiddlewareStripPrefix(prefix string) Middleware {
 	return func(next http.Handler) http.Handler {
-		return http.StripPrefix(prefix, next)
+		stripPrefixHandler := http.StripPrefix(prefix, next)
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			// Allow OPTIONS on the root only
+			if r.URL.Path == "/" && r.Method == "OPTIONS" {
+				next.ServeHTTP(w, r)
+				return
+			}
+			stripPrefixHandler.ServeHTTP(w, r)
+		})
 	}
 }


Test patch:
diff --git a/backend/ulozto/ulozto_test.go b/backend/ulozto/ulozto_test.go
index 99507fc24..8a7a54173 100644
--- a/backend/ulozto/ulozto_test.go
+++ b/backend/ulozto/ulozto_test.go
@@ -1,7 +1,6 @@
 package ulozto
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"testing"
@@ -10,7 +9,6 @@ import (
 	"github.com/rclone/rclone/backend/ulozto/api"
 	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/fs/hash"
-	"github.com/rclone/rclone/fs/object"
 	"github.com/rclone/rclone/fs/operations"
 	"github.com/rclone/rclone/fstest"
 	"github.com/stretchr/testify/require"
@@ -86,32 +84,3 @@ func TestListWithoutMetadata(t *testing.T) {
 	// Tear down
 	require.NoError(t, operations.Purge(ctx, f, ""))
 }
-
-// TestUploadLargeFile verifies that files over the supported threshold are not uploaded.
-func TestUploadLargeFile(t *testing.T) {
-	const (
-		remoteName = "TestUlozto:"
-		payload    = "foobar"
-		filesize   = maxFileSizeBytes + 1
-	)
-	ctx := context.Background()
-	fstest.Initialise()
-	subRemoteName, subRemoteLeaf, err := fstest.RandomRemoteName(remoteName)
-	require.NoError(t, err)
-	f, err := fs.NewFs(ctx, subRemoteName)
-	if errors.Is(err, fs.ErrorNotFoundInConfigFile) {
-		t.Logf("Didn't find %q in config file - skipping tests", remoteName)
-		return
-	}
-	require.NoError(t, err)
-
-	file := fstest.Item{ModTime: time.UnixMilli(123456789), Path: subRemoteLeaf, Size: int64(filesize)}
-	obji := object.NewStaticObjectInfo(file.Path, file.ModTime, file.Size, true, nil, nil)
-	// The payload buffer is just a placeholder which shouldn't be used
-	_, err = f.Put(ctx, bytes.NewBufferString(payload), obji)
-
-	require.Error(t, err, "File size over the supported max threshold.")
-
-	// Verify the remote stayed intact
-	fstest.CheckListing(t, f, []fstest.Item{})
-}
diff --git a/lib/http/middleware_test.go b/lib/http/middleware_test.go
index 15d0b0525..845e28013 100644
--- a/lib/http/middleware_test.go
+++ b/lib/http/middleware_test.go
@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -329,8 +330,11 @@ var _testCORSHeaderKeys = []string{
 
 func TestMiddlewareCORS(t *testing.T) {
 	servers := []struct {
-		name string
-		http Config
+		name    string
+		http    Config
+		tryRoot bool
+		method  string
+		status  int
 	}{
 		{
 			name: "CustomOrigin",
@@ -338,6 +342,40 @@ func TestMiddlewareCORS(t *testing.T) {
 				ListenAddr:  []string{"127.0.0.1:0"},
 				AllowOrigin: "http://test.rclone.org",
 			},
+			method: "GET",
+			status: http.StatusOK,
+		},
+		{
+			name: "WithBaseURL",
+			http: Config{
+				ListenAddr:  []string{"127.0.0.1:0"},
+				AllowOrigin: "http://test.rclone.org",
+				BaseURL:     "/baseurl/",
+			},
+			method: "GET",
+			status: http.StatusOK,
+		},
+		{
+			name: "WithBaseURLTryRootGET",
+			http: Config{
+				ListenAddr:  []string{"127.0.0.1:0"},
+				AllowOrigin: "http://test.rclone.org",
+				BaseURL:     "/baseurl/",
+			},
+			method:  "GET",
+			status:  http.StatusNotFound,
+			tryRoot: true,
+		},
+		{
+			name: "WithBaseURLTryRootOPTIONS",
+			http: Config{
+				ListenAddr:  []string{"127.0.0.1:0"},
+				AllowOrigin: "http://test.rclone.org",
+				BaseURL:     "/baseurl/",
+			},
+			method:  "OPTIONS",
+			status:  http.StatusOK,
+			tryRoot: true,
 		},
 	}
 
@@ -354,9 +392,14 @@ func TestMiddlewareCORS(t *testing.T) {
 			s.Serve()
 
 			url := testGetServerURL(t, s)
+			// Try the query on the root, ignoring the baseURL
+			if ss.tryRoot {
+				slash := strings.LastIndex(url[:len(url)-1], "/")
+				url = url[:slash+1]
+			}
 
 			client := &http.Client{}
-			req, err := http.NewRequest("GET", url, nil)
+			req, err := http.NewRequest(ss.method, url, nil)
 			require.NoError(t, err)
 
 			resp, err := client.Do(req)
@@ -365,8 +408,11 @@ func TestMiddlewareCORS(t *testing.T) {
 				_ = resp.Body.Close()
 			}()
 
-			require.Equal(t, http.StatusOK, resp.StatusCode, "should return ok")
+			require.Equal(t, ss.status, resp.StatusCode, "should return expected error code")
 
+			if ss.status == http.StatusNotFound {
+				return
+			}
 			testExpectRespBody(t, resp, expected)
 
 			for _, key := range _testCORSHeaderKeys {