diff mbox series

[v3,01/12] libe2p: Helpers for configuring the encoding superblock fields

Message ID 20181126221949.12172-2-krisman@collabora.com
State Accepted, archived
Headers show
Series Support encoding awareness and casefold | expand

Commit Message

Gabriel Krisman Bertazi Nov. 26, 2018, 10:19 p.m. UTC 
From: Gabriel Krisman Bertazi <krisman@collabora.co.uk>

Implement helper functions to convert the encoding name and specific
parameters requested by the user on the command line into the format
that is written to disk.

Changes since v2:
  - Rename defines to add EXT4_ prefix
  - Use unicode X.Y versioning scheme

Changes since v1:
  - Drop struct ext4_encoding_map name.
  - remove question mark in comment.
  - Reword 0x0 -> NULL
  - Prevent out of bound array access if requested invalid encoding

Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
---
 lib/e2p/Makefile.in  |  8 +++-
 lib/e2p/e2p.h        |  5 +++
 lib/e2p/encoding.c   | 97 ++++++++++++++++++++++++++++++++++++++++++++
 lib/ext2fs/ext2_fs.h |  5 +++
 4 files changed, 113 insertions(+), 2 deletions(-)
 create mode 100644 lib/e2p/encoding.c

Comments

Theodore Ts'o Nov. 30, 2018, 3:42 p.m. UTC | #1 
On Mon, Nov 26, 2018 at 05:19:38PM -0500, Gabriel Krisman Bertazi wrote:
> +	/* 0x1 */ {"utf8-10.0", (EXT4_UTF8_NORMALIZATION_TYPE_NFKD |
> +				 EXT4_UTF8_CASEFOLD_TYPE_NFKDCF)},

We're using 10.0 here even though the later in the patch we're
installing Unicode 11.0.  What if we just call this utf8-10+?  Unicode
releases new versions every six months these days, and so long as the
case fold rules don't change for any existing characters, but are only
added for new characters added to the new version of Unicode, it would
definitely be OK for strict mode.

Even in relaxed mode, if someone decided to use, say, Klingon
characters not recognized by the Unicode consortium in their system,
and later on the Unicode consortium reassigns those code points to
some obscure ancient script, it would be unfortunate, how much would
it be *our* problem?  The worst that could happen is that if case
folding were enabled, two file names that were previously unique would
be considered identical by the new case folding rules after the
rebooting into the new kernel.  If hashed directories were used, one
or both of the filenames might not be accessible, but it wouldn't lead
to an actual file system level inconsistency.  And data would only be
lost if the wrong file were to get accidentally deleted in the confusion.
 
I'm curious how Windows handles this problem.  Windows and Apple are
happy to include the latest set of emoji's as they become available
--- after all, that's a key competitive advantage for some markets :-)
--- so I'm guessing they must not be terribly strict about Unicode
versioning, either.  So maybe the right thing to do is to just call it
"utf8" and be done with it.  :-)

       	   					- Ted
Gabriel Krisman Bertazi Nov. 30, 2018, 8:46 p.m. UTC | #2 
"Theodore Y. Ts'o" <tytso@mit.edu> writes:

> On Mon, Nov 26, 2018 at 05:19:38PM -0500, Gabriel Krisman Bertazi wrote:
>> +	/* 0x1 */ {"utf8-10.0", (EXT4_UTF8_NORMALIZATION_TYPE_NFKD |
>> +				 EXT4_UTF8_CASEFOLD_TYPE_NFKDCF)},
>
> We're using 10.0 here even though the later in the patch we're
> installing Unicode 11.0.  What if we just call this utf8-10+?  Unicode
> releases new versions every six months these days, and so long as the
> case fold rules don't change for any existing characters, but are only
> added for new characters added to the new version of Unicode, it would
> definitely be OK for strict mode.
>
> Even in relaxed mode, if someone decided to use, say, Klingon
> characters not recognized by the Unicode consortium in their system,
> and later on the Unicode consortium reassigns those code points to
> some obscure ancient script, it would be unfortunate, how much would
> it be *our* problem?  The worst that could happen is that if case
> folding were enabled, two file names that were previously unique would
> be considered identical by the new case folding rules after the
> rebooting into the new kernel.  If hashed directories were used, one
> or both of the filenames might not be accessible, but it wouldn't lead
> to an actual file system level inconsistency.  And data would only be
> lost if the wrong file were to get accidentally deleted in the
> confusion.

If this is not our problem, it does get much easier.  But we might be
able to assist the user a bit more, if we store the version in the
superblock.

