fix(archiver): keep the configured archive name when resolution fails

internal/http/services/archiver/handler.go

@@ -114,6 +114,10 @@ func (c *Config) init() {
 		c.Prefix = "download_archive"
 	}
 
+	// Sanitize before the default so s.config.Name is always safe to put in a
+	// Content-Disposition header: net.ContentDispositionAttachment escapes the filename* form
+	// but emits filename="..." verbatim, and a configured name is not otherwise validated.
+	c.Name = sanitizeArchiveName(c.Name)
 	if c.Name == "" {
 		c.Name = "download"
 	}
@@ -204,8 +208,9 @@ func (s *svc) allAllowed(paths []string) error {
 */
 
 // resourceName resolves the name of a single resource so the archive can be named after it instead
-// of the generic "download". It returns an empty string on any failure, so the caller keeps the
-// default name. The name is sanitized via sanitizeArchiveName.
+// of the configured default. It returns an error when the lookup fails, and an empty name when the
+// resource yields nothing usable: neither a name nor a path, or a name that sanitizes away.
+// Callers fall back to their own default on either. The name is sanitized via sanitizeArchiveName.
 func (s *svc) resourceName(ctx context.Context, id *provider.ResourceId) (string, error) {
 	gatewayClient, err := s.gatewaySelector.Next()
 	if err != nil {
@@ -232,6 +237,20 @@ func (s *svc) resourceName(ctx context.Context, id *provider.ResourceId) (string
 	return sanitizeArchiveName(name), nil
 }
 
+// archiveName returns the name for the archive: the resource's own name when a single resource
+// was requested and could be resolved, and the configured default otherwise.
+func (s *svc) archiveName(ctx context.Context, resources []*provider.ResourceId) string {
+	if len(resources) != 1 {
+		return s.config.Name
+	}
+	name, err := s.resourceName(ctx, resources[0])
+	if err != nil || name == "" {
+		s.log.Debug().Err(err).Msg("could not resolve the archive name, using the default")
+		return s.config.Name
+	}
+	return name
+}
+
 // sanitizeArchiveName removes characters that would break the Content-Disposition header (CR, LF,
 // double quote) or let the name act as a path (slash, backslash), plus all control characters
 // (C0, DEL and C1). It returns an empty string if nothing usable is left.
@@ -305,19 +324,11 @@ func (s *svc) Handler() http.Handler {
 		}
 
 		// Name the archive after the resource when a single one was requested, instead of the
-		// generic "download". The name must be resolved here, before the body is streamed: the
+		// configured default. The name must be resolved here, before the body is streamed: the
 		// Content-Disposition header below is written before CreateZip/CreateTar run, so the name
 		// the walker resolves while building the archive would come too late.
 		// See https://github.com/opencloud-eu/reva/issues/308
-		archName := s.config.Name
-		if len(resources) == 1 {
-			if name, err := s.resourceName(ctx, resources[0]); name != "" && err == nil {
-				archName = name
-			} else {
-				s.log.Debug().Err(err).Msg("could not resolve the archive name, using the default")
-				archName = "download"
-			}
-		}
+		archName := s.archiveName(ctx, resources)
 		if format == "tar" {
 			archName += ".tar"
 		} else {

internal/http/services/archiver/handler_internal_test.go

@@ -104,6 +104,89 @@ func TestResourceName(t *testing.T) {
 	}
 }
 
+// TestArchiveName covers what archiveName does with resourceName's result, which TestResourceName
+// above does not reach. archiveName maps several distinct failures onto one fallback, so a
+// regression in that fallback stays invisible to the callee's own tests.
+//
+// resourceName returns ("", nil) when the resource has no usable name; the caller must treat that
+// like an error and keep s.config.Name. A deployment that configured a name must never silently
+// receive a different one.
+func TestArchiveName(t *testing.T) {
+	const configured = "meinarchiv"
+	ok := func(info *provider.ResourceInfo) *provider.StatResponse {
+		return &provider.StatResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, Info: info}
+	}
+
+	cases := []struct {
+		name    string
+		single  bool
+		resp    *provider.StatResponse
+		statErr error
+		selErr  error
+		want    string
+	}{
+		{name: "resolved name wins", single: true, resp: ok(&provider.ResourceInfo{Name: "Documents"}), want: "Documents"},
+		{name: "stat error keeps the configured name", single: true, statErr: errors.New("boom"), want: configured},
+		{name: "non-OK status keeps the configured name", single: true, resp: &provider.StatResponse{Status: &rpc.Status{Code: rpc.Code_CODE_NOT_FOUND}}, want: configured},
+		{name: "selector error keeps the configured name", single: true, selErr: errors.New("no gateway"), want: configured},
+		{name: "name sanitizing to empty keeps the configured name", single: true, resp: ok(&provider.ResourceInfo{Name: "/"}), want: configured},
+		{name: "empty name and path keep the configured name", single: true, resp: ok(&provider.ResourceInfo{}), want: configured},
+		{name: "several resources keep the configured name", single: false, want: configured},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			gw := cs3mocks.NewGatewayAPIClient(t)
+			if tc.single && tc.selErr == nil {
+				gw.EXPECT().Stat(mock.Anything, mock.Anything).Return(tc.resp, tc.statErr).Once()
+			}
+			log := zerolog.Nop()
+			s := &svc{
+				gatewaySelector: fakeSelector{client: gw, err: tc.selErr},
+				log:             &log,
+				config:          &Config{Name: configured},
+			}
+
+			resources := []*provider.ResourceId{{OpaqueId: "x"}}
+			if !tc.single {
+				resources = append(resources, &provider.ResourceId{OpaqueId: "y"})
+			}
+
+			if got := s.archiveName(context.Background(), resources); got != tc.want {
+				t.Errorf("archiveName() = %q, want %q", got, tc.want)
+			}
+		})
+	}
+}
+
+// TestConfigNameSanitized pins that a configured archive name cannot break the
+// Content-Disposition header. archiveName returns s.config.Name on every fallback path, so the
+// value reaches the header unmodified unless it is sanitized once at config time.
+func TestConfigNameSanitized(t *testing.T) {
+	cases := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{"plain name kept", "meinarchiv", "meinarchiv"},
+		{"double quote removed", `my"archive`, "myarchive"},
+		{"crlf removed", "arch\r\nive", "archive"},
+		{"slashes removed", "a/b", "ab"},
+		{"unset falls back", "", "download"},
+		{"sanitizing to empty falls back", "/", "download"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			c := &Config{Name: tc.in}
+			c.init()
+			if c.Name != tc.want {
+				t.Errorf("Config.init() Name = %q, want %q", c.Name, tc.want)
+			}
+		})
+	}
+}
+
 func TestArchiveNameContentDisposition(t *testing.T) {
 	// A name with umlauts survives sanitization and still encodes correctly:
 	// net.ContentDispositionAttachment emits both the RFC 6266 filename* form and the raw filename.