fix: directory marker support (#22)

Author: alukach

crates/core/src/api/list.rs

@@ -2,6 +2,8 @@
 //!
 //! Extracted from `proxy.rs` to keep the gateway focused on orchestration.
 
+use std::collections::HashSet;
+
 use crate::api::list_rewrite::ListRewrite;
 use crate::api::response::{ListBucketResult, ListBucketResultV1, ListCommonPrefix, ListContents};
 use crate::error::ProxyError;
@@ -127,9 +129,37 @@ pub(crate) fn build_list_xml(
         format!("{}/", backend_prefix)
     };
 
+    // Filter out S3 directory marker objects — 0-byte objects created by the
+    // S3 console (or similar tools) to represent "folders". These have a
+    // trailing `/` in their key which `object_store::Path` strips, causing
+    // them to leak into results. We detect them in three ways:
+    //
+    // 1. The marker's path matches a common prefix (e.g. key `photos/` →
+    //    Path("photos") collides with CommonPrefix "photos").
+    // 2. The marker's path equals the backend prefix itself (e.g. the root
+    //    directory marker for the entire backend prefix).
+    // 3. The marker's path equals the full listing prefix
+    //    (backend_prefix + client_prefix, minus trailing `/`). This is the
+    //    most common real-world case: listing "harvard-lil/staging-gov-data/"
+    //    returns a 0-byte key "harvard-lil/staging-gov-data/" which
+    //    object_store converts to Path("harvard-lil/staging-gov-data").
+    let common_prefix_set: HashSet<&object_store::path::Path> =
+        list_result.common_prefixes.iter().collect();
+
+    let full_list_prefix = format!("{}{}", strip_prefix, params.client_prefix);
+    let list_prefix_trimmed = full_list_prefix.trim_end_matches('/');
+
+    let is_directory_marker = |obj: &object_store::ObjectMeta| -> bool {
+        obj.size == 0
+            && (common_prefix_set.contains(&obj.location)
+                || obj.location.as_ref() == backend_prefix
+                || obj.location.as_ref() == list_prefix_trimmed)
+    };
+
     let mut contents: Vec<ListContents> = list_result
         .objects
         .iter()
+        .filter(|obj| !is_directory_marker(obj))
         .map(|obj| {
             let raw_key = obj.location.to_string();
             ListContents {
@@ -238,9 +268,24 @@ pub(crate) fn build_list_xml_v1(
         format!("{}/", backend_prefix)
     };
 
+    // Filter out S3 directory marker objects (see build_list_xml for details).
+    let common_prefix_set: HashSet<&object_store::path::Path> =
+        list_result.common_prefixes.iter().collect();
+
+    let full_list_prefix = format!("{}{}", strip_prefix, params.client_prefix);
+    let list_prefix_trimmed = full_list_prefix.trim_end_matches('/');
+
+    let is_directory_marker = |obj: &object_store::ObjectMeta| -> bool {
+        obj.size == 0
+            && (common_prefix_set.contains(&obj.location)
+                || obj.location.as_ref() == backend_prefix
+                || obj.location.as_ref() == list_prefix_trimmed)
+    };
+
     let mut contents: Vec<ListContents> = list_result
         .objects
         .iter()
+        .filter(|obj| !is_directory_marker(obj))
         .map(|obj| {
             let raw_key = obj.location.to_string();
             ListContents {
@@ -694,4 +739,240 @@ mod tests {
         );
         assert!(xml.contains("<Marker></Marker>") || xml.contains("<Marker/>"));
     }
+
+    /// Create a ListResult with objects that have explicit sizes, for testing
+    /// directory marker filtering.
+    fn make_list_result_with_sizes(
+        objects: &[(&str, u64)],
+        common_prefixes: &[&str],
+    ) -> ListResult {
+        ListResult {
+            objects: objects
+                .iter()
+                .map(|(k, size)| ObjectMeta {
+                    location: Path::from(*k),
+                    last_modified: Utc::now(),
+                    size: *size,
+                    e_tag: Some("\"abc\"".to_string()),
+                    version: None,
+                })
+                .collect(),
+            common_prefixes: common_prefixes.iter().map(|p| Path::from(*p)).collect(),
+        }
+    }
+
+    #[test]
+    fn test_directory_markers_filtered_from_v2_contents() {
+        let config = make_config(None);
+        // Simulate what object_store returns: a 0-byte "photos" object
+        // (from S3 key "photos/") alongside the "photos" common prefix,
+        // plus a real file.
+        let list_result =
+            make_list_result_with_sizes(&[("photos", 0), ("readme.txt", 42)], &["photos"]);
+
+        let params = ListXmlParams {
+            bucket_name: "my-bucket",
+            client_prefix: "",
+            delimiter: "/",
+            max_keys: 1000,
+            is_truncated: false,
+            key_count: 1,
+            start_after: &None,
+            continuation_token: &None,
+            next_continuation_token: None,
+            encoding_type: &None,
+        };
+
+        let xml = build_list_xml(&params, &list_result, &config, None).unwrap();
+
+        // The directory marker should NOT appear in Contents
+        assert!(
+            !xml.contains("<Key>photos</Key>"),
+            "Directory marker should be filtered out: {xml}"
+        );
+        // The real file should still be present
+        assert!(
+            xml.contains("<Key>readme.txt</Key>"),
+            "Real file should remain: {xml}"
+        );
+        // The common prefix should still be present
+        assert!(
+            xml.contains("<Prefix>photos/</Prefix>"),
+            "Common prefix should remain: {xml}"
+        );
+    }
+
+    #[test]
+    fn test_zero_byte_file_not_filtered_when_no_matching_prefix() {
+        let config = make_config(None);
+        // A 0-byte file that does NOT match any common prefix should be kept.
+        let list_result = make_list_result_with_sizes(&[("empty.txt", 0)], &[]);
+
+        let params = ListXmlParams {
+            bucket_name: "my-bucket",
+            client_prefix: "",
+            delimiter: "/",
+            max_keys: 1000,
+            is_truncated: false,
+            key_count: 1,
+            start_after: &None,
+            continuation_token: &None,
+            next_continuation_token: None,
+            encoding_type: &None,
+        };
+
+        let xml = build_list_xml(&params, &list_result, &config, None).unwrap();
+
+        assert!(
+            xml.contains("<Key>empty.txt</Key>"),
+            "Zero-byte file without matching prefix should be kept: {xml}"
+        );
+    }
+
+    #[test]
+    fn test_directory_markers_filtered_from_v1_contents() {
+        let config = make_config(None);
+        let list_result =
+            make_list_result_with_sizes(&[("photos", 0), ("readme.txt", 42)], &["photos"]);
+
+        let params = ListXmlParamsV1 {
+            bucket_name: "my-bucket",
+            client_prefix: "",
+            delimiter: "/",
+            max_keys: 1000,
+            is_truncated: false,
+            marker: "",
+            next_marker: None,
+            encoding_type: &None,
+        };
+
+        let xml = build_list_xml_v1(&params, &list_result, &config, None).unwrap();
+
+        assert!(
+            !xml.contains("<Key>photos</Key>"),
+            "Directory marker should be filtered out in V1: {xml}"
+        );
+        assert!(
+            xml.contains("<Key>readme.txt</Key>"),
+            "Real file should remain in V1: {xml}"
+        );
+    }
+
+    #[test]
+    fn test_directory_marker_at_list_prefix_filtered_v2() {
+        // Reproduces the real-world bug from source-cooperative/source.coop#245:
+        //
+        // Backend bucket: us-west-2.opendata.source.coop
+        // backend_prefix: "harvard-lil/" (the account-level prefix)
+        // Client lists with prefix: "staging-gov-data/"
+        // Full S3 prefix sent: "harvard-lil/staging-gov-data/"
+        //
+        // S3 returns a 0-byte key "harvard-lil/staging-gov-data/" in Contents
+        // (the directory marker). object_store::Path strips the trailing slash,
+        // giving location "harvard-lil/staging-gov-data". After strip_prefix
+        // ("harvard-lil/"), this becomes "staging-gov-data" — a phantom file
+        // that should not appear in the listing.
+        let config = make_config(Some("harvard-lil"));
+        let list_result = make_list_result_with_sizes(
+            &[
+                ("harvard-lil/staging-gov-data", 0),
+                ("harvard-lil/staging-gov-data/data/file.parquet", 1000),
+            ],
+            &["harvard-lil/staging-gov-data/data"],
+        );
+
+        let params = ListXmlParams {
+            bucket_name: "harvard-lil",
+            client_prefix: "staging-gov-data/",
+            delimiter: "/",
+            max_keys: 1000,
+            is_truncated: false,
+            key_count: 1,
+            start_after: &None,
+            continuation_token: &None,
+            next_continuation_token: None,
+            encoding_type: &None,
+        };
+
+        let xml = build_list_xml(&params, &list_result, &config, None).unwrap();
+
+        // The directory marker should NOT appear in Contents
+        assert!(
+            !xml.contains("<Key>staging-gov-data</Key>"),
+            "Directory marker at list prefix should be filtered out: {xml}"
+        );
+        // The real file should still appear (with prefix stripped)
+        assert!(
+            xml.contains("<Key>staging-gov-data/data/file.parquet</Key>"),
+            "Real file should remain: {xml}"
+        );
+        // The common prefix should still appear
+        assert!(
+            xml.contains("<Prefix>staging-gov-data/data/</Prefix>"),
+            "Common prefix should remain: {xml}"
+        );
+    }
+
+    #[test]
+    fn test_directory_marker_at_list_prefix_filtered_v1() {
+        // Same scenario as V2 test above, but for ListObjectsV1.
+        let config = make_config(Some("harvard-lil"));
+        let list_result = make_list_result_with_sizes(
+            &[
+                ("harvard-lil/staging-gov-data", 0),
+                ("harvard-lil/staging-gov-data/data/file.parquet", 1000),
+            ],
+            &["harvard-lil/staging-gov-data/data"],
+        );
+
+        let params = ListXmlParamsV1 {
+            bucket_name: "harvard-lil",
+            client_prefix: "staging-gov-data/",
+            delimiter: "/",
+            max_keys: 1000,
+            is_truncated: false,
+            marker: "",
+            next_marker: None,
+            encoding_type: &None,
+        };
+
+        let xml = build_list_xml_v1(&params, &list_result, &config, None).unwrap();
+
+        assert!(
+            !xml.contains("<Key>staging-gov-data</Key>"),
+            "Directory marker at list prefix should be filtered out in V1: {xml}"
+        );
+        assert!(
+            xml.contains("<Key>staging-gov-data/data/file.parquet</Key>"),
+            "Real file should remain in V1: {xml}"
+        );
+    }
+
+    #[test]
+    fn test_nonzero_byte_object_matching_prefix_not_filtered() {
+        let config = make_config(None);
+        // An object with size > 0 that happens to match a common prefix
+        // should NOT be filtered — only 0-byte markers are filtered.
+        let list_result = make_list_result_with_sizes(&[("photos", 100)], &["photos"]);
+
+        let params = ListXmlParams {
+            bucket_name: "my-bucket",
+            client_prefix: "",
+            delimiter: "/",
+            max_keys: 1000,
+            is_truncated: false,
+            key_count: 1,
+            start_after: &None,
+            continuation_token: &None,
+            next_continuation_token: None,
+            encoding_type: &None,
+        };
+
+        let xml = build_list_xml(&params, &list_result, &config, None).unwrap();
+
+        assert!(
+            xml.contains("<Key>photos</Key>"),
+            "Non-zero-byte object should not be filtered: {xml}"
+        );
+    }
 }