Skip to content

Commit

Permalink
Merge pull request PelicanPlatform#1710 from jhiemstrawisc/issue-1705
Browse files Browse the repository at this point in the history
Fix nil pointer bug caused by trying to list namespaces with no coll URL
  • Loading branch information
jhiemstrawisc authored Nov 5, 2024
2 parents 6821696 + cace7e4 commit 37efea4
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 1 deletion.
4 changes: 4 additions & 0 deletions client/handle_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -2591,6 +2591,10 @@ func (te *TransferEngine) walkDirUpload(job *clientTransferJob, transfers []tran
// This function performs the ls command by walking through the specified collections and printing the contents of the files
func listHttp(remoteUrl *pelican_url.PelicanURL, dirResp server_structs.DirectorResponse, token *tokenGenerator) (fileInfos []FileInfo, err error) {
// Get our collection listing host
if dirResp.XPelNsHdr.CollectionsUrl == nil {
return nil, errors.Errorf("Collections URL not found in director response. Are you sure there's an origin for prefix %s that supports listings?", dirResp.XPelNsHdr.Namespace)
}

collectionsUrl := dirResp.XPelNsHdr.CollectionsUrl
log.Debugln("Collections URL: ", collectionsUrl.String())

Expand Down
58 changes: 58 additions & 0 deletions client/handle_http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (

"github.com/pelicanplatform/pelican/config"
"github.com/pelicanplatform/pelican/error_codes"
"github.com/pelicanplatform/pelican/pelican_url"
"github.com/pelicanplatform/pelican/server_structs"
"github.com/pelicanplatform/pelican/server_utils"
"github.com/pelicanplatform/pelican/test_utils"
Expand Down Expand Up @@ -930,3 +931,60 @@ func TestNewTransferEngine(t *testing.T) {
assert.NoError(t, err)
})
}

func TestListHttp(t *testing.T) {
type test struct {
name string
pUrl *pelican_url.PelicanURL
dirResp server_structs.DirectorResponse
expectedError string
}
tests := []test{
{
name: "valid-collections-url",
pUrl: &pelican_url.PelicanURL{
Scheme: "pelican",
Host: "something.com",
Path: "/foo/bar/baz",
},
dirResp: server_structs.DirectorResponse{
XPelNsHdr: server_structs.XPelNs{
RequireToken: false,
Namespace: "/foo/bar",
CollectionsUrl: &url.URL{
Scheme: "https",
Host: "collections.example.com",
},
},
},
expectedError: "no such host", // punt on setting up a real server, and accept this "success" looks like a connection error
},
{
name: "no-collections-url",
pUrl: &pelican_url.PelicanURL{
Scheme: "pelican",
Host: "something.com",
Path: "/foo/bar/baz",
},
dirResp: server_structs.DirectorResponse{
XPelNsHdr: server_structs.XPelNs{
RequireToken: false,
Namespace: "/foo/bar",
},
},
expectedError: "Collections URL not found in director response. Are you sure there's an origin for prefix /foo/bar that supports listings?",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
_, err := listHttp(test.pUrl, test.dirResp, nil)
if test.expectedError != "" {
assert.Error(t, err)
assert.Contains(t, err.Error(), test.expectedError)
} else {
assert.NoError(t, err)
}
})
}
}
2 changes: 1 addition & 1 deletion client/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ func DoList(ctx context.Context, remoteObject string, options ...TransferOption)

fileInfos, err = listHttp(pUrl, dirResp, token)
if err != nil {
return nil, errors.Wrap(err, "failed to do the list")
return nil, errors.Wrap(err, "failed to perform list request")
}

return fileInfos, nil
Expand Down

0 comments on commit 37efea4

Please sign in to comment.