[PATCH] Fix AppleDouble conversion in vfs_fruit

Ralph Böhme slow at samba.org
Tue Oct 9 08:43:46 UTC 2018


Hi!

Attached is a patch that fixes AppleDouble conversion in vfs_fruit.

https://bugzilla.samba.org/show_bug.cgi?id=13649

vfs_fruit has a conversion routine to migrate stuff from macOS created 
AppleDouble files created on shares without a stream module.

Unfortunately the conversion of the mandatory FinderInfo metadata got dependent 
on the conversion of AppleDouble embodied xattrs. As the latter is only 
optional, for many AppleDouble files we did not convert FinderInfo.

The patchset is mainly a refactoring of ad_convert() in small digestible pieces, 
the fix is delivered with the last commit.

CI: https://gitlab.com/samba-team/devel/samba/pipelines/32377745

Manual testing: make test TESTS=vfs.fruit

Please review & push if happy. Thanks!

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46
-------------- next part --------------
From e46529da5cfacc9032f10b6c6bea67028d904dd7 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 7 Oct 2018 18:26:47 +0200
Subject: [PATCH 01/21] s4:torture: FinderInfo conversion test with AppleDouble
 without xattr data

This testcase demonstrates that the AppleDouble conversion in vfs_fruit
doesn't correctly convert the FinderInfo data from the AppleDouble file
to a stream.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 selftest/knownfail.d/samba3.vfs.fruit |   3 +
 source4/torture/vfs/fruit.c           | 258 ++++++++++++++++++++++++++++++++++
 2 files changed, 261 insertions(+)

diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
index 8df25bccb79..bc46c2f4922 100644
--- a/selftest/knownfail.d/samba3.vfs.fruit
+++ b/selftest/knownfail.d/samba3.vfs.fruit
@@ -1 +1,4 @@
 ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion\(nt4_dc\)
+^samba3.vfs.fruit metadata_netatalk.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
+^samba3.vfs.fruit metadata_stream.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
+^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
index a69ab7e5350..8d12a4c16e5 100644
--- a/source4/torture/vfs/fruit.c
+++ b/source4/torture/vfs/fruit.c
@@ -886,6 +886,147 @@ static char osx_adouble_w_xattr[] = {
 	0x00, 0x00, 0x00, 0x1c, 0x00, 0x1e, 0xff, 0xff
 };
 
+/*
+ * The buf below contains the following AppleDouble encoded data:
+ *
+ * -------------------------------------------------------------------------------
+ * MagicNumber: 00051607                                        : AppleDouble
+ * Version    : 00020000                                        : Version 2
+ * Filler     : 4D 61 63 20 4F 53 20 58 20 20 20 20 20 20 20 20 : Mac OS X
+ * Num. of ent: 0002                                            : 2
+ *
+ * -------------------------------------------------------------------------------
+ * Entry ID   : 00000002 : Resource Fork
+ * Offset     : 00000052 : 82
+ * Length     : 0000011E : 286
+ *
+ * -RAW DUMP--:  0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F : (ASCII)
+ * 00000000   : 00 00 01 00 00 00 01 00 00 00 00 00 00 00 00 1E : ................
+ * 00000010   : 54 68 69 73 20 72 65 73 6F 75 72 63 65 20 66 6F : This resource fo
+ * 00000020   : 72 6B 20 69 6E 74 65 6E 74 69 6F 6E 61 6C 6C 79 : rk intentionally
+ * 00000030   : 20 6C 65 66 74 20 62 6C 61 6E 6B 20 20 20 00 00 :  left blank   ..
+ * 00000040   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000050   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000060   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000070   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000080   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000090   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000A0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000B0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000C0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000D0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000E0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000F0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000100   : 00 00 01 00 00 00 01 00 00 00 00 00 00 00 00 1E : ................
+ * 00000110   : 00 00 00 00 00 00 00 00 00 1C 00 1E FF FF       : ..............
+ *
+ * Entry ID   : 00000009 : Finder Info
+ * Offset     : 00000032 : 50
+ * Length     : 00000020 : 32
+ *
+ * -NOTE------: cannot detect whether FInfo or DInfo. assume FInfo.
+ *
+ * -FInfo-----:
+ * Type       : 57415645 : WAVE
+ * Creator    : 5054756C : PTul
+ * isAlias    : 0
+ * Invisible  : 0
+ * hasBundle  : 0
+ * nameLocked : 0
+ * Stationery : 0
+ * CustomIcon : 0
+ * Reserved   : 0
+ * Inited     : 0
+ * NoINITS    : 0
+ * Shared     : 0
+ * SwitchLaunc: 0
+ * Hidden Ext : 0
+ * color      : 000      : none
+ * isOnDesk   : 0
+ * Location v : 0000     : 0
+ * Location h : 0000     : 0
+ * Fldr       : 0000     : ..
+ *
+ * -FXInfo----:
+ * Rsvd|IconID: 0000     : 0
+ * Rsvd       : 0000     : ..
+ * Rsvd       : 0000     : ..
+ * Rsvd       : 0000     : ..
+ * AreInvalid : 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * CustomBadge: 0
+ * ObjctIsBusy: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * RoutingInfo: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * Rsvd|commnt: 0000     : 0
+ * PutAway    : 00000000 : 0
+ *
+ * -RAW DUMP--:  0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F : (ASCII)
+ * 00000000   : 57 41 56 45 50 54 75 6C 00 00 00 00 00 00 00 00 : WAVEPTul........
+ * 00000010   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ *  *
+ * It was created with:
+ * $ hexdump -ve '"\t" 7/1 "0x%02x, " 1/1 " 0x%02x," "\n"'
+ */
+static char osx_adouble_without_xattr[] = {
+	0x00, 0x05, 0x16, 0x07, 0x00, 0x02, 0x00, 0x00,
+	0x4d, 0x61, 0x63, 0x20, 0x4f, 0x53, 0x20, 0x58,
+	0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+	0x00, 0x02, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00,
+	0x00, 0x52, 0x00, 0x00, 0x01, 0x1e, 0x00, 0x00,
+	0x00, 0x09, 0x00, 0x00, 0x00, 0x32, 0x00, 0x00,
+	0x00, 0x20, 0x57, 0x41, 0x56, 0x45, 0x50, 0x54,
+	0x75, 0x6c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+	0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x1e, 0x54, 0x68, 0x69, 0x73, 0x20, 0x72,
+	0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x20,
+	0x66, 0x6f, 0x72, 0x6b, 0x20, 0x69, 0x6e, 0x74,
+	0x65, 0x6e, 0x74, 0x69, 0x6f, 0x6e, 0x61, 0x6c,
+	0x6c, 0x79, 0x20, 0x6c, 0x65, 0x66, 0x74, 0x20,
+	0x62, 0x6c, 0x61, 0x6e, 0x6b, 0x20, 0x20, 0x20,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+	0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x1e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x1c, 0x00, 0x1e, 0xff, 0xff
+};
+
 /**
  * talloc and intialize an AfpInfo
  **/
