Re: BonoboStream from Moniker [Patch]

[Jens Finke <jens triq net>]

Hi Micheal,

On 25 Jan 2002, Michael Meeks wrote:
>
> 	Hmm - that sucks. Well - I suppose we're going to need to put some
> options on the end of the moniker - a little like file:/foo.bar,+ or
> something - use the fopen type syntax, escape commas in the string, and
> update the moniker.
>
> 	How does that sound ? it's a pain - but not that much, can you take it
> on ?

Ok, I have a patch ready (attached). But there is one problem I don't know
how to cope with: The escaping of the ',' won't work. Within the moniker
implementation you get always the unescaped string. This is because
bonobo_moniker_set_name unescapes it always.

Now, consider the following example: You want to open a stream for the
file '/foo.bar,w' for reading. You create the stream with:

 moniker = g_strconcat ("file:", bonobo_moniker_util_escape ("/foo.bar,w"),
                        NULL);
 stream = bonobo_get_object (moniker, "Bonobo/Stream", NULL);

Within the moniker implementation we get the string '/foo.bar,w' which
will be interpreted as open file '/foo.bar' for writing!

AFAICS this is only a problem if your file name ends on either ",w",
",w+", ",r" or ",r+" (these are the valid option suffixes, all other will
be overread) _and_ you don't add an option string explicitly. To reuse the
above example: Everything goes fine if you construct the moniker like:

 moniker = g_strconcat ("file:", bonobo_moniker_util_escape ("/foo.bar,w"),
                        ",r", NULL);

The question is: Can we go with this design? If so, it's not neccessary
to escape the string. Therefore bonobo_moniker_util_escape doesn't need to
know anything about the ',' character. Also, there is no need for
bonobo_moniker_util_unescape in get_storage_options then.

Personally I think this design is better than the implementation before.
Since the stream/storage stuff relies entirely on the monikers in Gnome 2,
it wouldn't be possible to create or even write files with BonoboStreams
otherwise.

Michael, what do you think?

Regards,

   Jens

-- 
"Wer die Freiheit aufgibt, um Sicherheit zu gewinnen, wird am Ende beides
verlieren." -- Benjamin Franklin

Index: gnome-vfs/monikers/ChangeLog
===================================================================
RCS file: /cvs/gnome/gnome-vfs/monikers/ChangeLog,v
retrieving revision 1.42
diff -u -p -r1.42 ChangeLog
--- gnome-vfs/monikers/ChangeLog	2002/01/17 17:59:25	1.42
+++ gnome-vfs/monikers/ChangeLog	2002/01/27 17:42:25
@@ -1,3 +1,10 @@
+2002-01-27  Jens Finke <jens triq net>
+
+	* bonobo-moniker-file.c (bonobo_moniker_file_resolve): Consider
+	stream/storage options.
+	(get_storage_options): New function. Extracts storage/stream
+	options from the moniker string.
+	
 2002-01-17  Darin Adler  <darin bentspoon com>
 
 	* bonobo-storage-fs.c: Added an include of <string.h>, needed because
Index: gnome-vfs/monikers/bonobo-moniker-file.c
===================================================================
RCS file: /cvs/gnome/gnome-vfs/monikers/bonobo-moniker-file.c,v
retrieving revision 1.18
diff -u -p -r1.18 bonobo-moniker-file.c
--- gnome-vfs/monikers/bonobo-moniker-file.c	2001/07/10 12:04:03	1.18
+++ gnome-vfs/monikers/bonobo-moniker-file.c	2002/01/27 17:42:25
@@ -18,52 +18,127 @@
 #include "bonobo-stream-fs.h"
 #include "bonobo-storage-fs.h"
 
+static char*
+get_storage_options (const char *fname, int *options)
+{
+	int len;
+	int pos = 0;
+	char *retval = NULL;
+
+	*options = 0;
+	len = strlen (fname);
+
+	if (len > 3) {
+		pos = len - 3;
+		if (!strcmp (&fname[pos], ",r+")) {
+			*options = Bonobo_Storage_READ | Bonobo_Storage_WRITE;
+			retval = bonobo_moniker_util_unescape (fname, pos);
+		}
+		else if (!strcmp (&fname[pos], ",w+")) {
+			*options = Bonobo_Storage_WRITE | Bonobo_Storage_CREATE;
+			retval = bonobo_moniker_util_unescape (fname, pos);
+		}
+	}
+	
+	if (len > 2 && !retval) {
+		pos = len - 2;
+		if (!strcmp (&fname[pos], ",w")) {
+			*options = Bonobo_Storage_WRITE;
+			retval = bonobo_moniker_util_unescape (fname, pos);
+		}
+		else if (!strcmp (&fname[pos], ",r")) {
+			*options = Bonobo_Storage_READ;
+			retval = bonobo_moniker_util_unescape (fname, pos);
+		}
+	}
+
+	if (retval) {
+		int backslashes = 0;
+		int p;
+
+		/* check if the ',' is escaped and thus invalid */
+		for (p = pos - 1; p >= 0; p--) {
+			if (fname[p] == '\\') 
+				backslashes++;
+			else 
+				break;
+		}
+
+		if ((backslashes % 2) == 1) {
+			g_free (retval);
+			retval = NULL;
+			*options = 0;
+		}
+	}
+
+	if (!retval) {
+		/* set default values */
+		*options = Bonobo_Storage_READ;
+		retval = bonobo_moniker_util_unescape (fname, len);
+	}
+		
+	return retval;
+}
+
 Bonobo_Unknown
 bonobo_moniker_file_resolve (BonoboMoniker               *moniker,
 			     const Bonobo_ResolveOptions *options,
 			     const CORBA_char            *requested_interface,
 			     CORBA_Environment           *ev)
 {
-	const char    *fname = bonobo_moniker_get_name (moniker);
+	int            storage_opts;
+	char           *fname; 
 	Bonobo_Unknown retval;
+	
+	fname = get_storage_options (bonobo_moniker_get_name (moniker),
+				     &storage_opts);
 
 	if (!strcmp (requested_interface, "IDL:Bonobo/Stream:1.0")) {
 		BonoboObject *stream;
-		
+
 		stream = BONOBO_OBJECT (bonobo_stream_fs_open (
-			fname, Bonobo_Storage_READ, 0664, ev));
+			fname, storage_opts, 0664, ev));
 
