diff --git a/client/fed_test.go b/client/fed_test.go index 60f1582d1..eb6bff5e5 100644 --- a/client/fed_test.go +++ b/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") +} diff --git a/client/handle_http.go b/client/handle_http.go index fd2bddc80..8463dea8b 100644 --- a/client/handle_http.go +++ b/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") diff --git a/client/main.go b/client/main.go index fff23f90f..eface4bf0 100644 --- a/client/main.go +++ b/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 { diff --git a/client/resources/test-https-origin.yml b/client/resources/test-https-origin.yml new file mode 100644 index 000000000..b38646f9e --- /dev/null +++ b/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