diff --git a/src/libostree/ostree-core-private.h b/src/libostree/ostree-core-private.h index 283944b4a9..2996e11a55 100644 --- a/src/libostree/ostree-core-private.h +++ b/src/libostree/ostree-core-private.h @@ -90,6 +90,7 @@ void _ostree_gfileinfo_to_stbuf (GFileInfo *file_info, struct stat *out_stbuf); gboolean _ostree_gfileinfo_equal (GFileInfo *a, GFileInfo *b); gboolean _ostree_stbuf_equal (struct stat *stbuf_a, struct stat *stbuf_b); GFileInfo *_ostree_mode_uidgid_to_gfileinfo (mode_t mode, uid_t uid, gid_t gid); +GVariant *_ostree_canonicalize_xattrs (GVariant *xattrs); gboolean _ostree_validate_structureof_xattrs (GVariant *xattrs, GError **error); static inline void diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index e67c23f81b..3e462838f6 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -43,7 +43,6 @@ G_STATIC_ASSERT (OSTREE_REPO_MODE_BARE_USER_ONLY == 3); G_STATIC_ASSERT (OSTREE_REPO_MODE_BARE_SPLIT_XATTRS == 4); static GBytes *variant_to_lenprefixed_buffer (GVariant *variant); -static GVariant *canonicalize_xattrs (GVariant *xattrs); #define ALIGN_VALUE(this, boundary) \ ((((unsigned long)(this)) + (((unsigned long)(boundary)) - 1)) \ @@ -317,8 +316,8 @@ compare_xattrs (const void *a_pp, const void *b_pp) } // Sort xattrs by name -static GVariant * -canonicalize_xattrs (GVariant *xattrs) +GVariant * +_ostree_canonicalize_xattrs (GVariant *xattrs) { // We always need to provide data, so NULL is canonicalized to the empty array if (xattrs == NULL) @@ -364,7 +363,7 @@ _ostree_file_header_new (GFileInfo *file_info, GVariant *xattrs) symlink_target = ""; // We always sort the xattrs now to ensure everything is in normal/canonical form. - g_autoptr (GVariant) tmp_xattrs = canonicalize_xattrs (xattrs); + g_autoptr (GVariant) tmp_xattrs = _ostree_canonicalize_xattrs (xattrs); g_autoptr (GVariant) ret = g_variant_new ("(uuuus@a(ayay))", GUINT32_TO_BE (uid), GUINT32_TO_BE (gid), @@ -1153,7 +1152,7 @@ ostree_create_directory_metadata (GFileInfo *dir_info, GVariant *xattrs) GVariant *ret_metadata = NULL; // We always sort the xattrs now to ensure everything is in normal/canonical form. - g_autoptr (GVariant) tmp_xattrs = canonicalize_xattrs (xattrs); + g_autoptr (GVariant) tmp_xattrs = _ostree_canonicalize_xattrs (xattrs); ret_metadata = g_variant_new ( "(uuu@a(ayay))", GUINT32_TO_BE (g_file_info_get_attribute_uint32 (dir_info, "unix::uid")), @@ -2342,7 +2341,8 @@ _ostree_validate_structureof_xattrs (GVariant *xattrs, GError **error) if (cmp == 0) return glnx_throw (error, "Duplicate xattr name, index=%d", i); else if (cmp > 0) - return glnx_throw (error, "Incorrectly sorted xattr name, index=%d", i); + return glnx_throw (error, "Incorrectly sorted xattr name (prev=%s, cur=%s), index=%d", + previous, name, i); } previous = name; i++; diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 18b2562c8a..c5a47d32c8 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -139,9 +139,12 @@ gboolean _ostree_write_bareuser_metadata (int fd, guint32 uid, guint32 gid, guint32 mode, GVariant *xattrs, GError **error) { - if (xattrs != NULL && !_ostree_validate_structureof_xattrs (xattrs, error)) - return FALSE; - g_autoptr (GVariant) filemeta = create_file_metadata (uid, gid, mode, xattrs); + GLNX_AUTO_PREFIX_ERROR ("Writing bareuser metadata", error); + // Like we do elsewhere, ensure xattrs are in canonical form. We don't strictly need + // this because we don't actually provide this data directly as input to a checksum today, + // but it's good hygeine to do so. + g_autoptr (GVariant) tmp_xattrs = _ostree_canonicalize_xattrs (xattrs); + g_autoptr (GVariant) filemeta = create_file_metadata (uid, gid, mode, tmp_xattrs); if (TEMP_FAILURE_RETRY (fsetxattr (fd, "user.ostreemeta", (char *)g_variant_get_data (filemeta), g_variant_get_size (filemeta), 0)) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 2f4e836c65..4e2f47d0e7 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -4098,7 +4098,10 @@ _ostree_repo_load_file_bare (OstreeRepo *self, const char *checksum, int *out_fd g_autoptr (GVariant) metadata = g_variant_ref_sink ( g_variant_new_from_bytes (OSTREE_FILEMETA_GVARIANT_FORMAT, bytes, FALSE)); - ret_xattrs = filemeta_to_stat (&stbuf, metadata); + g_autoptr (GVariant) read_xattrs = filemeta_to_stat (&stbuf, metadata); + // Old versions of ostree may have written these xattrs in non-canonical form. + // In order to aid comparisons, let's canonicalize here. + ret_xattrs = _ostree_canonicalize_xattrs (read_xattrs); if (S_ISLNK (stbuf.st_mode)) { if (out_symlink) diff --git a/tests/basic-test.sh b/tests/basic-test.sh index c8f853f8d6..3f31c7ce2e 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -19,7 +19,7 @@ set -euo pipefail -echo "1..$((90 + ${extra_basic_tests:-0}))" +echo "1..$((91 + ${extra_basic_tests:-0}))" CHECKOUT_U_ARG="" CHECKOUT_H_ARGS="-H" @@ -443,6 +443,24 @@ if ! skip_one_without_user_xattrs; then echo "ok local clone checkout" fi +if test -z "${OSTREE_NO_XATTRS:-}"; then + cd ${test_tmpdir} + ${CMD_PREFIX} ostree --repo=repo checkout ${CHECKOUT_U_ARG} test2 test2-checkout + touch test2-checkout/testxattrs + # Intentionally heavily interspersed (not sorted) + for n in a z q e f n c r i a b x t k y; do + setfattr -n user.$n -v x test2-checkout/testxattrs + done + cat repo/config + $OSTREE commit ${COMMIT_ARGS} -b test2 --tree=dir=test2-checkout --consume + $OSTREE ls -X test2 /testxattrs > out.txt + assert_file_has_content_literal out.txt "(b'user.a', [0x78])" + assert_file_has_content_literal out.txt "(b'user.z', [0x78])" + echo "ok sorted xattrs" +else + echo "ok # SKIP no xattrs" +fi + $OSTREE checkout -U test2 checkout-user-test2 echo "ok user checkout"