scrollkeeper-devel

[Scrollkeeper-devel] Porblem with temporary files creation

[Benjamin T. <ben...@bu...>]

Some days ago, I reported the following bug for Scrollkeeper (see below).
I think this is a major problem if there is more than one user or more 
than one GNOME 2 installation on the same host.

I hope this could be fixed soon.

Thank you.

Benjamin.


BUG 614263
==========

Problem with temporary file names Scrollkeeper 0.3.9 :

After installation, main window of Yelp, GNOME2 help browser, showed 
*no* GNOME docs.

The problem I found was that :

-- Yelp executes scrollkeeper-get-content-list to build its DB.

-- scrollkeeper-get-content-list write the content in a temporary file

-- Temporary files can be :
    /tmp/scrollkeeper-tempfile.#,
    where # is a number between 0 and 4.

-- In /tmp, all the five possible temporary files already existed and 
were owned by another user.
    On the same system we have about 4 or 5 GNOME 2 installations running.
 
-- scrollkeeper-get-content-list couldn't write its file but didn't 
complain, because the other user owned all the temporary files.

-- Yelp parsed on of the file created by the other user which did not 
contained all the entries.

-- As a result, I can access only the docs installed by the other user.


I think the function get_next_free_temp_file_path() in cl/get-cl.c has a 
problem.

-- Either, it must create the temporary file with write permission for 
everyone,
-- Or, the procedure to select a filename as to be changed (may be by 
using a random file name or adding
the username or UID to the temporary filename)



-- 
B e n j a m i n   T h e r y  - BULL/BI&S/GeODE/AIX & Linux CC

 mailto:Ben...@bu...
   http://www.bull.com

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Malcolm T. <ma...@co...>]

Hi Benjamin,

On Wed, Oct 02, 2002 at 09:58:37AM +0200, Benjamin Thery wrote:
> Some days ago, I reported the following bug for Scrollkeeper (see below).
> I think this is a major problem if there is more than one user or more 
> than one GNOME 2 installation on the same host.
> 
> I hope this could be fixed soon.

Thanks for the bug report. You are correct, the problem will happen
exactly as you have observed and it is most likely to be triggered in
scenarios like yours where there are multiple users on the same system.

As for fixing it, there are two possibilities (and this is the part
where I keep hoping that other people on the list might comment on my
ideas):

(1) We use mkstemp to generate a unique filename each time (and include
our own implementation for use on systems that don't have mkstemp).

	Advantages:
		- Avoids the recent security problem (which is fixed in
		  CVS, btw).
		- Gets around the "maximum of five files" problem.

	Disadvantages:
		- There is going to be a metric truckload of files with
		  stupid names floating around in some subdirectory of
		  the tmp directory after a while. In reality, these are
		  never going to be cleaned up except by something like
		  tmpwatch (which is not frequent enough on, say, Linux
		  systems).

(2) The scrollkeeper-get-cl binary dumps the result to stdout, rather
than saving to a file.

	Advantages:
		- Never have to worry about stray files lying around.
		- Makes scrollkeeper easily multiuser without having to
		  sweat on a lot of the locking issues all the time.

	Disadvantages:
		- Breaks backwards compatibility. All applications
		  currently using scrollkeeper via scrolkeeper-get-cl
		  and friends.

Although option (2) seems clearly preferable to me, the backwards
compatibility issues means it is probably an 0.5 feature. This means we
may have to go with the "dozens of stupid temporary" files in the
0.3/0.4 timeframe unless somebody can come up with a better idea.

Anybody have thoughts?

Malcolm

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Benjamin T. <ben...@bu...>]

Malcolm Tredinnick wrote:

[...]

>Although option (2) seems clearly preferable to me, the backwards
>compatibility issues means it is probably an 0.5 feature. This means we
>may have to go with the "dozens of stupid temporary" files in the
>0.3/0.4 timeframe unless somebody can come up with a better idea.
>
>Anybody have thoughts?
>
>Malcolm
>
>  
>
Option (2) seems the cleanest to me too, but I don't know if the guys at 
Yelp will like it ;-)

