> > All regression tests pass with this case and no ABI or source > > incompatabilities are introduced. > > You're still playing extremely fast and loose with that claim of "no > incompatibilities introduced". This may not *directly* break an > application that uses PQsetNoticeProcessor, but it breaks the purpose > that the app is using PQsetNoticeProcessor for, namely to intercept > notices. If it doesn't also override the fnoticeProc then it hasn't > intercepted all the notices.
Those notices were sent to setderr earlier, they were already broken: at least now there's a ghost of a chance at the app at picking up those errors. > What I was envisioning was a simple little subroutine that takes a > format string and some args, formats into a work buffer, and then > passes the formatted string to the existing notice hook. > Essentially, moving the five lines you're unhappy about into a > common subroutine. This gets the job done without *any* > modification of the application API. *nods* > A bigger problem with the whole approach is that there's no way for > the application to install a notice processor before > PasswordFromFile is invoked, because that occurs during PQconnectdb. > So any notices emitted by same will necessarily fall into the lap of > the default notice processor. Which is why I thought fprinting to stderr isn't that bad of an idea given there are only two instances inside of PasswordFromFile that make use of the noticeProc. :) > I didn't hear any strong objection to the idea of treating the > weak-protection complaint as a hard error (connection failure), so > we can fix the already-existing problem by doing that. libpq's a library used by many many applications ... do you envison calling abort(), exit() or _exit()? Seems really broken to halt the app based on FS permissions. Imagine an ISP web based environment with PHP using libpq to connect to PostgreSQL and an insecure .pgpass file. Issuing a warning should be plenty sufficient. Since this is a case of "mother knows best," IMHO, it'd probably be wise to only print these warnings if a debugging variable was set inside of libpq. > As for the issue of whether to complain about a file that exists but > isn't readable, Peter exhibited lots of precedent for not doing so. > I'm inclined to think we should drop that part. *shrug* Whatever the consensus is, raising awareness isn't a bad thing, halting execution can be fatal. The app that carries the most weight in terms of precedent with me is gnupg because gnupg intentionally deals with secure content/passwords. Here are the perm checks that gnupg does: *) make sure the homedir, .gnupg dir, and libdir are owned by the user/root *) make sure the homedir, .gnupg dir, and libdir are "safe" for the user/root It's not extensive, but, in the event of a problem, it only prints to stderr and it doesn't bomb out (gnupg-1.2.2/g10/g10.c:885). With GnuPG as the extreme in terms of security and its list of checks a bit more extensive (though not exhaustive), bombing out seems like a bad idea. Here's another case to not bomb out. An admin in a php web env types: % vi ~www/.pgpass [enters in a .pgpass file] % chown www ~www/.pgpass % chmod 0600 ~www/.pgpass Game over. During the period of time from when the admin writes the file to chmod 0600's the file, all database requests will be denied... which could be substantial in a web environment, and bad for PostgreSQL's image, esp with the talk of MySQL alienating its users and Pg trying to pick up steam in the PHP community. -sc PS I really will lay off the topic, just trying to make a good case for a usable and secure library. :) -- Sean Chittenden ---------------------------(end of broadcast)--------------------------- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match