@@ -2021,6 +2162,122 @@ static bool test_adouble_conversion(struct torture_context *tctx,
 	return ret;
 }
 
+/*
+ * Test conversion of AppleDouble file without embedded xattr data
+ */
+static bool test_adouble_conversion_wo_xattr(struct torture_context *tctx,
+					     struct smb2_tree *tree)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
+	const char *fname = BASEDIR "\\test_adouble_conversion";
+	const char *adname = BASEDIR "/._test_adouble_conversion";
+	NTSTATUS status;
+	struct smb2_handle testdirh;
+	bool ret = true;
+	const char *streams[] = {
+		"::$DATA",
+		AFPINFO_STREAM,
+		AFPRESOURCE_STREAM
+	};
+	struct smb2_create create;
+	struct smb2_find find;
+	unsigned int count;
+	union smb_search_data *d;
+	const char *data = "This resource fork intentionally left blank";
+	size_t datalen = strlen(data);
+
+	smb2_deltree(tree, BASEDIR);
+
+	status = torture_smb2_testdir(tree, BASEDIR, &testdirh);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"torture_smb2_testdir failed\n");
+	smb2_util_close(tree, testdirh);
+
+	ret = torture_setup_file(tctx, tree, fname, false);
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "torture_setup_file failed\n");
+
+	ret = torture_setup_file(tctx, tree, adname, false);
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "torture_setup_file failed\n");
+
+	ret = write_stream(tree, __location__, tctx, mem_ctx,
+			   adname, NULL, 0,
+			   sizeof(osx_adouble_without_xattr),
+			   osx_adouble_without_xattr);
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "write_stream failed\n");
+
+	ret = enable_aapl(tctx, tree);
+	torture_assert_goto(tctx, ret == true, ret, done, "enable_aapl failed");
+
+	/*
+	 * Issue a smb2_find(), this triggers the server-side conversion
+	 */
+
+	create = (struct smb2_create) {
+		.in.desired_access = SEC_RIGHTS_DIR_READ,
+		.in.create_options = NTCREATEX_OPTIONS_DIRECTORY,
+		.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY,
+		.in.share_access = NTCREATEX_SHARE_ACCESS_READ,
+		.in.create_disposition = NTCREATEX_DISP_OPEN,
+		.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS,
+		.in.fname = BASEDIR,
+	};
+
+	status = smb2_create(tree, tctx, &create);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_create failed\n");
+
+	find = (struct smb2_find) {
+		.in.file.handle = create.out.file.handle,
+		.in.pattern = "*",
+		.in.max_response_size = 0x1000,
+		.in.level = SMB2_FIND_ID_BOTH_DIRECTORY_INFO,
+	};
+
+	status = smb2_find_level(tree, tree, &find, &count, &d);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_find_level failed\n");
+
+	status = smb2_util_close(tree, create.out.file.handle);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_util_close failed");
+
+	/*
+	 * Check number of streams
+	 */
+
+	ret = check_stream_list(tree, tctx, fname, 3, streams, false);
+	torture_assert_goto(tctx, ret == true, ret, done, "check_stream_list");
+
+
+	/*
+	 * Check Resourcefork data can be read.
+	 */
+
+	ret = check_stream(tree, __location__, tctx, mem_ctx,
+			   fname, AFPRESOURCE_STREAM,
+			   16, datalen, 0, datalen, data);
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "check AFPRESOURCE_STREAM failed\n");
+
+	/*
+	 * Check FinderInfo data has been migrated to stream.
+	 */
+
+	ret = check_stream(tree, __location__, tctx, mem_ctx,
+			   fname, AFPINFO_STREAM,
+			   0, 60, 16, 8, "WAVEPTul");
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "check AFPINFO_STREAM failed\n");
+
+done:
+	smb2_deltree(tree, BASEDIR);
+	talloc_free(mem_ctx);
+	return ret;
+}
+
 static bool test_aapl(struct torture_context *tctx,
 		      struct smb2_tree *tree)
 {
@@ -4866,6 +5123,7 @@ struct torture_suite *torture_vfs_fruit(TALLOC_CTX *ctx)
 	torture_suite_add_1smb2_test(suite, "copy-chunk streams", test_copy_chunk_streams);
 	torture_suite_add_1smb2_test(suite, "OS X AppleDouble file conversion", test_adouble_conversion);
 	torture_suite_add_1smb2_test(suite, "NFS ACE entries", test_nfs_aces);
+	torture_suite_add_1smb2_test(suite, "OS X AppleDouble file conversion without embedded xattr", test_adouble_conversion_wo_xattr);
 
 	return suite;
 }
-- 
2.13.6


From e830d0d0947432ce3f9b4c2bd1532004b77495b5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 11 Sep 2018 14:05:43 +0200
Subject: [PATCH 02/21] vfs_fruit: fix two comments

Thanks to the recent addition of ad_convert_xattr() we now correctly
handle this case.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 894c361a05f..65d6618ff2d 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1055,8 +1055,7 @@ static bool ad_convert_xattr(struct adouble *ad,
  * Convert from Apple's ._ file to Netatalk
  *
  * Apple's AppleDouble may contain a FinderInfo entry longer then 32
- * bytes containing packed xattrs. Netatalk can't deal with that, so
- * we simply discard the packed xattrs.
+ * bytes containing packed xattrs.
  *
  * @return -1 in case an error occurred, 0 if no conversion was done, 1
  * otherwise
@@ -1374,9 +1373,7 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 
 	/*
 	 * Try to fixup AppleDouble files created by OS X with xattrs
-	 * appended to the ADEID_FINDERI entry. We simply remove the
-	 * xattrs blob, this means any fancy xattr that was stored
-	 * there is lost.
+	 * appended to the ADEID_FINDERI entry.
 	 */
 
 	ret = ad_convert(ad, smb_fname, ad->ad_fd);
-- 
2.13.6


From 89a66db0754041dd54beb9c309eafa4ce584ab7b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 15:12:44 +0200
Subject: [PATCH 03/21] vfs_fruit: add a filler bytes pointer to struct adouble

This can later be used to distinguish between macOS created AppleDouble
files and AppleDouble files created by Samba or Netatalk.

macOS:    "Mac OS X        "
Samba:    "Netatalk        "
Netatalk: "Netatalk        "

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 65d6618ff2d..4819d37a4d9 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -262,6 +262,7 @@ typedef enum {ADOUBLE_META, ADOUBLE_RSRC} adouble_type_t;
 #define ADEDLEN_VERSION     4
 #define ADEDLEN_FILLER      16
 #define AD_FILLER_TAG       "Netatalk        " /* should be 16 bytes */
+#define AD_FILLER_TAG_OSX   "Mac OS X        " /* should be 16 bytes */
 #define ADEDLEN_NENTRIES    2
 #define AD_HEADER_LEN       (ADEDLEN_MAGIC + ADEDLEN_VERSION + \
 			     ADEDLEN_FILLER + ADEDLEN_NENTRIES) /* 26 */