As a temporary fix, isn't it possible to have only one temporary file by 
user, suffixed by the login name?
/tmp/scrollkeeper-tempfile.<login name>

Thus, we will have at most one temporary file by user in /tmp and it 
removes the problem of file names "shared" with other users (and access 
problem linked to it).

(I hope my suggestion is not completely stupid)

Benjamin

-- 
B e n j a m i n   T h e r y  - BULL/BI&S/GeODE/AIX & Linux CC

 mailto:Ben...@bu...
   http://www.bull.com

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Malcolm T. <ma...@co...>]

On Wed, Oct 02, 2002 at 10:51:53AM +0200, Benjamin Thery wrote:
> Malcolm Tredinnick wrote:
> [...]
> 
> >Although option (2) seems clearly preferable to me, the backwards
> >compatibility issues means it is probably an 0.5 feature. This means we
> >may have to go with the "dozens of stupid temporary" files in the
> >0.3/0.4 timeframe unless somebody can come up with a better idea.
> >
> >Anybody have thoughts?
> >
> >Malcolm
> >
> >  
> >
> Option (2) seems the cleanest to me too, but I don't know if the guys at 
> Yelp will like it ;-)

It's not just Yelp (the change required in Yelp is pretty simple -- I've
already looked at that). The problem is all the applications we don't
know about that are relying on the current behaviour.

> As a temporary fix, isn't it possible to have only one temporary file by 
> user, suffixed by the login name?
> /tmp/scrollkeeper-tempfile.<login name>

That's not a bad idea. There may be some situation where more than one
file is required (I must admit to not having thought about why five was
chosen), but a per-user system using login name is something to work
with in any case. Kind of obvious, in retrospect. :-)

Thanks for the idea. I'll think some more about this and code something
up when I have some time (won't be for a few days).

Cheers,
Malcolm

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Sander V. <San...@Su...>]

On Wed, 2 Oct 2002, Malcolm Tredinnick wrote:

> > As a temporary fix, isn't it possible to have only one temporary file by 
> > user, suffixed by the login name?
> > /tmp/scrollkeeper-tempfile.<login name>
> 
> That's not a bad idea. There may be some situation where more than one
> file is required (I must admit to not having thought about why five was
> chosen), but a per-user system using login name is something to work
> with in any case. Kind of obvious, in retrospect. :-)
> 

Please don't do this - it will give bad results if you are logged in into
an account more than once and the machines have different databases.

> Thanks for the idea. I'll think some more about this and code something
> up when I have some time (won't be for a few days).
> 
> Cheers,
> Malcolm
> 

	Sander

	There are voices in the street,
	And the sound of running feet,
	And they whisper the word --
	Revolution!

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Malcolm T. <ma...@co...>]

On Wed, Oct 02, 2002 at 01:56:18PM +0100, Sander Vesik wrote:
> On Wed, 2 Oct 2002, Malcolm Tredinnick wrote:
> 
> > > As a temporary fix, isn't it possible to have only one temporary file by 
> > > user, suffixed by the login name?
> > > /tmp/scrollkeeper-tempfile.<login name>
> > 
> > That's not a bad idea. There may be some situation where more than one
> > file is required (I must admit to not having thought about why five was
> > chosen), but a per-user system using login name is something to work
> > with in any case. Kind of obvious, in retrospect. :-)
> > 
> 
> Please don't do this - it will give bad results if you are logged in into
> an account more than once and the machines have different databases.

No it won't, because you still have to allow multiple files, so
scrollkeeper will still return a filename specific for that run. You
can't just have a single file (or even just a limited number of files)
or predicatable names because otherwise there is the same denial of
service situation as before.

Malcolm

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Sander V. <San...@Su...>]

On Thu, 3 Oct 2002, Malcolm Tredinnick wrote:

