> I'm sorry Juliusz, but I have send you patch with ask to test it.

Yes, and I acknowledge that I didn't reply speedily enough.  I was
generally feeling uneasy about your patch, but didn't have time to
think about why.

The recent OpenSSL stuff was what I needed to take the time to think
about it.

>> 3. You changed Polipo's behaviour without documenting it

> Please see Debian changelog.

Inserting an entry in the changelog is not documentation.

>> binary.  This will confuse your users and will confuse your friendly
>> upstream when he tries to help your users with debugging.

> In commonly way users must contact with package maintainer or by
> mailing lists, not private.

Judging from the contents of my mailbox, this is not entirely true.

For some historical context, please have a look at

  http://www.dcs.ed.ac.uk/home/jec/programs/xfsft/

which is a page written by me in the mid-90s.  At the time, I was
receiving litterally hundreds of mails from Redhat users complaining
that the Redhat version didn't behave according to the docs.

>> What is more, it will create a rather glaring security hole for anyone
>> who replaces his Debian binary with an upstream binary (which is
>> something people sometimes do, when they need a more recent version).

> It is not a Debian way, sorry.

Which does not mean people don't do it.

>> Your patch manipulates the process' *user* mask, which must never be
>> manipulated by a program.  The umask is a global process feature.

> It's changed only for one, fopen() function, and then changed back.
> What is wrong?

Suppose I decide to perform expiry of the cache in a low-priority
thread; then the umask that the thread gets changes over time.

Suppose a user sets his umask to 077, in order to make sure files are
not group-readable.  Then your patch will cause the log file to be
group readable.

Since the umask is historically the *user* mask, this counts as
unexpected behaviour.

>> The proper way to do what you need is to use open(O_CREAT) with the
>> right permissions (but obeying the umask), then pass the file
>> descriptor to fdopen.  Of course, the permissions should be
>> configurable by a config variable.
>
> Well, this looks some better. Will you apply this in the next version
> or I may do another patch?

I'll be glad to apply a patch to do that to the upstream code.  In the
meantime, please revert your patch.

                                        Juliusz

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft 
Defy all challenges. Microsoft(R) Visual Studio 2008. 
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Polipo-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/polipo-users

Reply via email to