bug in writing smb.conf using MBCS and UTF8 from SWAT.

Ryo Kawahara rkawa at lbe.co.jp
Fri Dec 8 13:11:34 GMT 2000


Hi all.
I've found bugs in SWAT which occurs when we write parameters
and share names in MBCS or utf8, which is newly added in 2.2-alpha.
The patch applies to samba-2.2.0-alhpa1.

I've checked the source code of SWAT and now I see that
SWAT is using client code page, which is the internal encoding of
samba, for character encoding. Am I correct?

And when we load smb.conf with it, multibyte characters in the file
are correctly converted from unix encoding (specified by codingsystem=)
to dos encoding (i.e. internal encoding), then displayed on web.

But when we "commit changes", "create share" or "view config"  with SWAT,
(inverse) multibyte character conversion is done incorrectly.
The reason why this occurs is as follows:

1. lp_dump, lp_dump_one(),etc. which are used by SWAT and testparm
   are not aware of what character code is used. since always dos encoding
   is used, values are written in wrong character code into smb.conf, which
   must be written in unix code.

2. For this reason, testparm also has a problem that its output is always
   encoded in client code page.

3. lp_do_parameter() and add_service(),
   which does unix_to_dos() since it might be designed for
   reading smb.conf, is re-used from SWAT to create share with dos encoded
   characters.

To fix 1, I have change the interface of lp_dump() like:
void lp_dump(FILE *f, BOOL show_defaults, int maxtoprint, char *(*dos_to_ext)(char *, BOOL));
void lp_dump_one(FILE *f, BOOL show_defaults, int snum, char *(*dos_to_ext)(char *, BOOL));
Now we can specify what encoding should be used to print by passing
conversion function pointer.

For 2, new commandline argument -t is added to specify terminal encoding.
This is done by SUGJ, especially Motonobu Takahashi <monyo at home.monyo.com>.

To fix 3, dos_to_unix() conversion is inserted in commit_parameter()
before calling lp_xxxx() functions. I know this is really ad-hoc fix.

This patch was originally a part of SWAT gettext()izing patch:
http://samba.org/cgi-bin/samba-patches/incoming?id=187;user=guest
but that patch includes new features and bug fix together,
so I have separeted this bug fix part.
I'll recreate gettext()izing patch later.

There are not so many utf8-enabled editors, but
with this patch, utf8 codingsystem is easily used with SWAT.
(mozilla and IE can handle utf8 correctly.)
Only one problem is, that SWAT can handle only single client code page
(i.e. SWAT encoding), so we can't edit multilingual smb.conf with it.
This bug fix will be obsoleted if samba internal encoding is changed
to UCS4 or something. 

///////////////////////////////////////////////////////////////
// Ryo Kawahara (rkawa at lbe.co.jp)
// website: http://www3.lbe.co.jp/~rkawa/
///////////////////////////////////////////////////////////////

The patch follows.


diff -uNr --exclude-from=patchexcl.txt samba-2.2.0-alpha1/docs/yodldocs/testparm.1.yo samba-2.2.0-alpha1-swatfix/docs/yodldocs/testparm.1.yo
--- samba-2.2.0-alpha1/docs/yodldocs/testparm.1.yo	Fri Mar 31 07:19:35 2000
+++ samba-2.2.0-alpha1-swatfix/docs/yodldocs/testparm.1.yo	Fri Dec  8 20:08:10 2000
@@ -50,6 +50,12 @@
 dit(bf(-L servername)) Sets the value of the %L macro to servername. This
 is useful for testing include files specified with the %L macro.
 
