Ls fails when both -O & -D, added check

Added check for 405 returned from object store if it does not allow
PROPFIND. This is hit when an origin/namespace enables listings but the
object store does not have a PROPFIND command. Added unit test for this
check as well.

client/fed_test.go

@@ -26,6 +26,8 @@ import (
 	_ "embed"
 	"fmt"
 	"io"
+	"net/http"
+	"net/http/httptest"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -63,6 +65,9 @@ var (
 
 	//go:embed resources/pub-origin-no-directread.yml
 	pubOriginNoDirectRead string
+
+	//go:embed resources/test-https-origin.yml
+	httpsOriginConfig string
 )
 
 // A test that spins up a federation, and tests object get and put
@@ -893,29 +898,13 @@ func TestObjectList(t *testing.T) {
 		}
 	})
 
-	// Test that when both dironly and objectonly are specified, we work as normal
+	// Test that when both dironly and objectonly are specified, we throw an error
 	t.Run("testPelicanObjectLsDirOnlyAndObjectOnly", func(t *testing.T) {
-		// Set path for ls
-		for _, export := range fed.Exports {
-			listURL, r, w, originalStdout, buf := setupObjectListTest(export)
-			go func() {
-				if export.Capabilities.PublicReads {
-					err := client.DoList(fed.Ctx, listURL, client.WithObjectOnlyOption(true), client.WithDirOnlyOption(true))
-					require.NoError(t, err)
-					w.Close()
-				} else {
-					err := client.DoList(fed.Ctx, listURL, client.WithObjectOnlyOption(true), client.WithDirOnlyOption(true), client.WithTokenLocation(tempToken.Name()))
-					require.NoError(t, err)
-					w.Close()
-				}
-			}()
-			_, err = io.Copy(buf, r)
-			require.NoError(t, err)
-			os.Stdout = originalStdout
+		listURL := fmt.Sprintf("pelican://%s:%s%s", param.Server_Hostname.GetString(), strconv.Itoa(param.Server_WebPort.GetInt()), fed.Exports[0].FederationPrefix)
 
-			assert.Contains(t, buf.String(), "hello_world.txt")
-			assert.Contains(t, buf.String(), "test")
-		}
+		err := client.DoList(fed.Ctx, listURL, client.WithObjectOnlyOption(true), client.WithDirOnlyOption(true))
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "cannot specify both dironly and object only flags, as they are mutually exclusive")
 	})
 
 	// Test that long works with JSON format
@@ -988,3 +977,43 @@ func TestObjectList(t *testing.T) {
 		assert.Contains(t, err.Error(), "404: object not found")
 	})
 }
+
+// This tests object ls but for an origin that supports listings but with an object store that does not support PROPFIND.
+// We should get a 405 returned. This is a seperate test since we need a completely different origin
+func TestObjectList405Error(t *testing.T) {
+	viper.Reset()
+	viper.Set("ConfigDir", t.TempDir())
+	server_utils.ResetOriginExports()
+	defer viper.Reset()
+	defer server_utils.ResetOriginExports()
+	err := config.InitClient()
+	require.NoError(t, err)
+
+	// Set up our http backend so that we can return a 405 on a PROPFIND
+	body := "Hello, World!"
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == "HEAD" && r.URL.Path == "/test2/hello_world" {
+			w.Header().Set("Content-Length", strconv.Itoa(len(body)))
+			w.WriteHeader(http.StatusOK)
+			return
+		} else if r.Method == "GET" && r.URL.Path == "/test2/hello_world" {
+			w.Header().Set("Content-Length", strconv.Itoa(len(body)))
+			w.WriteHeader(http.StatusPartialContent)
+			_, err := w.Write([]byte(body))
+			require.NoError(t, err)
+			return
+		} else if r.Method == "PROPFIND" && r.URL.Path == "/test2/hello_world" {
+			w.WriteHeader(http.StatusMethodNotAllowed)
+		}
+	}))
+	defer srv.Close()
+	viper.Set("Origin.HttpServiceUrl", srv.URL+"/test2")
+
+	config.InitConfig()
+	fed := fed_test_utils.NewFedTest(t, httpsOriginConfig)
+	host := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt())
+
+	err = client.DoList(fed.Ctx, "pelican://"+host+"/test/hello_world")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "405: object listings are not supported by the specified origin")
+}

client/handle_http.go

@@ -2499,6 +2499,9 @@ func listHttp(ctx context.Context, remoteObjectUrl *url.URL, directorUrl string,
 				fileInfos = append(fileInfos, file)
 				return fileInfos, nil
 			}
+		} else if gowebdav.IsErrCode(err, http.StatusMethodNotAllowed) {
+			// We replace the error from gowebdav with our own because gowebdav returns: "ReadDir /prefix/different-path/: 405" which is not very user friendly
+			return nil, errors.New("405: object listings are not supported by the specified origin")
 		}
 		// Otherwise, a different error occurred and we should return it
 		return nil, errors.Wrap(err, "failed to read remote directory")

client/main.go

@@ -532,7 +532,7 @@ func DoList(ctx context.Context, remoteObject string, options ...TransferOption)
 	long := false
 	dironly := false
 	objectonly := false
-	jsn := false
+	asJSON := false
 	for _, option := range options {
 		switch option.Ident() {
 		case identTransferOptionTokenLocation{}:
@@ -548,14 +548,13 @@ func DoList(ctx context.Context, remoteObject string, options ...TransferOption)
 		case identTransferOptionObjectOnly{}:
 			objectonly = option.Value().(bool)
 		case identTransferOptionJson{}:
-			jsn = option.Value().(bool)
+			asJSON = option.Value().(bool)
 		}
 	}
 
 	// If a user specifies dirOnly and objectOnly, this means basic functionality (list both objects and directories) so just remove the flags
 	if dironly && objectonly {
-		dironly = false
-		objectonly = false
+		return errors.New("cannot specify both dironly and object only flags, as they are mutually exclusive")
 	}
 
 	if ns.UseTokenOnRead && token == "" {
@@ -575,7 +574,7 @@ func DoList(ctx context.Context, remoteObject string, options ...TransferOption)
 	if long {
 		w := tabwriter.NewWriter(os.Stdout, 1, 2, 10, ' ', tabwriter.TabIndent|tabwriter.DiscardEmptyColumns)
 		// If we want JSON format, we append the file info to a slice of fileInfo structs so that we can marshal it
-		if jsn {
+		if asJSON {
 			jsonData, err := json.Marshal(fileInfos)
 			if err != nil {
 				return errors.Wrap(err, "failed to marshal file/directory info to JSON format")
@@ -588,7 +587,7 @@ func DoList(ctx context.Context, remoteObject string, options ...TransferOption)
 			fmt.Fprintln(w, info.Name+"\t"+strconv.FormatInt(info.Size, 10)+"\t"+info.ModTime.Format("2006-01-02 15:04:05"))
 		}
 		w.Flush()
-	} else if jsn {
+	} else if asJSON {
 		// In this case, we are not using the long option (-L) and want a JSON format
 		jsonInfo := []string{}
 		for _, info := range fileInfos {

client/resources/test-https-origin.yml

@@ -0,0 +1,8 @@
+# This is used to test an origin with an https backend
+Logging:
+  Cache:
+    Scitokens: debug
+Origin:
+  StorageType: https
+  FederationPrefix: /test
+  EnablePublicReads: true