Re: [HACKERS] pg_dump -Ft failed on Windows XP
Someday we can move this to /port, but for now, let's get it into CVS. Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches It will be applied as soon as one of the PostgreSQL committers reviews and approves it. --- Zeugswetter Andreas DCP SD wrote: Apparently it won't work at all if TMP isn't set? I'm not *too* concerned about that, since TMP is normally set by the OS itself. There's one set in the system environment (to c:\windows\temp or whatrever) and then it's overridden by one set by the OS when it loads a user profile. OK, then maybe not having it would be equivalent to /tmp-not-writable on Unix, ie, admin error. Also to the point, what would you fall back to? Current directory maybe? It tries \ (tested on Win 2000), if the dir argument is NULL and TMP is not set. But TMP is usually set. Attached is a working version not yet adapted to port/. - memoryleak fixed - use _tmpname and _fdopen not the compatibility tmpname and fdopen (imho only cosmetic) - EACCES fixed (Win2000 needs _S_IREAD | _S_IWRITE or fails with EACCES, even as Admin) - I suggest adding a prefix pg_temp_ (for leftover temp files after crash, the name I get is then usually pg_temp_2) Andreas Content-Description: pg_dump_tempfile.patch.txt [ Attachment, skipping... ] ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Apparently it won't work at all if TMP isn't set?
I'm not *too* concerned about that, since TMP is normally set by the
OS
itself. There's one set in the system environment (to
c:\windows\temp
or whatrever) and then it's overridden by one set by the OS when it
loads a user profile.
OK, then maybe not having it would be equivalent to /tmp-not-writable
on Unix, ie, admin error.
Also to the point, what would you fall back to?
Current directory maybe?
It tries \ (tested on Win 2000), if the dir argument is NULL and TMP is
not set.
But TMP is usually set.
Attached is a working version not yet adapted to port/.
- memoryleak fixed
- use _tmpname and _fdopen not the compatibility tmpname and fdopen
(imho only cosmetic)
- EACCES fixed (Win2000 needs _S_IREAD | _S_IWRITE or fails with EACCES,
even as Admin)
- I suggest adding a prefix pg_temp_ (for leftover temp files after
crash,
the name I get is then usually pg_temp_2)
Andreas
Index: bin/pg_dump/pg_backup_tar.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_backup_tar.c,v
retrieving revision 1.50
diff -c -r1.50 pg_backup_tar.c
*** bin/pg_dump/pg_backup_tar.c 12 Feb 2006 06:11:50 - 1.50
--- bin/pg_dump/pg_backup_tar.c 21 Apr 2006 09:22:00 -
***
*** 362,368
--- 362,388
{
tm = calloc(1, sizeof(TAR_MEMBER));
+ #ifndef WIN32
tm-tmpFH = tmpfile();
+ #else
+ /* on win32, tmpfile() generates a filename in the root
directory, which requires
+* administrative permissions to write to. */
+ while (1)
+ {
+ char *tmpname;
+ int fd;
+
+ tmpname = _tempnam(NULL, pg_temp_);
+ if (tmpname == NULL)
+ break;
+ fd = _open(tmpname, _O_RDWR | _O_CREAT | _O_EXCL |
_O_BINARY | _O_TEMPORARY, _S_IREAD | _S_IWRITE);
+ free(tmpname);
+ if (fd == -1 errno == EEXIST)
+ continue; /* Try again with a new name if file
exists */
+ if (fd != -1)
+ tm-tmpFH = _fdopen(fd, w+b);
+ break;
+ }
+ #endif
if (tm-tmpFH == NULL)
die_horribly(AH, modulename, could not generate
temporary file name: %s\n, strerror(errno));
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] pg_dump -Ft failed on Windows XP
I got the following message when I ran pg_dump with -Ft option on Windows XP. pg_dump -V pg_dump (PostgreSQL) 8.1.2 pg_dump -Ft test C:\backup\xxx.out pg_dump: [tar archiver] could not generate temporary file name: Permission denied pg_dump calls tmpfile() in pg_backup_tar.c:tarOpen(). Win32's tmpfile() creates the file into root folder. But non-administrator users can't create files into root folder. So, I think it fails that non-administrator users run pg_dump with -Ft option. Indeed, that's definitly a bug. Quick patch attached. It does appear to work, but there may be a better way? //Magnus pg_dump_tempfile.patch Description: pg_dump_tempfile.patch ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Am Donnerstag, 20. April 2006 10:47 schrieb Magnus Hagander: Indeed, that's definitly a bug. Quick patch attached. It does appear to work, but there may be a better way? This patch introduces a security hole because an attacker could create, say, a suitable symlink between the time the name is generated and the file is opened. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Indeed, that's definitly a bug. Quick patch attached. It does appear to work, but there may be a better way? This patch introduces a security hole because an attacker could create, say, a suitable symlink between the time the name is generated and the file is opened. Good point. I guess what I need to do is use open() specifying O_CREATE, and then fdopen() that file. Question: Is the use of O_TEMPORARY to open() portable? (my win32 docs say it will make the file automatically deleted when the last descriptor is closed, which I didn't know before. That would make the patch much simpler, but might require #ifdefs?) //Magnus ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Indeed, that's definitly a bug. Quick patch attached. It does appear to work, but there may be a better way? This patch introduces a security hole because an attacker could create, say, a suitable symlink between the time the name is generated and the file is opened. Good point. I guess what I need to do is use open() specifying O_CREATE, and then fdopen() that file. Question: Is the use of O_TEMPORARY to open() portable? (my win32 docs say it will make the file automatically deleted when the last descriptor is closed, which I didn't know before. That would make the patch much simpler, but might require #ifdefs?) Actually, since I'm running out the door, here is a new attempt that changes behaviour only on win32. And that also appears to work, but may be wrong ;-) //Magnus pg_dump_tempfile.patch Description: pg_dump_tempfile.patch ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Am Donnerstag, 20. April 2006 13:03 schrieb Magnus Hagander: Question: Is the use of O_TEMPORARY to open() portable? (my win32 docs say it will make the file automatically deleted when the last descriptor is closed, which I didn't know before. That would make the patch much simpler, but might require #ifdefs?) I think it would be more elegant if you wrote a replacement implementation of tmpfile() for pgport and did not change pg_dump at all. And/or write a bug to Microsoft about a buggy C library. :) -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Question: Is the use of O_TEMPORARY to open() portable? (my win32 docs say it will make the file automatically deleted when the last descriptor is closed, which I didn't know before. That would make the patch much simpler, but might require #ifdefs?) I think it would be more elegant if you wrote a replacement implementation of tmpfile() for pgport and did not change pg_dump at all. And/or write a bug to Microsoft about a buggy C library. :) It's not buggy. It's well documented behaviour,and per my linux manpage for the file it's also OK per spec: The standard does not specify the directory that tmpfile() will use. Glibc will try the path prefix P_tmpdir defined in stdio.h, and if that fails the directory /tmp. It's certainly *stupid*, but not buggy ;-) //Magnus ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Am Donnerstag, 20. April 2006 13:21 schrieb Magnus Hagander: It's not buggy. It's well documented behaviour,and per my linux manpage for the file it's also OK per spec: The standard does not specify the directory that tmpfile() will use. Glibc will try the path prefix P_tmpdir defined in stdio.h, and if that fails the directory /tmp. The spec says The tmpfile() function shall create a temporary file and open a corresponding stream. The file shall be automatically deleted when all references to the file are closed. The file is opened as in fopen() for update (w+). If the implementation is such that it tries to create the file in a directory that the user does not have write permission to, it's a bug. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] pg_dump -Ft failed on Windows XP
It's not buggy. It's well documented behaviour,and per my linux manpage for the file it's also OK per spec: The standard does not specify the directory that tmpfile() will use. Glibc will try the path prefix P_tmpdir defined in stdio.h, and if that fails the directory /tmp. The spec says The tmpfile() function shall create a temporary file and open a corresponding stream. The file shall be automatically deleted when all references to the file are closed. The file is opened as in fopen() for update (w+). If the implementation is such that it tries to create the file in a directory that the user does not have write permission to, it's a bug. Well, you're never gonig to convince MS of that :-) And either way, the runtime we're usnig now isn't especially current (MSVC6), so even in the unlikely event that they did fix it, it wouldn't help us. So we'll definitly need to fix it ourselves. Did the code I sent the last time look reasonable? //Magnus ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Magnus Hagander [EMAIL PROTECTED] writes: Win32's tmpfile() creates the file into root folder. But non-administrator users can't create files into root folder. In other words, tmpfile() doesn't work at all on Win32? Seems like the appropriate place to be filing a bug report is at microsoft.com. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Win32's tmpfile() creates the file into root folder. But non-administrator users can't create files into root folder. In other words, tmpfile() doesn't work at all on Win32? Seems like the appropriate place to be filing a bug report is at microsoft.com. If works if you're an administrator or power user. Sure, we can file a report, but if they bother to fix it, that will be for Visual Studio 2005 (if we're lucky, more likely for the next version after that). So we can forget all about mingw then, since it uses an old version of the runtime... Though I doubt they'd fix it anyway, since their documentation alreadyu tells you what to do to get around it. //Magnus ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Peter Eisentraut [EMAIL PROTECTED] writes: If the implementation is such that it tries to create the file in a directory that the user does not have write permission to, it's a bug. Well, I think it would be a valid implementation on Unix to always try to create the file in /tmp, which'd likely fail if someone had revoked world write on /tmp --- but doing the latter is administrator error, not a library fault. OTOH, if / is *supposed* to be non world writable on Win32, then trying to create temp files there indicates a seriously brain-damaged library. It should be trying to create the file in a place where the user is expected to have permission to do it. Has anyone looked to see with tmpfile() actually does though? I'm a bit surprised that it doesn't create stuff in the same directory as tmpnam(). I wonder if Magnus and Yoshiyuki are testing under different conditions. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] pg_dump -Ft failed on Windows XP
From: Tom Lane [EMAIL PROTECTED] Subject: Re: [HACKERS] pg_dump -Ft failed on Windows XP Date: Thu, 20 Apr 2006 10:00:48 -0400 Magnus Hagander [EMAIL PROTECTED] writes: Win32's tmpfile() creates the file into root folder. But non-administrator users can't create files into root folder. In other words, tmpfile() doesn't work at all on Win32? Seems like the appropriate place to be filing a bug report is at microsoft.com. Yes. There is the description of Win32's tmpfile() at the following URL. http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vclib/html/_crt_tmpfile.asp -- Yoshiyuki Asaba [EMAIL PROTECTED] ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] pg_dump -Ft failed on Windows XP
If the implementation is such that it tries to create the file in a directory that the user does not have write permission to, it's a bug. Well, I think it would be a valid implementation on Unix to always try to create the file in /tmp, which'd likely fail if someone had revoked world write on /tmp --- but doing the latter is administrator error, not a library fault. OTOH, if / is *supposed* to be non world writable on Win32, then trying to create temp files there indicates a seriously brain-damaged library. It should be trying to create the file in a place where the user is expected to have permission to do it. Prior to Windows XP, users had write permissions in the root IIRC. They definitly had in NT4, but I think they did in 2000 as well. Has anyone looked to see with tmpfile() actually does though? I'm a bit surprised that it doesn't create stuff in the same directory as tmpnam(). I wonder if Magnus and Yoshiyuki are testing under different conditions. I have repeated the problem with CVS head on XP SP2. It *does* create it there (or rather, it tries to). tmpnam() returns a file in the current dir per documentation, but I see it generating one in the root instead. tempnam() uses TMP environment variable. tmpfile() and tmpnam() both use the same function to generate the filename. //Magnus ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Magnus Hagander [EMAIL PROTECTED] writes: I have repeated the problem with CVS head on XP SP2. It *does* create it there (or rather, it tries to). tmpnam() returns a file in the current dir per documentation, but I see it generating one in the root instead. tempnam() uses TMP environment variable. tmpfile() and tmpnam() both use the same function to generate the filename. Hm. I guess I concur with Peter's conclusion: the cleanest fix is to put our own implementation of tmpfile() into src/port/. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] pg_dump -Ft failed on Windows XP
I have repeated the problem with CVS head on XP SP2. It *does* create it there (or rather, it tries to). tmpnam() returns a file in the current dir per documentation, but I see it generating one in the root instead. tempnam() uses TMP environment variable. tmpfile() and tmpnam() both use the same function to generate the filename. Hm. I guess I concur with Peter's conclusion: the cleanest fix is to put our own implementation of tmpfile() into src/port/. Ok. Should be easy enough once the code is fine - can you comment on the patch as sent, if the code itself looks right provided i wrap it up in a function in port/? //Magnus ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Magnus Hagander [EMAIL PROTECTED] writes: Ok. Should be easy enough once the code is fine - can you comment on the patch as sent, if the code itself looks right provided i wrap it up in a function in port/? Not sure if the error handling is adequate --- are there any cases besides EEXIST that should loop? A look at http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vclib/html/_crt__tempnam.2c_._wtempnam.2c_.tmpnam.2c_._wtmpnam.asp suggests that tempnam() is also pretty fragile, esp. if you're passing NULLs. Apparently it won't work at all if TMP isn't set? regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Ok. Should be easy enough once the code is fine - can you comment on the patch as sent, if the code itself looks right provided i wrap it up in a function in port/? Not sure if the error handling is adequate --- are there any cases besides EEXIST that should loop? Well, per docs I can get: EACCES (will certainly fail again since we're still working the same directory) EEXIST (covered) EINVAL = invalid oflag or pmode, which should never happen EMFILE = too many open files (that would be the one, but I doubd it'd help that much) ENOENT = path ont found (would happen agai non retry) A look at http://msdn.microsoft.com/library/default.asp?url=/library/en- us/vclib/html/_crt__tempnam.2c_._wtempnam.2c_.tmpnam.2c_._wtmpnam.asp suggests that tempnam() is also pretty fragile, esp. if you're passing NULLs. Apparently it won't work at all if TMP isn't set? I'm not *too* concerned about that, since TMP is normally set by the OS itself. There's one set in the system environment (to c:\windows\temp or whatrever) and then it's overridden by one set by the OS when it loads a user profile. Also to the point, what would you fall back to? //Magnus ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] pg_dump -Ft failed on Windows XP
Magnus Hagander [EMAIL PROTECTED] writes: Apparently it won't work at all if TMP isn't set? I'm not *too* concerned about that, since TMP is normally set by the OS itself. There's one set in the system environment (to c:\windows\temp or whatrever) and then it's overridden by one set by the OS when it loads a user profile. OK, then maybe not having it would be equivalent to /tmp-not-writable on Unix, ie, admin error. Also to the point, what would you fall back to? Current directory maybe? Actually, the thing that struck me about the man page was that first it said current dir was a fallback, and then it said it wasn't. Is this just typical Microsoft brain damage, or is there a reason to avoid using current dir for this? regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