-		if (BONOBO_EX (ev))
+		if (BONOBO_EX (ev)) {
+			g_free (fname);
 			return CORBA_OBJECT_NIL;
+		}
 
 		if (!stream) {
 			g_warning ("Failed to open stream '%s'", fname);
 			CORBA_exception_set (ev, CORBA_USER_EXCEPTION,
 					     ex_Bonobo_Moniker_InterfaceNotFound, NULL);
+			g_free (fname);
 			return CORBA_OBJECT_NIL;
 		}
 
+		g_free (fname);
 		return CORBA_Object_duplicate (BONOBO_OBJREF (stream), ev);
 
 	} else if (!strcmp (requested_interface, "IDL:Bonobo/Storage:1.0")) {
 		BonoboObject *storage;
 		
 		storage = BONOBO_OBJECT (bonobo_storage_fs_open (
-			fname, Bonobo_Storage_READ, 0664, ev));
+			fname, storage_opts, 0664, ev));
 
-		if (BONOBO_EX (ev))
+		if (BONOBO_EX (ev)) {
+			g_free (fname);
 			return CORBA_OBJECT_NIL;
+		}
 
 		if (!storage) {
 			g_warning ("Failed to open storage '%s'", fname);
 			CORBA_exception_set (ev, CORBA_USER_EXCEPTION,
 					     ex_Bonobo_Moniker_InterfaceNotFound, NULL);
+			g_free (fname);
 			return CORBA_OBJECT_NIL;
 		}
 
+		g_free (fname);
 		return CORBA_Object_duplicate (BONOBO_OBJREF (storage), ev);
 	}
-
+	g_free (fname);
+			
 	retval = bonobo_moniker_use_extender (
 		"OAFIID:Bonobo_MonikerExtender_file",
 		moniker, options, requested_interface, ev);
Index: libbonobo/ChangeLog
===================================================================
RCS file: /cvs/gnome/libbonobo/ChangeLog,v
retrieving revision 1.284
diff -u -p -r1.284 ChangeLog
--- libbonobo/ChangeLog	2002/01/21 17:16:28	1.284
+++ libbonobo/ChangeLog	2002/01/27 17:42:28
@@ -1,3 +1,8 @@
+2002-01-27  Jens Finke <jens triq net>
+
+	* bonobo/bonobo-moniker-util.c (bonobo_moniker_util_escape):
+	Escape ',' character too.
+
 2002-01-21  Michael Meeks  <michael ximian com>
 
 	* bonobo/bonobo-generic-factory.c 
Index: libbonobo/bonobo/bonobo-moniker-util.c
===================================================================
RCS file: /cvs/gnome/libbonobo/bonobo/bonobo-moniker-util.c,v
retrieving revision 1.54
diff -u -p -r1.54 bonobo-moniker-util.c
--- libbonobo/bonobo/bonobo-moniker-util.c	2002/01/10 10:58:46	1.54
+++ libbonobo/bonobo/bonobo-moniker-util.c	2002/01/27 17:42:29
@@ -335,7 +335,8 @@ bonobo_moniker_util_escape (const char *
 			break;
 		else if (string [i] == '\\' ||
 			 string [i] == '#'  ||
-			 string [i] == '!')
+			 string [i] == '!'  ||
+			 string [i] == ',')
 			backslashes ++;
 	}
 	
@@ -347,7 +348,8 @@ bonobo_moniker_util_escape (const char *
 	for (i = offset; i < len; i++) {
 		if (string [i] == '\\' ||
 		    string [i] == '#'  ||
-		    string [i] == '!')
+		    string [i] == '!'  ||
+		    string [i] == ',')
 			*p++ = '\\';
 		*p++ = string [i];
 	}