+label(minust)
+dit(bf(-t encoding)) Print parameters with encoding.
+The value of codingsystem parameter is used if omitted.
+See url(bf("codingsystem"))(smb.conf.5.html#codingsystem) for
+the list of available values to be specified.
+
 label(configfilename)
 dit(bf(configfilename)) This is the name of the configuration file to
 check. If this parameter is not present then the default
diff -uNr --exclude-from=patchexcl.txt samba-2.2.0-alpha1/source/include/kanji.h samba-2.2.0-alpha1-swatfix/source/include/kanji.h
--- samba-2.2.0-alpha1/source/include/kanji.h	Tue Oct  3 10:14:36 2000
+++ samba-2.2.0-alpha1-swatfix/source/include/kanji.h	Wed Dec  6 15:55:32 2000
@@ -166,6 +166,7 @@
 extern char *(*multibyte_strtok)(char *s1, const char *s2);
 extern char *(*_dos_to_unix)(char *str, BOOL overwrite);
 extern char *(*_unix_to_dos)(char *str, BOOL overwrite);
+extern char *(*_dos_to_dos)(char *str, BOOL overwrite);
 extern BOOL (*is_multibyte_char)(char c);
 extern int (*_skip_multibyte_char)(char c);
 
@@ -175,6 +176,7 @@
 #define strtok(s1, s2) ((*multibyte_strtok)((s1), (s2)))
 #define dos_to_unix(x,y) ((*_dos_to_unix)((x), (y)))
 #define unix_to_dos(x,y) ((*_unix_to_dos)((x), (y)))
+#define dos_to_dos(x,y) ((*_dos_to_dos)((x), (y)))
 #define skip_multibyte_char(c) ((*_skip_multibyte_char)((c)))
 
 #endif /* _KANJI_C_ */
diff -uNr --exclude-from=patchexcl.txt samba-2.2.0-alpha1/source/include/proto.h samba-2.2.0-alpha1-swatfix/source/include/proto.h
--- samba-2.2.0-alpha1/source/include/proto.h	Thu Nov 23 09:53:22 2000
+++ samba-2.2.0-alpha1-swatfix/source/include/proto.h	Wed Dec  6 15:33:23 2000
@@ -1578,8 +1578,8 @@
 	     BOOL add_ipc);
 void lp_resetnumservices(void);
 int lp_numservices(void);
-void lp_dump(FILE * f, BOOL show_defaults, int maxtoprint);
-void lp_dump_one(FILE * f, BOOL show_defaults, int snum);
+void lp_dump(FILE *f, BOOL show_defaults, int maxtoprint, char *(*dos_to_ext)(char *, BOOL));
+void lp_dump_one(FILE *f, BOOL show_defaults, int snum, char *(*dos_to_ext)(char *, BOOL));
 int lp_servicenumber(char *pszServiceName);
 char *volume_label(int snum);
 int lp_server_role(void);
diff -uNr --exclude-from=patchexcl.txt samba-2.2.0-alpha1/source/lib/kanji.c samba-2.2.0-alpha1-swatfix/source/lib/kanji.c
--- samba-2.2.0-alpha1/source/lib/kanji.c	Sat Oct  7 02:13:38 2000
+++ samba-2.2.0-alpha1-swatfix/source/lib/kanji.c	Wed Dec  6 15:49:27 2000
@@ -1673,3 +1673,19 @@
     break; 
   }
 }
+/* *******************************************************
+   function(s) for "dynamic" encoding of SWAT output.
+   in this version, only dos_to_dos, dos_to_unix, unix_to_dos
+   are used for bug fix. conversion to web encoding
+   (to catalog file encoding) is not needed because
+   they are using same character codes.
+   **************************************************** */
+static char *no_conversion(char *str, BOOL bOverwrite)
+{
+	static pstring temp;
+	if(bOverwrite)
+		return str;
+	pstrcpy(temp, str);
+	return temp;
+}
+char *(*_dos_to_dos)(char *, BOOL) = no_conversion;
diff -uNr --exclude-from=patchexcl.txt samba-2.2.0-alpha1/source/param/loadparm.c samba-2.2.0-alpha1-swatfix/source/param/loadparm.c
--- samba-2.2.0-alpha1/source/param/loadparm.c	Thu Nov 23 10:01:47 2000
+++ samba-2.2.0-alpha1-swatfix/source/param/loadparm.c	Fri Dec  8 18:05:52 2000
@@ -2627,7 +2627,7 @@
 /***************************************************************************
 print a parameter of the specified type
 ***************************************************************************/
-static void print_parameter(struct parm_struct *p, void *ptr, FILE * f)
+static void print_parameter(struct parm_struct *p, void *ptr, FILE * f,  char *(*dos_to_ext)(char *, BOOL))
 {
 	int i;
 	switch (p->type)
@@ -2666,15 +2666,24 @@
 
 		case P_GSTRING:
 		case P_UGSTRING:
-			if ((char *)ptr)
-				fprintf(f, "%s", (char *)ptr);
+  		        if ((char *)ptr) {
+			  if (p->flags & FLAG_DOS_STRING)
+			      fprintf(f, "%s", dos_to_ext((char *)ptr, False));
+			  else
+			      fprintf(f, "%s", (char *)ptr);
+			}
 			break;
 
 		case P_STRING:
 		case P_USTRING:
-			if (*(char **)ptr)
-				fprintf(f, "%s", *(char **)ptr);
+		        if (*(char **)ptr) {
+			    if(p->flags & FLAG_DOS_STRING)
+				fprintf(f,"%s",dos_to_ext(*(char **)ptr, False));
+			    else
+				fprintf(f,"%s",*(char **)ptr);
+			}
 			break;
+
 		case P_SEP:
 			break;
 	}