> On Wed, Oct 02, 2002 at 01:56:18PM +0100, Sander Vesik wrote:
> > On Wed, 2 Oct 2002, Malcolm Tredinnick wrote:
> > 
> > > > As a temporary fix, isn't it possible to have only one temporary file by 
> > > > user, suffixed by the login name?
> > > > /tmp/scrollkeeper-tempfile.<login name>
> > > 
> > > That's not a bad idea. There may be some situation where more than one
> > > file is required (I must admit to not having thought about why five was
> > > chosen), but a per-user system using login name is something to work
> > > with in any case. Kind of obvious, in retrospect. :-)
> > > 
> > 
> > Please don't do this - it will give bad results if you are logged in into
> > an account more than once and the machines have different databases.
> 
> No it won't, because you still have to allow multiple files, so
> scrollkeeper will still return a filename specific for that run. You
> can't just have a single file (or even just a limited number of files)
> or predicatable names because otherwise there is the same denial of
> service situation as before.
> 

Right - just that Benjamin's /tmp/scrollkeeper-tempfile.<login name>
pattern always matches just one file. 8-)

> Malcolm
> 

	Sander

	There are voices in the street,
	And the sound of running feet,
	And they whisper the word --
	Revolution!

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Dan M. <mu...@al...>]

On Wed, 2 Oct 2002, Sander Vesik wrote:

> On Thu, 3 Oct 2002, Malcolm Tredinnick wrote:
> 
> > On Wed, Oct 02, 2002 at 01:56:18PM +0100, Sander Vesik wrote:
> > > On Wed, 2 Oct 2002, Malcolm Tredinnick wrote:
> > > 
> > > > > As a temporary fix, isn't it possible to have only one temporary file by 
> > > > > user, suffixed by the login name?
> > > > > /tmp/scrollkeeper-tempfile.<login name>
> > > > 
> > > > That's not a bad idea. There may be some situation where more than one
> > > > file is required (I must admit to not having thought about why five was
> > > > chosen), but a per-user system using login name is something to work
> > > > with in any case. Kind of obvious, in retrospect. :-)
> > > > 
> > > 
> > > Please don't do this - it will give bad results if you are logged in into
> > > an account more than once and the machines have different databases.
> > 
> > No it won't, because you still have to allow multiple files, so
> > scrollkeeper will still return a filename specific for that run. You
> > can't just have a single file (or even just a limited number of files)
> > or predicatable names because otherwise there is the same denial of
> > service situation as before.
> > 
> 
> Right - just that Benjamin's /tmp/scrollkeeper-tempfile.<login name>
> pattern always matches just one file. 8-)

Hi all.  It sounds like a pretty good concensus has been reached that we 
should:
(1) For 0.3.x, insert the login name in the tempfile, while keeping the 
incremented # from 0 to 5.
(2) For 0.5.x and later, possibly abandon the use of temporary files all 
together.

Agreed?

For (2), I'd be interested to hear arguments for or against it.  We always
planned to turn ScrollKeeper into a proper library, but started with
scripts so that it would be simple and easy for people to use with help
browsers written in different languages or using different XML libraries.  
The script approach won't work when ScrollKeeper gets more complex, so the
scripts would eventually have to be replaced by one or more libraries.
Right now, I'm starting to wonder if using scripts and temporary files was
more trouble than it was worth.  Is there a value in keeping the scripts
and temporary files, or do people think we should turn ScrollKeeper
strictly into a C library which passes XML trees around?

-Dan

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[John F. <jf...@in...>]

On Tue, 2002-10-22 at 00:09, Dan Mueth wrote:

> 
> For (2), I'd be interested to hear arguments for or against it.  We always
> planned to turn ScrollKeeper into a proper library, but started with
> scripts so that it would be simple and easy for people to use with help
> browsers written in different languages or using different XML libraries.  
> The script approach won't work when ScrollKeeper gets more complex, so the
> scripts would eventually have to be replaced by one or more libraries.
> Right now, I'm starting to wonder if using scripts and temporary files was
> more trouble than it was worth.  Is there a value in keeping the scripts
> and temporary files, or do people think we should turn ScrollKeeper
> strictly into a C library which passes XML trees around?
> 

If we have the hacking resources (do we?) I'd like to see it turned into
a library.

