Merge pull request #11 from kjsanger/bug/return-404-correctly

Fix error-type checking to send HTTP 404 correctly

cmd/sqyrrl.go

@@ -29,8 +29,6 @@ import (
 	"github.com/spf13/cobra"
 	"golang.org/x/term"
 
-	"github.com/sirupsen/logrus"
-
 	"sqyrrl/server"
 )
 
@@ -128,11 +126,6 @@ func startServer(cmd *cobra.Command, args []string) {
 }
 
 func main() {
-	// go-irodsclient uses logrus, so set its log level to something appropriate. I did
-	// look at a shim to forward logrus log events to zerolog, in order to manage them
-	// there, but that seemed not to add enough value to justify the work.
-	logrus.SetLevel(logrus.ErrorLevel)
-
 	rootCmd := &cobra.Command{
 		Use:              "sqyrrl",
 		Short:            "Sqyrrl.",

go.mod

@@ -3,13 +3,12 @@ module sqyrrl
 go 1.22
 
 require (
-	github.com/cyverse/go-irodsclient v0.14.1
+	github.com/cyverse/go-irodsclient v0.14.3
 	github.com/microcosm-cc/bluemonday v1.0.26
 	github.com/onsi/ginkgo/v2 v2.17.0
 	github.com/onsi/gomega v1.30.0
 	github.com/rs/xid v1.5.0
 	github.com/rs/zerolog v1.32.0
-	github.com/sirupsen/logrus v1.7.0
 	github.com/spf13/cobra v1.8.0
 	golang.org/x/term v0.18.0
 )
@@ -29,6 +28,7 @@ require (
 	github.com/mitchellh/go-homedir v1.1.0 // indirect
 	github.com/patrickmn/go-cache v2.1.0+incompatible // indirect
 	github.com/pkg/errors v0.9.1 // indirect
+	github.com/sirupsen/logrus v1.7.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
 	github.com/stretchr/testify v1.8.4 // indirect
 	golang.org/x/net v0.21.0 // indirect
@@ -38,3 +38,5 @@ require (
 	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
+
+// replace github.com/cyverse/go-irodsclient => ../go-irodsclient

go.sum

@@ -6,8 +6,8 @@ github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMn
 github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc=
 github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
-github.com/cyverse/go-irodsclient v0.14.1 h1:8PixUAs4Q/A0k1vM8cMoCd6EOyMESfkMVGseVjmNMDg=
-github.com/cyverse/go-irodsclient v0.14.1/go.mod h1:eBXha3cwfrM0p1ijYVqsrLJQHpRwTfpA4c5dKCQsQFc=
+github.com/cyverse/go-irodsclient v0.14.3 h1:B6BI3H2nUoh6pu76EB0eKBPdFY9lq1qppuHfDbz9UB0=
+github.com/cyverse/go-irodsclient v0.14.3/go.mod h1:eBXha3cwfrM0p1ijYVqsrLJQHpRwTfpA4c5dKCQsQFc=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

server/handlers_test.go

@@ -110,7 +110,7 @@ var _ = Describe("iRODS Get Handler", func() {
 				conn, err = irodsFS.GetIOConnection()
 				Expect(err).NotTo(HaveOccurred())
 
-				err = fs.ChangeDataObjectAccess(conn, remotePath, types.IRODSAccessLevelRead,
+				err = fs.ChangeDataObjectAccess(conn, remotePath, types.IRODSAccessLevelReadObject,
 					server.PublicUser, testZone, false)
 				Expect(err).NotTo(HaveOccurred())
 
@@ -125,12 +125,12 @@ var _ = Describe("iRODS Get Handler", func() {
 						Str("zone", ac.UserZone).
 						Str("expected_zone", testZone).
 						Str("access", ac.AccessLevel.ChmodString()).
-						Str("expected_access", types.IRODSAccessLevelRead.ChmodString()).
+						Str("expected_access", types.IRODSAccessLevelReadObject.ChmodString()).
 						Msg("ACL")
 
 					if ac.UserName == server.PublicUser &&
 						ac.UserZone == testZone &&
-						server.LevelsEqual(ac.AccessLevel, types.IRODSAccessLevelRead) {
+						server.LevelsEqual(ac.AccessLevel, types.IRODSAccessLevelReadObject) {
 						publicAccess = true
 					}
 				}

server/irods.go

@@ -169,7 +169,7 @@ func isPublicReadable(logger zerolog.Logger, filesystem *fs.FileSystem,
 
 		if effectiveUserZone == pathZone &&
 			ac.UserName == PublicUser &&
-			LevelsEqual(ac.AccessLevel, types.IRODSAccessLevelRead) {
+			LevelsEqual(ac.AccessLevel, types.IRODSAccessLevelReadObject) {
 			logger.Trace().
 				Str("path", path).
 				Msg("Public read access found")
@@ -196,11 +196,28 @@ func getFileRange(logger zerolog.Logger, w http.ResponseWriter, r *http.Request,
 
 	defer filesystem.Release()
 
-	if !filesystem.ExistsFile(path) {
-		logger.Info().
-			Str("path", path).
-			Msg("Requested path does not exist")
-		http.NotFound(w, r)
+	// Don't use filesystem.ExistsFile(path) here because it will return false if the
+	// file _does_ exist on the iRODS server, but the server is down or unreachable.
+	//
+	// filesystem.StatFile(path) is better because we can check for the error type.
+	_, err = filesystem.StatFile(path)
+	if err != nil {
+		if types.IsAuthError(err) {
+			logger.Err(err).
+				Str("path", path).
+				Msg("Failed to authenticate with iRODS")
+			writeErrorResponse(logger, w, http.StatusUnauthorized)
+			return
+		}
+		if types.IsFileNotFoundError(err) {
+			logger.Info().
+				Str("path", path).
+				Msg("Requested path does not exist")
+			writeErrorResponse(logger, w, http.StatusNotFound)
+			return
+		}
+		logger.Err(err).Str("path", path).Msg("Failed to stat file")
+		writeErrorResponse(logger, w, http.StatusInternalServerError)
 		return
 	}