@@ -2828,7 +2837,7 @@
 /***************************************************************************
 Display the contents of the global structure.
 ***************************************************************************/
-static void dump_globals(FILE * f)
+static void dump_globals(FILE *f, char *(*dos_to_ext)(char *, BOOL))
 {
 	int i;
 	fprintf(f, "# Global parameters\n[global]\n");
@@ -2841,7 +2850,8 @@
 			if (defaults_saved && is_default(i))
 				continue;
 			fprintf(f, "\t%s = ", parm_table[i].label);
-			print_parameter(&parm_table[i], parm_table[i].ptr, f);
+			print_parameter(&parm_table[i], parm_table[i].ptr, f,
+					dos_to_ext);
 			fprintf(f, "\n");
 		}
 }
@@ -2862,11 +2872,11 @@
 /***************************************************************************
 Display the contents of a single services record.
 ***************************************************************************/
-static void dump_a_service(service * pService, FILE * f)
+static void dump_a_service(service * pService, FILE * f, char *(*dos_to_ext)(char *, BOOL))
 {
 	int i;
 	if (pService != &sDefault)
-		fprintf(f, "\n[%s]\n", pService->szService);
+		fprintf(f, "\n[%s]\n", dos_to_ext(pService->szService, False));
 
 	for (i = 0; parm_table[i].label; i++)
 		if (parm_table[i].class == P_LOCAL &&
@@ -2893,7 +2903,8 @@
 
 			fprintf(f, "\t%s = ", parm_table[i].label);
 			print_parameter(&parm_table[i],
-					((char *)pService) + pdiff, f);
+					((char *)pService) + pdiff, f,
+					dos_to_ext);
 			fprintf(f, "\n");
 		}
 }
@@ -3260,7 +3271,7 @@
 /***************************************************************************
 Display the contents of the services array in human-readable form.
 ***************************************************************************/
-void lp_dump(FILE * f, BOOL show_defaults, int maxtoprint)
+void lp_dump(FILE *f, BOOL show_defaults, int maxtoprint, char *(*dos_to_ext)(char *, BOOL))
 {
 	int iService;
 
@@ -3269,24 +3280,24 @@
 		defaults_saved = False;
 	}
 
-	dump_globals(f);
+	dump_globals(f, dos_to_ext);
 
-	dump_a_service(&sDefault, f);
+	dump_a_service(&sDefault, f, dos_to_ext);
 
 	for (iService = 0; iService < maxtoprint; iService++)
-		lp_dump_one(f, show_defaults, iService);
+		lp_dump_one(f, show_defaults, iService, dos_to_ext);
 }
 
 /***************************************************************************
 Display the contents of one service in human-readable form.
 ***************************************************************************/
-void lp_dump_one(FILE * f, BOOL show_defaults, int snum)
+void lp_dump_one(FILE *f, BOOL show_defaults, int snum, char *(*dos_to_ext)(char *, BOOL))
 {
 	if (VALID(snum))
 	{
 		if (iSERVICE(snum).szService[0] == '\0')
 			return;
-		dump_a_service(pSERVICE(snum), f);
+		dump_a_service(pSERVICE(snum), f, dos_to_ext);
 	}
 }
 
diff -uNr --exclude-from=patchexcl.txt samba-2.2.0-alpha1/source/utils/testparm.c samba-2.2.0-alpha1-swatfix/source/utils/testparm.c
--- samba-2.2.0-alpha1/source/utils/testparm.c	Wed Jul 19 05:15:15 2000
+++ samba-2.2.0-alpha1-swatfix/source/utils/testparm.c	Fri Dec  8 19:57:52 2000
@@ -146,6 +146,7 @@
 	printf("\t-s                  Suppress prompt for enter\n");
 	printf("\t-h                  Print usage\n");
 	printf("\t-L servername       Set %%L macro to servername\n");
+	printf("\t-t encoding         Print parameters with encoding\n");
 	printf("\tconfigfilename      Configuration file to test\n");
 	printf("\thostname hostIP.    Hostname and Host IP address to test\n");
 	printf("\t                    against \"host allow\" and \"host deny\"\n");
@@ -163,6 +164,9 @@
   int s;
   BOOL silent_mode = False;
   int ret = 0;
+  pstring term_code;
+
+  *term_code = 0;
 
   TimeInit();
 
@@ -170,7 +174,7 @@
   
   charset_initialise();
 