Cheers,
John
-- 
John Fleck
jf...@in... (h) jf...@ab... (w)
http://www.inkstain.net http://www.abqjournal.com

"Sometimes, a diner is all about the mac and cheese." 
  - Zippy the Pinhead

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Malcolm T. <ma...@co...>]

On Tue, Oct 22, 2002 at 08:10:11PM -0600, John Fleck wrote:
> On Tue, 2002-10-22 at 00:09, Dan Mueth wrote:
> 
> > 
> > For (2), I'd be interested to hear arguments for or against it.  We always
> > planned to turn ScrollKeeper into a proper library, but started with
> > scripts so that it would be simple and easy for people to use with help
> > browsers written in different languages or using different XML libraries.  
> > The script approach won't work when ScrollKeeper gets more complex, so the
> > scripts would eventually have to be replaced by one or more libraries.
> > Right now, I'm starting to wonder if using scripts and temporary files was
> > more trouble than it was worth.  Is there a value in keeping the scripts
> > and temporary files, or do people think we should turn ScrollKeeper
> > strictly into a C library which passes XML trees around?
> > 
> 
> If we have the hacking resources (do we?) I'd like to see it turned into
> a library.

Scrollkeeper already has most of the infrastructure to be a library (it
even builds one correctly). Before going the whole way down that path, I
want to look at Mikael's comment about Yelp startup times. It sort of
surprises me that not having the files in /tmp makes it so slower, since
that is awfully close to just a file copy. Something is going on there
that I do no understand yet.

Malcolm

-- 
The cost of feathers has risen; even down is up!

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Mikael H. <mi...@co...>]

ons 2002-10-23 klockan 04.12 skrev Malcolm Tredinnick:
> On Tue, Oct 22, 2002 at 08:10:11PM -0600, John Fleck wrote:
> > On Tue, 2002-10-22 at 00:09, Dan Mueth wrote:
>
> > If we have the hacking resources (do we?) I'd like to see it turned into
> > a library.
> 
> Scrollkeeper already has most of the infrastructure to be a library (it
> even builds one correctly). Before going the whole way down that path, I
> want to look at Mikael's comment about Yelp startup times. It sort of
> surprises me that not having the files in /tmp makes it so slower, since
> that is awfully close to just a file copy. Something is going on there
> that I do no understand yet.

What I think it does it that it tries to merge several files into one
file. Or at least that is how it should do it (iirc).

If you run on Swedish it will merge the Swedish and the C files into one
file that uses the values from Swedish where it's found and the ones in
the C file otherwise. 

I think it would be nice to be able to tell it to _only_ use Swedish
even if that will lead to less docs.

Though this process should really be going quite fast, might just be
some bug somewhere making it slow.

Regards,
  Mikael Hallendal
-- 
Mikael Hallendal                mi...@co...
CodeFactory AB                  http://www.codefactory.se/
                                Cell: +46 (0)709 718 918

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Mikael H. <mi...@co...>]

ons 2002-10-02 klockan 10.51 skrev Benjamin Thery:

> Option (2) seems the cleanest to me too, but I don't know if the guys at 
> Yelp will like it ;-)

Hi!

Sorry for being late on replying.

Basically I'm not totally against it. Though I would have to cache the
result in Yelp anyway. Creating the xml-file put in /tmp takes a fair
amount of time (try removing them all, start yelp and then start yelp
again and you will notice a pretty large difference in startup time).

Since the stuff for doing these cache-files is already present in
Scrollkeeper I would recommend using this.

On a side note, Malcolm, do you have commit rights to the Scrollkeeper
CVS so that you can check in your patches and make a new release of
scrollkeeper?

Regards,
  Mikael Hallendal

-- 
Mikael Hallendal                mi...@co...
CodeFactory AB                  http://www.codefactory.se/
                                Cell: +46 (0)709 718 918

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Malcolm T. <ma...@co...>]

On Sun, Oct 20, 2002 at 12:13:52PM +0200, Mikael Hallendal wrote:
[...]
> On a side note, Malcolm, do you have commit rights to the Scrollkeeper
> CVS so that you can check in your patches and make a new release of
> scrollkeeper?