We only allow the user to specify utf8 without requiring a version in
mkfs, just like you said, but we still write the unicode version in the
superblock.  The kernel will always mount using the newest unicode, but
recommend the user to run fsck if there is a version mismatch.  fsck can
then check the filesystem using first the newest version, and if an
invalid is found, it tries to use the exact superblock version.  If the
second attempt doesn't fail, we can rehash the entry, because no real
inconsistencies actually exist. If the rehash triggers a collision, we
could ask the user interactively what to do, if we can be interactive in
fsck (we can't, right?).  Otherwise, if we can't solve the collisions,
we set a flag in the superblock to force the exact version when mounting
the next time.  The user loses normalization of new scripts, but we warn
them about it, and the existing data is preserved and accessible.
Finally, if no collision is detected, or if we can solve all of then, we
write the new hashes and silently update the unicode version flag in the
superblock in fsck.

The interface becomes simpler for the common user, we basically hide
unicode versioning from someone that is not playing with ancient
scripts, and they still benefit from the new version by just rebooting
to an updated kernel.  But we still give the user that actually cares
about ancient scripts a way to fix her situation.

> I'm curious how Windows handles this problem.  Windows and Apple are
> happy to include the latest set of emoji's as they become available
> --- after all, that's a key competitive advantage for some markets :-)
> --- so I'm guessing they must not be terribly strict about Unicode
> versioning, either.  So maybe the right thing to do is to just call it
> "utf8" and be done with it.  :-)

I just did some tests on a macbook.  The machine was on xnu-4570.71.1~1,
which is pre-unicode 11 and creating a file with a unicode 11+ sequence
triggers an "illegal byte sequence" error.  After updating the system
(to xnu-4903.221.2), i can create files using new emoticons. So, from what
i can tell, apple is using a more strict mode that reject invalid
sequences.

Windows seems more permissive, I can create a unicode 11 file on a
system I am sure doesn't have unicode 11 support, since it is much older
than that version, and I can check the file name on disk is the one i
asked for.
diff mbox series

Patch

diff --git a/lib/e2p/Makefile.in b/lib/e2p/Makefile.in
index 2b0aa1915130..68d534cdaf11 100644
--- a/lib/e2p/Makefile.in
+++ b/lib/e2p/Makefile.in
@@ -19,7 +19,8 @@  all::	e2p.pc
 OBJS=		feature.o fgetflags.o fsetflags.o fgetversion.o fsetversion.o \
 		getflags.o getversion.o hashstr.o iod.o ls.o ljs.o mntopts.o \
 		parse_num.o pe.o pf.o ps.o setflags.o setversion.o uuid.o \
-		ostype.o percent.o crypto_mode.o fgetproject.o fsetproject.o
+		ostype.o percent.o crypto_mode.o fgetproject.o fsetproject.o \
+		encoding.o
 
 SRCS=		$(srcdir)/feature.c $(srcdir)/fgetflags.c \
 		$(srcdir)/fsetflags.c $(srcdir)/fgetversion.c \
@@ -29,7 +30,7 @@  SRCS=		$(srcdir)/feature.c $(srcdir)/fgetflags.c \
 		$(srcdir)/pe.c $(srcdir)/pf.c $(srcdir)/ps.c \
 		$(srcdir)/setflags.c $(srcdir)/setversion.c $(srcdir)/uuid.c \
 		$(srcdir)/ostype.c $(srcdir)/percent.c $(srcdir)/crypto_mode.c \
-		$(srcdir)/fgetproject.c $(srcdir)/fsetproject.c
+		$(srcdir)/fgetproject.c $(srcdir)/fsetproject.c $(srcdir)/encoding.c
 HFILES= e2p.h
 
 LIBRARY= libe2p
@@ -147,6 +148,9 @@  getversion.o: $(srcdir)/getversion.c $(top_builddir)/lib/config.h \
 hashstr.o: $(srcdir)/hashstr.c $(top_builddir)/lib/config.h \
  $(top_builddir)/lib/dirpaths.h $(srcdir)/e2p.h \
  $(top_srcdir)/lib/ext2fs/ext2_fs.h $(top_builddir)/lib/ext2fs/ext2_types.h
+encoding.o: $(srcdir)/encoding.c $(top_builddir)/lib/config.h \
+ $(top_builddir)/lib/dirpaths.h $(srcdir)/e2p.h \
+ $(top_srcdir)/lib/ext2fs/ext2_fs.h $(top_builddir)/lib/ext2fs/ext2_types.h
 iod.o: $(srcdir)/iod.c $(top_builddir)/lib/config.h \
  $(top_builddir)/lib/dirpaths.h $(srcdir)/e2p.h \
  $(top_srcdir)/lib/ext2fs/ext2_fs.h $(top_builddir)/lib/ext2fs/ext2_types.h