@@ -414,6 +415,7 @@ struct adouble {
 	adouble_type_t            ad_type;
 	uint32_t                  ad_magic;
 	uint32_t                  ad_version;
+	uint8_t                   *ad_filler;
 	struct ad_entry           ad_eid[ADEID_MAX];
 	char                     *ad_data;
 	struct ad_xattr_header    adx_header;
@@ -837,6 +839,8 @@ static bool ad_unpack(struct adouble *ad, const size_t nentries,
 		return false;
 	}
 
+	ad->ad_filler = (uint8_t *)ad->ad_data + ADEDOFF_FILLER;
+
 	adentries = RSVAL(ad->ad_data, ADEDOFF_NENTRIES);
 	if (adentries != nentries) {
 		DEBUG(1, ("invalid number of entries: %zu\n",
-- 
2.13.6


From 935715c7d5590d4ff410fcc7a0c90b483bb28b9d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 2 Oct 2018 14:51:05 +0200
Subject: [PATCH 04/21] vfs_fruit: move setting ADEID_FINDERI length to
 ad_convert_xattr()

ad_convert_xattr() does the conversion of the xattr data in the
AppleDouble file, so we should update it's size there and should not
defer it to the caller.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 4819d37a4d9..3f3c20ac9e8 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1052,6 +1052,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 		fsp = NULL;
 	}
 
+	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
 	return true;
 }
 
@@ -1096,7 +1097,6 @@ static int ad_convert(struct adouble *ad,
 			ad_getentrylen(ad, ADEID_RFORK));
 	}
 
-	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
 	ad_setentryoff(ad, ADEID_RFORK,
 		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
 
-- 
2.13.6


From bd4ac1f0822e7df64fb13b9f6d143c9cdb7fd564 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 4 Oct 2018 08:23:59 +0200
Subject: [PATCH 05/21] vfs_fruit: do direct return from error checks in
 ad_convert()

Subsequent commits will move the mmap() into the subfunctions. This
change just prepares for that.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 3f3c20ac9e8..18e0a6c3fdd 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1081,8 +1081,7 @@ static int ad_convert(struct adouble *ad,
 	map = mmap(NULL, origlen, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
 	if (map == MAP_FAILED) {
 		DEBUG(2, ("mmap AppleDouble: %s\n", strerror(errno)));
-		rc = -1;
-		goto exit;
+		return -1;
 	}
 
 	ok = ad_convert_xattr(ad, smb_fname, map);
@@ -1106,12 +1105,18 @@ static int ad_convert(struct adouble *ad,
 	 */
 	rc = ftruncate(fd, ad_getentryoff(ad, ADEID_RFORK)
 		       + ad_getentrylen(ad, ADEID_RFORK));
-
-exit:
-	if (map != MAP_FAILED) {
+	if (rc != 0) {
 		munmap(map, origlen);
+		return -1;
 	}
-	return rc;
+
+	rc = munmap(map, origlen);
+	if (rc != 0) {
+		DBG_ERR("munmap failed: %s\n", strerror(errno));
+		return -1;
+	}
+
+	return 0;
 }
 
 /**
-- 
2.13.6


From bb8da935f828e6c1ac66d0a67d36f519b6f449a0 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 4 Oct 2018 08:51:28 +0200
Subject: [PATCH 06/21] vfs_fruit: remove unneeded fd argument from
 ad_convert()

Use the struct adouble member ad_fd instead of passing it as an
argument. Who did that in the first place? :)

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 18e0a6c3fdd..a7a6233784a 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1066,8 +1066,7 @@ static bool ad_convert_xattr(struct adouble *ad,
  * otherwise
  **/
 static int ad_convert(struct adouble *ad,
-		      const struct smb_filename *smb_fname,
-		      int fd)
+		      const struct smb_filename *smb_fname)
 {
 	int rc = 0;
 	char *map = MAP_FAILED;
@@ -1078,7 +1077,8 @@ static int ad_convert(struct adouble *ad,
 		ad_getentrylen(ad, ADEID_RFORK);
 
 	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
-	map = mmap(NULL, origlen, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+	map = mmap(NULL, origlen, PROT_READ|PROT_WRITE, MAP_SHARED,
+		   ad->ad_fd, 0);
 	if (map == MAP_FAILED) {
 		DEBUG(2, ("mmap AppleDouble: %s\n", strerror(errno)));
 		return -1;
@@ -1103,7 +1103,7 @@ static int ad_convert(struct adouble *ad,
 	 * FIXME: direct ftruncate(), but we don't have a fsp for the
 	 * VFS call
 	 */
-	rc = ftruncate(fd, ad_getentryoff(ad, ADEID_RFORK)
+	rc = ftruncate(ad->ad_fd, ad_getentryoff(ad, ADEID_RFORK)
 		       + ad_getentrylen(ad, ADEID_RFORK));
 	if (rc != 0) {
 		munmap(map, origlen);
@@ -1385,7 +1385,7 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 	 * appended to the ADEID_FINDERI entry.
 	 */
 
-	ret = ad_convert(ad, smb_fname, ad->ad_fd);
+	ret = ad_convert(ad, smb_fname);
 	if (ret != 0) {
 		DBG_WARNING("Failed to convert [%s]\n", smb_fname->base_name);
 		return len;
-- 
2.13.6


From 5e8019e4d18346ba84fc8ce6280ff403ce6b766f Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:14:40 +0200
Subject: [PATCH 07/21] vfs_fruit: move storing of modified struct adouble to
 ad_convert()

ad_convert() modified it, so let ad_convert() also save it to disk. No
change in behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index a7a6233784a..8a260b37e51 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1071,6 +1071,7 @@ static int ad_convert(struct adouble *ad,
 	int rc = 0;
 	char *map = MAP_FAILED;
 	size_t origlen;
+	ssize_t len;
 	bool ok;
 
 	origlen = ad_getentryoff(ad, ADEID_RFORK) +
@@ -1116,6 +1117,18 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
+	ok = ad_pack(ad);
+	if (!ok) {
+		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
+		return -1;
+	}
+
+	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
+	if (len != AD_DATASZ_DOT_UND) {
+		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -1391,18 +1404,6 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 		return len;
 	}
 
-	ok = ad_pack(ad);
-	if (!ok) {
-		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
-		return -1;
-	}
-
-	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
-	if (len != AD_DATASZ_DOT_UND) {
-		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
-		return -1;
-	}
-
 	p_ad = ad_get_entry(ad, ADEID_FINDERI);
 	if (p_ad == NULL) {
 		return -1;
-- 
2.13.6


From aafc55566f018a0fc5fc5bfa718c8c6f75bd9bf4 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:25:27 +0200
Subject: [PATCH 08/21] vfs_fruit: move FinderInfo conversion to helper
 function and call it from ad_convert()

No change in behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 192 ++++++++++++++++++++++++--------------------
 1 file changed, 104 insertions(+), 88 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 8a260b37e51..573e62635df 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1056,6 +1056,102 @@ static bool ad_convert_xattr(struct adouble *ad,
 	return true;
 }
 
+static bool ad_convert_finderinfo(struct adouble *ad,
+				  const struct smb_filename *smb_fname)
+{
+	char *p_ad = NULL;
+	AfpInfo *ai = NULL;
+	DATA_BLOB aiblob;
+	struct smb_filename *stream_name = NULL;
+	files_struct *fsp = NULL;
+	size_t size;
+	ssize_t nwritten;
+	NTSTATUS status;
+	int saved_errno = 0;
+
+	p_ad = ad_get_entry(ad, ADEID_FINDERI);
+	if (p_ad == NULL) {
+		return false;
+	}
+
+	ai = afpinfo_new(talloc_tos());
+	if (ai == NULL) {
+		return false;
+	}
+
+	memcpy(ai->afpi_FinderInfo, p_ad, ADEDLEN_FINDERI);
+
+	aiblob = data_blob_talloc(talloc_tos(), NULL, AFP_INFO_SIZE);
+	if (aiblob.data == NULL) {
+		TALLOC_FREE(ai);
+		return false;
+	}
+
+	size = afpinfo_pack(ai, (char *)aiblob.data);
+	TALLOC_FREE(ai);
+	if (size != AFP_INFO_SIZE) {
+		return false;
+	}
+
+	stream_name = synthetic_smb_fname(talloc_tos(),
+					  smb_fname->base_name,
+					  AFPINFO_STREAM,
+					  NULL,
+					  smb_fname->flags);
+	if (stream_name == NULL) {
+		data_blob_free(&aiblob);
+		DBG_ERR("synthetic_smb_fname failed\n");
+		return false;
+	}
+
+	DBG_DEBUG("stream_name: %s\n", smb_fname_str_dbg(stream_name));
+
+	status = SMB_VFS_CREATE_FILE(
+		ad->ad_handle->conn,		/* conn */
+		NULL,				/* req */
+		0,				/* root_dir_fid */
+		stream_name,			/* fname */
+		FILE_GENERIC_WRITE,		/* access_mask */
+		FILE_SHARE_READ | FILE_SHARE_WRITE, /* share_access */
+		FILE_OPEN_IF,			/* create_disposition */
+		0,				/* create_options */
+		0,				/* file_attributes */
+		INTERNAL_OPEN_ONLY,		/* oplock_request */
+		NULL,				/* lease */
+		0,				/* allocation_size */
+		0,				/* private_flags */
+		NULL,				/* sd */
+		NULL,				/* ea_list */
+		&fsp,				/* result */
+		NULL,				/* psbuf */
+		NULL, NULL);			/* create context */
+	TALLOC_FREE(stream_name);
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
+		return false;
+	}
+
+	nwritten = SMB_VFS_PWRITE(fsp,
+				  aiblob.data,
+				  aiblob.length,
+				  0);
+	if (nwritten == -1) {
+		DBG_ERR("SMB_VFS_PWRITE failed\n");
+		saved_errno = errno;
+		close_file(NULL, fsp, ERROR_CLOSE);
+		errno = saved_errno;
+		return false;
+	}
+
+	status = close_file(NULL, fsp, NORMAL_CLOSE);
+	if (!NT_STATUS_IS_OK(status)) {
+		return false;
+	}
+	fsp = NULL;
+
+	return true;
+}
+
 /**
  * Convert from Apple's ._ file to Netatalk
  *
@@ -1129,6 +1225,13 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
+	ok = ad_convert_finderinfo(ad, smb_fname);
+	if (!ok) {
+		DBG_ERR("Failed to convert [%s]\n",
+			smb_fname_str_dbg(smb_fname));
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -1325,15 +1428,8 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 {
 	SMB_STRUCT_STAT sbuf;
 	char *p_ad = NULL;
-	AfpInfo *ai = NULL;
-	DATA_BLOB aiblob;
-	struct smb_filename *stream_name = NULL;
-	files_struct *fsp = NULL;
-	ssize_t len;
 	size_t size;
-	ssize_t nwritten;
-	NTSTATUS status;
-	int saved_errno = 0;
+	ssize_t len;
 	int ret;
 	bool ok;
 
@@ -1404,86 +1500,6 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 		return len;
 	}
 
-	p_ad = ad_get_entry(ad, ADEID_FINDERI);
-	if (p_ad == NULL) {
-		return -1;
-	}
-
-	ai = afpinfo_new(talloc_tos());
-	if (ai == NULL) {
-		return -1;
-	}
-
-	memcpy(ai->afpi_FinderInfo, p_ad, ADEDLEN_FINDERI);
-
-	aiblob = data_blob_talloc(talloc_tos(), NULL, AFP_INFO_SIZE);
-	if (aiblob.data == NULL) {
-		TALLOC_FREE(ai);
-		return -1;
-	}
-
-	size = afpinfo_pack(ai, (char *)aiblob.data);
-	TALLOC_FREE(ai);
-	if (size != AFP_INFO_SIZE) {
-		return -1;
-	}
-
-	stream_name = synthetic_smb_fname(talloc_tos(),
-					  smb_fname->base_name,
-					  AFPINFO_STREAM,
-					  NULL,
-					  smb_fname->flags);
-	if (stream_name == NULL) {
-		data_blob_free(&aiblob);
-		DBG_ERR("synthetic_smb_fname failed\n");
-		return -1;
-	}
-
-	DBG_DEBUG("stream_name: %s\n", smb_fname_str_dbg(stream_name));
-
-	status = SMB_VFS_CREATE_FILE(
-		ad->ad_handle->conn,		/* conn */
-		NULL,				/* req */
-		0,				/* root_dir_fid */
-		stream_name,			/* fname */
-		FILE_GENERIC_WRITE,		/* access_mask */
-		FILE_SHARE_READ | FILE_SHARE_WRITE, /* share_access */
-		FILE_OPEN_IF,			/* create_disposition */
-		0,				/* create_options */
-		0,				/* file_attributes */
-		INTERNAL_OPEN_ONLY,		/* oplock_request */
-		NULL,				/* lease */
-		0,				/* allocation_size */
-		0,				/* private_flags */
-		NULL,				/* sd */
-		NULL,				/* ea_list */
-		&fsp,				/* result */
-		NULL,				/* psbuf */
-		NULL, NULL);			/* create context */
-	TALLOC_FREE(stream_name);
-	if (!NT_STATUS_IS_OK(status)) {
-		DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
-		return -1;
-	}
-
-	nwritten = SMB_VFS_PWRITE(fsp,
-				  aiblob.data,
-				  aiblob.length,
-				  0);
-	if (nwritten == -1) {
-		DBG_ERR("SMB_VFS_PWRITE failed\n");
-		saved_errno = errno;
-		close_file(NULL, fsp, ERROR_CLOSE);
-		errno = saved_errno;
-		return -1;
-	}
-
-	status = close_file(NULL, fsp, NORMAL_CLOSE);
-	if (!NT_STATUS_IS_OK(status)) {
-		return -1;
-	}
-	fsp = NULL;
-
 	return len;
 }
 
-- 
2.13.6


From 10f44a7e6c3733bc13219337ea2c391934e6ef34 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:26:46 +0200
Subject: [PATCH 09/21] vfs_fruit: move FinderInfo lenght check to ad_convert()

The final step in consolidating all conversion related work in
ad_convert(). No change in behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 573e62635df..e2cd61ed9cd 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1170,6 +1170,10 @@ static int ad_convert(struct adouble *ad,
 	ssize_t len;
 	bool ok;
 
+	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
+		return 0;
+	}
+
 	origlen = ad_getentryoff(ad, ADEID_RFORK) +
 		ad_getentrylen(ad, ADEID_RFORK);
 
@@ -1485,10 +1489,6 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 		return -1;
 	}
 
-	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
-		return len;
-	}
-
 	/*
 	 * Try to fixup AppleDouble files created by OS X with xattrs
 	 * appended to the ADEID_FINDERI entry.
-- 
2.13.6


From 18eef4495b2684041682f5b783748bf7ee4567cf Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 19:13:16 +0200
Subject: [PATCH 10/21] vfs_fruit: split out truncating from ad_convert()

This may look a little ill-advised as this increases line count, but
the goal here is modularizing ad_convert() itself and making it as slick
as possible helps achieving that goal.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index e2cd61ed9cd..abe9b028f41 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1152,6 +1152,24 @@ static bool ad_convert_finderinfo(struct adouble *ad,
 	return true;
 }
 
+static bool ad_convert_truncate(struct adouble *ad,
+				const struct smb_filename *smb_fname)
+{
+	int rc;
+
+	/*
+	 * FIXME: direct ftruncate(), but we don't have a fsp for the
+	 * VFS call
+	 */
+	rc = ftruncate(ad->ad_fd, ad_getentryoff(ad, ADEID_RFORK)
+		       + ad_getentrylen(ad, ADEID_RFORK));
+	if (rc != 0) {
+		return false;
+	}
+
+	return true;
+}
+
 /**
  * Convert from Apple's ._ file to Netatalk
  *
@@ -1200,13 +1218,8 @@ static int ad_convert(struct adouble *ad,
 	ad_setentryoff(ad, ADEID_RFORK,
 		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
 
-	/*
-	 * FIXME: direct ftruncate(), but we don't have a fsp for the
-	 * VFS call
-	 */
-	rc = ftruncate(ad->ad_fd, ad_getentryoff(ad, ADEID_RFORK)
-		       + ad_getentrylen(ad, ADEID_RFORK));
-	if (rc != 0) {
+	ok = ad_convert_truncate(ad, smb_fname);
+	if (!ok) {
 		munmap(map, origlen);
 		return -1;
 	}
-- 
2.13.6


From c9a08b8f46de851321a8b7a958cd24dcb487107b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 19:15:04 +0200
Subject: [PATCH 11/21] vfs_fruit: use ADEDOFF_RFORK_DOT_UND offset macro in
 ad_convert_truncate()

We really want the fixed size offset here, not a calculated one. Note
that "ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI" is equal to
ADEDOFF_RFORK_DOT_UND.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index abe9b028f41..5766e777a57 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1161,8 +1161,8 @@ static bool ad_convert_truncate(struct adouble *ad,
 	 * FIXME: direct ftruncate(), but we don't have a fsp for the
 	 * VFS call
 	 */
-	rc = ftruncate(ad->ad_fd, ad_getentryoff(ad, ADEID_RFORK)
-		       + ad_getentrylen(ad, ADEID_RFORK));
+	rc = ftruncate(ad->ad_fd, ADEDOFF_RFORK_DOT_UND +
+		       ad_getentrylen(ad, ADEID_RFORK));
 	if (rc != 0) {
 		return false;
 	}
-- 
2.13.6


From 5bfb4884692cdbe915b4fd9dbfdaf76a38b940e3 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:44:53 +0200
Subject: [PATCH 12/21] vfs_fruit: split out moving of the resource fork

No change in behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 5766e777a57..263fa3e6e6c 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1170,6 +1170,24 @@ static bool ad_convert_truncate(struct adouble *ad,
 	return true;
 }
 
+static bool ad_convert_move_reso(struct adouble *ad,
+				 const struct smb_filename *smb_fname,
+				 char *map)
+{
+	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
+		return true;
+	}
+
+	memmove(map + ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI,
+		map + ad_getentryoff(ad, ADEID_RFORK),
+		ad_getentrylen(ad, ADEID_RFORK));
+
+	ad_setentryoff(ad, ADEID_RFORK,
+		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
+
+	return true;
+}
+
 /**
  * Convert from Apple's ._ file to Netatalk
  *
@@ -1209,15 +1227,12 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
-	if (ad_getentrylen(ad, ADEID_RFORK) > 0) {
-		memmove(map + ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI,
-			map + ad_getentryoff(ad, ADEID_RFORK),
-			ad_getentrylen(ad, ADEID_RFORK));
+	ok = ad_convert_move_reso(ad, smb_fname, map);
+	if (!ok) {
+		munmap(map, origlen);
+		return -1;
 	}
 
-	ad_setentryoff(ad, ADEID_RFORK,
-		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
-
 	ok = ad_convert_truncate(ad, smb_fname);
 	if (!ok) {
 		munmap(map, origlen);
-- 
2.13.6


From 88c2b79a62d7ff0a368806991c1065afe4fc0004 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 19:15:04 +0200
Subject: [PATCH 13/21] vfs_fruit: use ADEDOFF_RFORK_DOT_UND offset macro in
 ad_convert_move_reso()

We really want the fixed size offset here, not a calculated one. Note
that "ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI" is equal to
ADEDOFF_RFORK_DOT_UND.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 263fa3e6e6c..4098f3efcbc 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1178,12 +1178,11 @@ static bool ad_convert_move_reso(struct adouble *ad,
 		return true;
 	}
 
-	memmove(map + ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI,
+	memmove(map + ADEDOFF_RFORK_DOT_UND,
 		map + ad_getentryoff(ad, ADEID_RFORK),
 		ad_getentrylen(ad, ADEID_RFORK));
 
-	ad_setentryoff(ad, ADEID_RFORK,
-		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
+	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
 
 	return true;
 }
-- 
2.13.6


From cf681f3632d10ab88de5397c54934287ba7682be Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:52:32 +0200
Subject: [PATCH 14/21] vfs_fruit: fix error returns in ad_convert_xattr()

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 4098f3efcbc..b2afa8a9730 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -985,14 +985,14 @@ static bool ad_convert_xattr(struct adouble *ad,
 		    !NT_STATUS_EQUAL(status, NT_STATUS_NONE_MAPPED))
 		{
 			DBG_ERR("string_replace_allocate failed\n");
-			return -1;
+			return false;
 		}
 
 		tmp = mapped_name;
 		mapped_name = talloc_asprintf(talloc_tos(), ":%s", tmp);
 		TALLOC_FREE(tmp);
 		if (mapped_name == NULL) {
-			return -1;
+			return false;
 		}
 
 		stream_name = synthetic_smb_fname(talloc_tos(),
@@ -1003,7 +1003,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(mapped_name);
 		if (stream_name == NULL) {
 			DBG_ERR("synthetic_smb_fname failed\n");
-			return -1;
+			return false;
 		}
 
 		DBG_DEBUG("stream_name: %s\n", smb_fname_str_dbg(stream_name));
@@ -1030,7 +1030,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(stream_name);
 		if (!NT_STATUS_IS_OK(status)) {
 			DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
-			return -1;
+			return false;
 		}
 
 		nwritten = SMB_VFS_PWRITE(fsp,
@@ -1042,12 +1042,12 @@ static bool ad_convert_xattr(struct adouble *ad,
 			saved_errno = errno;
 			close_file(NULL, fsp, ERROR_CLOSE);
 			errno = saved_errno;
-			return -1;
+			return false;
 		}
 
 		status = close_file(NULL, fsp, NORMAL_CLOSE);
 		if (!NT_STATUS_IS_OK(status)) {
-			return -1;
+			return false;
 		}
 		fsp = NULL;
 	}
-- 
2.13.6


From 8dbe5ca7de873d63d0e6cd70200b774bd5b2edac Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:59:18 +0200
Subject: [PATCH 15/21] vfs_fruit: let the ad_convert_*() subfunctions mmap as
 needed

This may mean that we mmap twice when we convert an AppleDouble file,
but this is the only sane way to cleanly modularize ad_convert().

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 98 ++++++++++++++++++++++++++++-----------------
 1 file changed, 62 insertions(+), 36 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index b2afa8a9730..3b085664d98 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -943,13 +943,16 @@ static bool ad_unpack(struct adouble *ad, const size_t nentries,
 }
 
 static bool ad_convert_xattr(struct adouble *ad,
-			     const struct smb_filename *smb_fname,
-			     char *map)
+			     const struct smb_filename *smb_fname)
 {
 	static struct char_mappings **string_replace_cmaps = NULL;
+	char *map = MAP_FAILED;
+	size_t maplen;
 	uint16_t i;
 	int saved_errno = 0;
 	NTSTATUS status;
+	int rc;
+	bool ok;
 
 	if (ad->adx_header.adx_num_attrs == 0) {
 		return true;
@@ -967,6 +970,17 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(mappings);
 	}
 
+	maplen = ad_getentryoff(ad, ADEID_RFORK) +
+		ad_getentrylen(ad, ADEID_RFORK);
+
+	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
+	map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED,
+		   ad->ad_fd, 0);
+	if (map == MAP_FAILED) {
+		DBG_ERR("mmap AppleDouble: %s\n", strerror(errno));
+		return false;
+	}
+
 	for (i = 0; i < ad->adx_header.adx_num_attrs; i++) {
 		struct ad_xattr_entry *e = &ad->adx_entries[i];
 		char *mapped_name = NULL;
@@ -985,14 +999,16 @@ static bool ad_convert_xattr(struct adouble *ad,
 		    !NT_STATUS_EQUAL(status, NT_STATUS_NONE_MAPPED))
 		{
 			DBG_ERR("string_replace_allocate failed\n");
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		tmp = mapped_name;
 		mapped_name = talloc_asprintf(talloc_tos(), ":%s", tmp);
 		TALLOC_FREE(tmp);
 		if (mapped_name == NULL) {
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		stream_name = synthetic_smb_fname(talloc_tos(),
@@ -1003,7 +1019,8 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(mapped_name);
 		if (stream_name == NULL) {
 			DBG_ERR("synthetic_smb_fname failed\n");
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		DBG_DEBUG("stream_name: %s\n", smb_fname_str_dbg(stream_name));
@@ -1030,7 +1047,8 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(stream_name);
 		if (!NT_STATUS_IS_OK(status)) {
 			DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		nwritten = SMB_VFS_PWRITE(fsp,
@@ -1042,18 +1060,29 @@ static bool ad_convert_xattr(struct adouble *ad,
 			saved_errno = errno;
 			close_file(NULL, fsp, ERROR_CLOSE);
 			errno = saved_errno;
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		status = close_file(NULL, fsp, NORMAL_CLOSE);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			ok = false;
+			goto fail;
 		}
 		fsp = NULL;
 	}
 
 	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
-	return true;
+	ok = true;
+
+fail:
+	rc = munmap(map, maplen);
+	if (rc != 0) {
+		DBG_ERR("munmap failed: %s\n", strerror(errno));
+		return false;
+	}
+
+	return ok;
 }
 
 static bool ad_convert_finderinfo(struct adouble *ad,
@@ -1171,17 +1200,37 @@ static bool ad_convert_truncate(struct adouble *ad,
 }
 
 static bool ad_convert_move_reso(struct adouble *ad,
-				 const struct smb_filename *smb_fname,
-				 char *map)
+				 const struct smb_filename *smb_fname)
 {
+	char *map = MAP_FAILED;
+	size_t maplen;
+	int rc;
+
 	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
 		return true;
 	}
 
+	maplen = ad_getentryoff(ad, ADEID_RFORK) +
+		ad_getentrylen(ad, ADEID_RFORK);
+
+	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
+	map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED,
+		   ad->ad_fd, 0);
+	if (map == MAP_FAILED) {
+		DBG_ERR("mmap AppleDouble: %s\n", strerror(errno));
+		return false;
+	}
+
 	memmove(map + ADEDOFF_RFORK_DOT_UND,
 		map + ad_getentryoff(ad, ADEID_RFORK),
 		ad_getentrylen(ad, ADEID_RFORK));
 
+	rc = munmap(map, maplen);
+	if (rc != 0) {
+		DBG_ERR("munmap failed: %s\n", strerror(errno));
+		return false;
+	}
+
 	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
 
 	return true;
@@ -1199,9 +1248,6 @@ static bool ad_convert_move_reso(struct adouble *ad,
 static int ad_convert(struct adouble *ad,
 		      const struct smb_filename *smb_fname)
 {
-	int rc = 0;
-	char *map = MAP_FAILED;
-	size_t origlen;
 	ssize_t len;
 	bool ok;
 
@@ -1209,38 +1255,18 @@ static int ad_convert(struct adouble *ad,
 		return 0;
 	}
 
-	origlen = ad_getentryoff(ad, ADEID_RFORK) +
-		ad_getentrylen(ad, ADEID_RFORK);
-
-	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
-	map = mmap(NULL, origlen, PROT_READ|PROT_WRITE, MAP_SHARED,
-		   ad->ad_fd, 0);
-	if (map == MAP_FAILED) {
-		DEBUG(2, ("mmap AppleDouble: %s\n", strerror(errno)));
-		return -1;
-	}
-
-	ok = ad_convert_xattr(ad, smb_fname, map);
+	ok = ad_convert_xattr(ad, smb_fname);
 	if (!ok) {
-		munmap(map, origlen);
 		return -1;
 	}
 
-	ok = ad_convert_move_reso(ad, smb_fname, map);
+	ok = ad_convert_move_reso(ad, smb_fname);
 	if (!ok) {
-		munmap(map, origlen);
 		return -1;
 	}
 
 	ok = ad_convert_truncate(ad, smb_fname);
 	if (!ok) {
-		munmap(map, origlen);
-		return -1;
-	}
-
-	rc = munmap(map, origlen);
-	if (rc != 0) {
-		DBG_ERR("munmap failed: %s\n", strerror(errno));
 		return -1;
 	}
 
-- 
2.13.6


From 9441a263f65bb321e9df1d09c7bed2f025de1b29 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 17:07:45 +0200
Subject: [PATCH 16/21] vfs_fruit: let the ad_convert_*() subfunction update
 the on-disk AppleDoube header as needed

Another step in simplifying ad_convert() itself. It means that we may
write to disk twice, but is only ever done once per AppleDouble file.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 3b085664d98..bb8f67bd381 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -949,6 +949,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 	char *map = MAP_FAILED;
 	size_t maplen;
 	uint16_t i;
+	ssize_t len;
 	int saved_errno = 0;
 	NTSTATUS status;
 	int rc;
@@ -1073,6 +1074,20 @@ static bool ad_convert_xattr(struct adouble *ad,
 	}
 
 	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
+
+	ok = ad_pack(ad);
+	if (!ok) {
+		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
+		goto fail;
+	}
+
+	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
+	if (len != AD_DATASZ_DOT_UND) {
+		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
+		ok = false;
+		goto fail;
+	}
+
 	ok = true;
 
 fail:
@@ -1204,7 +1219,9 @@ static bool ad_convert_move_reso(struct adouble *ad,
 {
 	char *map = MAP_FAILED;
 	size_t maplen;
+	ssize_t len;
 	int rc;
+	bool ok;
 
 	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
 		return true;
@@ -1233,6 +1250,18 @@ static bool ad_convert_move_reso(struct adouble *ad,
 
 	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
 
+	ok = ad_pack(ad);
+	if (!ok) {
+		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
+		return false;
+	}
+
+	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
+	if (len != AD_DATASZ_DOT_UND) {
+		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
+		return false;
+	}
+
 	return true;
 }
 
@@ -1248,7 +1277,6 @@ static bool ad_convert_move_reso(struct adouble *ad,
 static int ad_convert(struct adouble *ad,
 		      const struct smb_filename *smb_fname)
 {
-	ssize_t len;
 	bool ok;
 
 	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
@@ -1270,18 +1298,6 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
-	ok = ad_pack(ad);
-	if (!ok) {
-		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
-		return -1;
-	}
-
-	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
-	if (len != AD_DATASZ_DOT_UND) {
-		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
-		return -1;
-	}
-
 	ok = ad_convert_finderinfo(ad, smb_fname);
 	if (!ok) {
 		DBG_ERR("Failed to convert [%s]\n",
-- 
2.13.6


From a0487ffc31cfeef4674dc67c01315b0e345598e8 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 22:05:43 +0200
Subject: [PATCH 17/21] vfs_fruit: call ad_convert_move_reso() from
 ad_convert_xattr()

ad_convert_xattr() is the place that triggers the need to move the
resource fork, so it should also call ad_convert_move_reso().

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 113 ++++++++++++++++++++++----------------------
 1 file changed, 57 insertions(+), 56 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index bb8f67bd381..75aab6a8207 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -942,6 +942,58 @@ static bool ad_unpack(struct adouble *ad, const size_t nentries,
 	return true;
 }
 
+static bool ad_convert_move_reso(struct adouble *ad,
+				 const struct smb_filename *smb_fname)
+{
+	char *map = MAP_FAILED;
+	size_t maplen;
+	ssize_t len;
+	int rc;
+	bool ok;
+
+	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
+		return true;
+	}
+
+	maplen = ad_getentryoff(ad, ADEID_RFORK) +
+		ad_getentrylen(ad, ADEID_RFORK);
+
+	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
+	map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED,
+		   ad->ad_fd, 0);
+	if (map == MAP_FAILED) {
+		DBG_ERR("mmap AppleDouble: %s\n", strerror(errno));
+		return false;
+	}
+
+
+	memmove(map + ADEDOFF_RFORK_DOT_UND,
+		map + ad_getentryoff(ad, ADEID_RFORK),
+		ad_getentrylen(ad, ADEID_RFORK));
+
+	rc = munmap(map, maplen);
+	if (rc != 0) {
+		DBG_ERR("munmap failed: %s\n", strerror(errno));
+		return false;
+	}
+
+	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
+
+	ok = ad_pack(ad);
+	if (!ok) {
+		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
+		return false;
+	}
+
+	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
+	if (len != AD_DATASZ_DOT_UND) {
+		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
+		return false;
+	}
+
+	return true;
+}
+
 static bool ad_convert_xattr(struct adouble *ad,
 			     const struct smb_filename *smb_fname)
 {
@@ -1088,6 +1140,11 @@ static bool ad_convert_xattr(struct adouble *ad,
 		goto fail;
 	}
 
+	ok = ad_convert_move_reso(ad, smb_fname);
+	if (!ok) {
+		goto fail;
+	}
+
 	ok = true;
 
 fail:
@@ -1214,57 +1271,6 @@ static bool ad_convert_truncate(struct adouble *ad,
 	return true;
 }
 
-static bool ad_convert_move_reso(struct adouble *ad,
-				 const struct smb_filename *smb_fname)
-{
-	char *map = MAP_FAILED;
-	size_t maplen;
-	ssize_t len;
-	int rc;
-	bool ok;
-
-	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
-		return true;
-	}
-
-	maplen = ad_getentryoff(ad, ADEID_RFORK) +
-		ad_getentrylen(ad, ADEID_RFORK);
-
-	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
-	map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED,
-		   ad->ad_fd, 0);
-	if (map == MAP_FAILED) {
-		DBG_ERR("mmap AppleDouble: %s\n", strerror(errno));
-		return false;
-	}
-
-	memmove(map + ADEDOFF_RFORK_DOT_UND,
-		map + ad_getentryoff(ad, ADEID_RFORK),
-		ad_getentrylen(ad, ADEID_RFORK));
-
-	rc = munmap(map, maplen);
-	if (rc != 0) {
-		DBG_ERR("munmap failed: %s\n", strerror(errno));
-		return false;
-	}
-
-	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
-
-	ok = ad_pack(ad);
-	if (!ok) {
-		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
-		return false;
-	}
-
-	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
-	if (len != AD_DATASZ_DOT_UND) {
-		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
-		return false;
-	}
-
-	return true;
-}
-
 /**
  * Convert from Apple's ._ file to Netatalk
  *
@@ -1288,11 +1294,6 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
-	ok = ad_convert_move_reso(ad, smb_fname);
-	if (!ok) {
-		return -1;
-	}
-
 	ok = ad_convert_truncate(ad, smb_fname);
 	if (!ok) {
 		return -1;
-- 
2.13.6


From bb0594bac1a1c54f8d0b19ecd6f6144fc31d5bc9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 8 Oct 2018 12:51:37 +0200
Subject: [PATCH 18/21] vfs_fruit: add check for OS X filler in FinderInfo
 conversion

This ensures that the function only acts on AppleDouble files created by
macOS and not AppleDouble files created by us that are already in the
correct format (only using the Resource Fork).

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 75aab6a8207..7eb14e2f836 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1169,6 +1169,12 @@ static bool ad_convert_finderinfo(struct adouble *ad,
 	ssize_t nwritten;
 	NTSTATUS status;
 	int saved_errno = 0;
+	int cmp;
+
+	cmp = memcmp(ad->ad_filler, AD_FILLER_TAG_OSX, ADEDLEN_FILLER);
+	if (cmp != 0) {
+		return true;
+	}
 
 	p_ad = ad_get_entry(ad, ADEID_FINDERI);
 	if (p_ad == NULL) {
-- 
2.13.6


From fd5aaae138b9bf59dd97aa91905bb274ef61f475 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 8 Oct 2018 18:43:51 +0200
Subject: [PATCH 19/21] vfs_fruit: add out arg "converted_xattr" to
 ad_convert_xattr

Used to let the caller know if a conversion has been done. Currently not
used in the caller, that comes next.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 7eb14e2f836..d179ef7c7c4 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -995,7 +995,8 @@ static bool ad_convert_move_reso(struct adouble *ad,
 }
 
 static bool ad_convert_xattr(struct adouble *ad,
-			     const struct smb_filename *smb_fname)
+			     const struct smb_filename *smb_fname,
+			     bool *converted_xattr)
 {
 	static struct char_mappings **string_replace_cmaps = NULL;
 	char *map = MAP_FAILED;
@@ -1007,6 +1008,8 @@ static bool ad_convert_xattr(struct adouble *ad,
 	int rc;
 	bool ok;
 
+	*converted_xattr = false;
+
 	if (ad->adx_header.adx_num_attrs == 0) {
 		return true;
 	}
@@ -1145,6 +1148,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 		goto fail;
 	}
 
+	*converted_xattr = true;
 	ok = true;
 
 fail:
@@ -1290,12 +1294,13 @@ static int ad_convert(struct adouble *ad,
 		      const struct smb_filename *smb_fname)
 {
 	bool ok;
+	bool converted_xattr;
 
 	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
 		return 0;
 	}
 
-	ok = ad_convert_xattr(ad, smb_fname);
+	ok = ad_convert_xattr(ad, smb_fname, &converted_xattr);
 	if (!ok) {
 		return -1;
 	}
-- 
2.13.6


From 2130a4fe51a63e6960216d25f8a434b1dc9fff69 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 8 Oct 2018 18:47:32 +0200
Subject: [PATCH 20/21] vfs_fruit: make call to ad_convert_truncate() optional

Call ad_convert_truncate() based on whether the previous call
ad_convert_xattr() returned converted_xattr=true.

Upcoming fixes for a different Samba bug (#13642) will hook into calling
ad_convert_truncate() in other cases, this also prepares for that.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index d179ef7c7c4..cbc334c3c4a 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1305,9 +1305,11 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
-	ok = ad_convert_truncate(ad, smb_fname);
-	if (!ok) {
-		return -1;
+	if (converted_xattr) {
+		ok = ad_convert_truncate(ad, smb_fname);
+		if (!ok) {
+			return -1;
+		}
 	}
 
 	ok = ad_convert_finderinfo(ad, smb_fname);
-- 
2.13.6


From fb96861e30b33405172c8905dde58a16a97595ba Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 9 Oct 2018 10:15:37 +0200
Subject: [PATCH 21/21] vfs_fruit: move check in ad_convert() to ad_convert_*()
 subfunctions

Currently the whole conversion is skipped if the FinderInfo entry in the
AppleDouble file is of the default size (ie not containing xattrs).

That also means we never converted FinderInfo from the AppleDouble file
to stream format. This change finally fixes this.

Note that this keeps failing with streams_depot, much like the existing
known-fail of "samba3.vfs.fruit streams_depot.OS X AppleDouble file
conversion". Fixing the conversion to work with vfs_streams_depot is a
task for another day.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 selftest/knownfail.d/samba3.vfs.fruit | 2 --
 source3/modules/vfs_fruit.c           | 8 ++++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
index bc46c2f4922..6307e2b3404 100644
--- a/selftest/knownfail.d/samba3.vfs.fruit
+++ b/selftest/knownfail.d/samba3.vfs.fruit
@@ -1,4 +1,2 @@
 ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
-^samba3.vfs.fruit metadata_stream.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
 ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index cbc334c3c4a..d3b8dfeabd2 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1010,6 +1010,10 @@ static bool ad_convert_xattr(struct adouble *ad,
 
 	*converted_xattr = false;
 
+	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
+		return true;
+	}
+
 	if (ad->adx_header.adx_num_attrs == 0) {
 		return true;
 	}
@@ -1296,10 +1300,6 @@ static int ad_convert(struct adouble *ad,
 	bool ok;
 	bool converted_xattr;
 
-	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
-		return 0;
-	}
-
 	ok = ad_convert_xattr(ad, smb_fname, &converted_xattr);
 	if (!ok) {
 		return -1;
-- 
2.13.6



More information about the samba-technical mailing list