Yes, I do. I've been comitting various patches over the past few weeks
(but haven't got to this problem yet).

If Dan doesn't get back soon, I'll do an interim release just to get a
few of the recent changes out into public use.

Malcolm
-- 
Why be difficult when, with a little bit of effort, you could be
impossible.

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Sander V. <San...@Su...>]

On Wed, 2 Oct 2002, Malcolm Tredinnick wrote:

> Hi Benjamin,
> 
> (1) We use mkstemp to generate a unique filename each time (and include
> our own implementation for use on systems that don't have mkstemp).
> 
> 	Advantages:
> 		- Avoids the recent security problem (which is fixed in
> 		  CVS, btw).
> 		- Gets around the "maximum of five files" problem.
> 
> 	Disadvantages:
> 		- There is going to be a metric truckload of files with
> 		  stupid names floating around in some subdirectory of
> 		  the tmp directory after a while. In reality, these are
> 		  never going to be cleaned up except by something like
> 		  tmpwatch (which is not frequent enough on, say, Linux
> 		  systems).
> 

I think we should just declear cleaning these up the responsibility of
whatever app uses scrollkeeper 8-) 

> (2) The scrollkeeper-get-cl binary dumps the result to stdout, rather
> than saving to a file.
> 
> 	Advantages:
> 		- Never have to worry about stray files lying around.
> 		- Makes scrollkeeper easily multiuser without having to
> 		  sweat on a lot of the locking issues all the time.
> 
> 	Disadvantages:
> 		- Breaks backwards compatibility. All applications
> 		  currently using scrollkeeper via scrolkeeper-get-cl
> 		  and friends.
> 
> Although option (2) seems clearly preferable to me, the backwards
> compatibility issues means it is probably an 0.5 feature. This means we
> may have to go with the "dozens of stupid temporary" files in the
> 0.3/0.4 timeframe unless somebody can come up with a better idea.
> 

this would be preferable, but really, normal client apps should just link
to a scrollkeeper shared library and avoid the 'dumping out the tree'
process with it all just hapenning in memory and not touching disk at all.
At least in principle, it should be possible to run in a totaly read-only
mode...

> Anybody have thoughts?
> 
> Malcolm
> 

	Sander

	There are voices in the street,
	And the sound of running feet,
	And they whisper the word --
	Revolution!

Re: [Scrollkeeper-devel] Porblem with temporary files creation

[Sander V. <San...@Su...>]

On Wed, 2 Oct 2002, Benjamin Thery wrote:

> -- Temporary files can be :
>     /tmp/scrollkeeper-tempfile.#,
>     where # is a number between 0 and 4.
> 
> -- In /tmp, all the five possible temporary files already existed and 
> were owned by another user.
>     On the same system we have about 4 or 5 GNOME 2 installations running.
>  

The sunray server here that has gnome2 (i'm not sure why we don't have it
on all of them) has normaly ~ 20 active gnome2 logins. yelp/scrollkeeper
don't really work on it as it stands 8-(

> -- scrollkeeper-get-content-list couldn't write its file but didn't 
> complain, because the other user owned all the temporary files.
> 
> -- Yelp parsed on of the file created by the other user which did not 
> contained all the entries.
> 
> -- As a result, I can access only the docs installed by the other user.
> 
> 
> I think the function get_next_free_temp_file_path() in cl/get-cl.c has a 
> problem.
> 
> -- Either, it must create the temporary file with write permission for 
> everyone,

This could be a huge & bad security problem.

> -- Or, the procedure to select a filename as to be changed (may be by 
> using a random file name or adding
> the username or UID to the temporary filename)
> 

This would be preferable - also yelp (and similar client programs) should
probably delete the fiel handed to them after they are done with it...

> 
> 
> -- 
> B e n j a m i n   T h e r y  - BULL/BI&S/GeODE/AIX & Linux CC
> 
>  mailto:Ben...@bu...
>    http://www.bull.com
> 

	Sander

	There are voices in the street,
	And the sound of running feet,
	And they whisper the word --
	Revolution!