-  while ((opt = getopt(argc, argv,"shL:")) != EOF) {
+  while ((opt = getopt(argc, argv,"shL:t:")) != EOF) {
   switch (opt) {
     case 's':
       silent_mode = True;
@@ -182,6 +186,9 @@
       usage(argv[0]);
       exit(0);
       break;
+    case 't':
+      pstrcpy(term_code,optarg);
+      break;
     default:
       printf("Incorrect program usage\n");
       usage(argv[0]);
@@ -250,13 +257,16 @@
     }
   }
 
+  if (*term_code)
+    interpret_coding_system(term_code);
+
   if (argc < 3) {
     if (!silent_mode) {
       printf("Press enter to see a dump of your service definitions\n");
       fflush(stdout);
       getc(stdin);
     }
-    lp_dump(stdout,True, lp_numservices());
+    lp_dump(stdout,True, lp_numservices(), _dos_to_unix);
   }
   
   if (argc >= 3) {
diff -uNr --exclude-from=patchexcl.txt samba-2.2.0-alpha1/source/web/swat.c samba-2.2.0-alpha1-swatfix/source/web/swat.c
--- samba-2.2.0-alpha1/source/web/swat.c	Wed Sep 13 16:08:09 2000
+++ samba-2.2.0-alpha1-swatfix/source/web/swat.c	Fri Dec  8 21:53:10 2000
@@ -326,13 +326,13 @@
 /****************************************************************************
   write a config file 
 ****************************************************************************/
-static void write_config(FILE *f, BOOL show_defaults)
+static void write_config(FILE *f, BOOL show_defaults, char *(*dos_to_ext)(char *, BOOL))
 {
 	fprintf(f, "# Samba config file created using SWAT\n");
 	fprintf(f, "# from %s (%s)\n", cgi_remote_host(), cgi_remote_addr());
 	fprintf(f, "# Date: %s\n\n", timestring(False));
-	
-	lp_dump(f, show_defaults, iNumNonAutoPrintServices);	
+
+	lp_dump(f, show_defaults, iNumNonAutoPrintServices, dos_to_ext);
 }
 
 /****************************************************************************
@@ -348,9 +348,9 @@
 		return 0;
 	}
 
-	write_config(f, False);
+	write_config(f, False, _dos_to_unix);
 	if (snum)
-		lp_dump_one(f, False, snum);
+		lp_dump_one(f, False, snum, _dos_to_unix);
 	fclose(f);
 
 	lp_killunused(NULL);
@@ -373,6 +373,9 @@
 	int i;
 	char *s;
 
+	/* lp_do_parameter() will do unix_to_dos(v). */
+	if(parm->flags & FLAG_DOS_STRING)
+		dos_to_unix(v, True);
 	if (snum < 0 && parm->class == P_LOCAL) {
 		/* this handles the case where we are changing a local
 		   variable globally. We need to change the parameter in 
@@ -472,7 +475,7 @@
 	}
 
 	printf("<p><pre>");
-	write_config(stdout, full_view);
+	write_config(stdout, full_view, _dos_to_dos);
 	printf("</pre>");
 	printf("</form>\n");
 }
@@ -551,8 +554,13 @@
 	}
 
 	if (cgi_variable("createshare") && (share=cgi_variable("newshare"))) {
+		/* add_a_service() which is called by lp_copy_service()
+		   will do unix_to_dos() conversion,
+		   so we need dos_to_unix() before the lp_copy_service(). */
+		pstring unix_share;
+		pstrcpy(unix_share, dos_to_unix(share, False));
 		load_config(False);
-		lp_copy_service(GLOBALS_SNUM, share);
+		lp_copy_service(GLOBALS_SNUM, unix_share);
 		iNumNonAutoPrintServices = lp_numservices();
 		save_reload(0);
 		snum = lp_servicenumber(share);
@@ -886,8 +894,13 @@
 	}
 
 	if (cgi_variable("createshare") && (share=cgi_variable("newshare"))) {
+		/* add_a_service() which is called by lp_copy_service()
+		   will do unix_to_dos() conversion,
+		   so we need dos_to_unix() before the lp_copy_service(). */
+		pstring unix_share;
+		pstrcpy(unix_share, dos_to_unix(share, False));
 		load_config(False);
-		lp_copy_service(GLOBALS_SNUM, share);
+		lp_copy_service(GLOBALS_SNUM, unix_share);
 		iNumNonAutoPrintServices = lp_numservices();
 		snum = lp_servicenumber(share);
 		lp_do_parameter(snum, "print ok", "Yes");




More information about the samba-technical mailing list