diff --git a/lib/e2p/e2p.h b/lib/e2p/e2p.h
index d70b59a5d358..c3a6b2587bf6 100644
--- a/lib/e2p/e2p.h
+++ b/lib/e2p/e2p.h
@@ -80,3 +80,8 @@  unsigned int e2p_percent(int percent, unsigned int base);
 
 const char *e2p_encmode2string(int num);
 int e2p_string2encmode(char *string);
+
+int e2p_str2encoding(const char *string);
+const char *e2p_encoding2str(int encoding);
+int e2p_get_encoding_flags(int encoding);
+int e2p_str2encoding_flags(int encoding, char *param, __u16 *flags);
diff --git a/lib/e2p/encoding.c b/lib/e2p/encoding.c
new file mode 100644
index 000000000000..a441647aaea3
--- /dev/null
+++ b/lib/e2p/encoding.c
@@ -0,0 +1,97 @@ 
+/*
+ * encoding.c --- convert between encoding magic numbers and strings
+ *
+ * Copyright (C) 2018  Collabora Ltd.
+ *
+ * %Begin-Header%
+ * This file may be redistributed under the terms of the GNU Library
+ * General Public License, version 2.
+ * %End-Header%
+ */
+
+#include "config.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <ctype.h>
+#include <errno.h>
+#include <stdio.h>
+
+#include "e2p.h"
+
+#define ARRAY_SIZE(array)			\
+        (sizeof(array) / sizeof(array[0]))
+
+static const struct {
+	char *name;
+	__u16 default_flags;
+} ext4_encoding_map[] = {
+	/* 0x0 */ { "ascii", 0},
+	/* 0x1 */ {"utf8-10.0", (EXT4_UTF8_NORMALIZATION_TYPE_NFKD |
+				 EXT4_UTF8_CASEFOLD_TYPE_NFKDCF)},
+};
+
+static const struct enc_flags {
+	__u16 flag;
+	char *param;
+} encoding_flags[] = {
+	{ EXT4_ENC_STRICT_MODE_FL, "strict" },
+};
+
+/* Return a positive number < 0xff indicating the encoding magic number
+ * or a negative value indicating error. */
+int e2p_str2encoding(const char *string)
+{
+	int i;
+
+	for (i = 0 ; i < ARRAY_SIZE(ext4_encoding_map); i++)
+		if (!strcmp(string, ext4_encoding_map[i].name))
+			return i;
+
+	return -EINVAL;
+}
+
+const char *e2p_encoding2str(int encoding)
+{
+	if (encoding < ARRAY_SIZE(ext4_encoding_map))
+		return ext4_encoding_map[encoding].name;
+	return NULL;
+}
+
+int e2p_get_encoding_flags(int encoding)
+{
+	if (encoding < ARRAY_SIZE(ext4_encoding_map))
+		return ext4_encoding_map[encoding].default_flags;
+	return 0;
+}
+
+int e2p_str2encoding_flags(int encoding, char *param, __u16 *flags)
+{
+	char *f = strtok(param, "-");
+	const struct enc_flags *fl;
+	int i, neg = 0;
+
+	while (f) {
+		neg = 0;
+		if (!strncmp("no", f, 2)) {
+			neg = 1;
+			f += 2;
+		}
+
+		for (i = 0; i < ARRAY_SIZE(encoding_flags); i++) {
+			fl = &encoding_flags[i];
+			if (!strcmp(fl->param, f)) {
+				if (neg)
+					*flags &= ~fl->flag;
+				else
+					*flags |= fl->flag;
+
+				goto next_flag;
+			}
+		}
+		return -EINVAL;
+	next_flag:
+		f = strtok(NULL, "-");
+	}
+	return 0;
+}
diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
index f1c405b76339..36ae7ae41c47 100644
--- a/lib/ext2fs/ext2_fs.h
+++ b/lib/ext2fs/ext2_fs.h
@@ -16,6 +16,7 @@ 
 #ifndef _LINUX_EXT2_FS_H
 #define _LINUX_EXT2_FS_H
 
+#include <stddef.h>
 #include <ext2fs/ext2_types.h>		/* Changed from linux/types.h */
 
 #ifndef __GNUC_PREREQ
@@ -1127,4 +1128,8 @@  struct mmp_struct {
  */
 #define EXT4_INLINE_DATA_DOTDOT_SIZE	(4)
 
+#define EXT4_ENC_STRICT_MODE_FL			(1 << 0) /* Reject invalid sequences */
+#define EXT4_UTF8_NORMALIZATION_TYPE_NFKD	(1 << 1)
+#define EXT4_UTF8_CASEFOLD_TYPE_NFKDCF		(1 << 4)
+
 #endif	/* _LINUX_EXT2_FS_H */