Re: [pgadmin-hackers] Patch : PGPASSFILE fix
I think, we need to agree what exactly solution should be. About creating
parent directories.It's going to complicate solution, path can be of any depth.
i.e. /a/b/c/d/e/.pgpass, and none of these folders could present. Are we going
to keep on creating all folders ?
regards,
Prasad
Sent: Tuesday, March 10, 2015 at 7:09 AM
From: "Ashesh Vashi" ut
To: Prasad
Cc: "Dave Page" , pgadmin-hackers
Subject: Re: [pgadmin-hackers] Patch : PGPASSFILE fix
Hi Prasad,
On Thu, Mar 5, 2015 at 4:20 AM, Prasad wrote:
Hi,
As mentioned in my earlier communication code calling this function is checking
for file existence. So if we decide to add code for creation of full path, then
similar code has to be removed from location of call to this function.
Otherwise, it will end up with multiple error messages. It's wxWidget's wxFile
that throws error.
So, I've created two patches, and we can go with one of them.
1. Let GetConfigFile function just read value from PGPASSFILE and return as it
is as like, similar to way it creates default path(It doesn't create file in
case of default path as well). And calling functions are taking care of path
validation and error messages.
This won't work.
We should create the file, if it does not exists (and, the path).2. Let
GetConfigFile function read value from PGPASSFILE and create file path ,it will
show error message in case it can't. In this case calling code only should
check existence of file before going ahead, and not try to create or read file,
otherwise , user will end up with multiple message boxes with same error.
The patch, you shared, do not create the path (parent directories) for the
PGPASSFILE (if it does not exists).
You're only creating the file, which is not right.
NOTE:
Please do not mix tabs and spaces in your patch.
I am still not able to apply the patch using 'git apply' utility.
--
Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company[http://www.enterprisedb.com]
http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi]
regards,
Prasad
Sent: Wednesday, March 04, 2015 at 11:35 AM
From: "Ashesh Vashi"
, func
To: "Dave Page"
Cc: Prasad , pgadmin-hackers
Subject: Re: [pgadmin-hackers] Patch : PGPASSFILE fix
On Wed, Mar 4, 2015 at 4:40 PM, Dave Page
wrote:
On Wed, Mar 4, 2015 at 11:06 AM, Ashesh Vashi
wrote:
On Wed, Mar 4, 2015 at 4:09 PM, Dave Page
wrote:
I think we should try to create the full path if necessary, and simply
throw an error if we can't.
And, I think - we should switch back to default pgpass configuration file.
No, because that's a security risk (writing the password to a file that wasn't
what the user intended).
Agree.
--
Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL
Company[http://www.enterprisedb.com[http://www.enterprisedb.com]]
http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi][http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi]]
--
Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL
Company[http://www.enterprisedb.com[http://www.enterprisedb.com]]
http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi][http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi]]
On Wed, Mar 4, 2015 at 10:01 AM, Prasad
wrote:
> Alright , I'll revert to PGPASS check.
> Existing function only creates folder containing file. With this case, whats
> expected ? Reading value in PGPASSFILE and try to create folder containing
> pgpass file (Assuming it's valid path)? Remember, it's environment variable.
> User can specify anything in there. Some garbage value as well. If we don't
> do any validation there, user will automatically see error with complain
> about file ?
>
> thanks and regards,
> Prasad
>
>
> Sent: Wednesday, March 04, 2015 at 7:48 AM
> From: "Ashesh Vashi"
>
> To: Prasad
>
> Cc: pgadmin-hackers
>
> Subject: Re: [pgadmin-hackers] Patch : PGPASSFILE fix
>
> On Wed, Mar 4, 2015 at 8:44 AM, Prasad
>
> wrote:
>
> Ashesh,
>
> Thanks for reviewing patch,
> Code I have removed in I think, was switch statement inside if condition,
> which doesn't make sense.
> ie.
> if (var == 2)
> {
> switch (var)
> case 2:
> .
> break;
> }
>
> that's why I removed it, because it's redundant.
> Agree about redundancy, but you've also removed the code for checking the
> PGPASS check at the start of the function.
> i.e.
> @@ -762,35 +762,33 @@ void sysSettings::SetCanonicalLanguage(const wxLanguage
> &lang)
> //
> wxString sysSettings::GetConfigFile(configFileName cfgname)
> {
> - if (cfgname == PGPASS)
> - {
>
> I am not agree with that.
> About creation of directory, I'm not sure if this validation is required.
>Existing code creates directory postgresql (only on windows) ac
[pgadmin-hackers] TODO list
Hi, How up to date is this list ? http://www.pgadmin.org/development/todo.php Had cursory glance, and second item in Essentials appear to be already done. regards, Prasad -- Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers
Re: [pgadmin-hackers] Patch : PGPASSFILE fix
On Wed, Mar 11, 2015 at 1:06 PM, Prasad wrote: > I think, we need to agree what exactly solution should be. About creating > parent directories.It's going to complicate solution, path can be of any > depth. i.e. /a/b/c/d/e/.pgpass, and none of these folders could present. > Are we going to keep on creating all folders ? > Agree - it's going to be complicated. Dave? > > regards, > Prasad > > > > Sent: Tuesday, March 10, 2015 at 7:09 AM > From: "Ashesh Vashi" ut > To: Prasad > Cc: "Dave Page" , pgadmin-hackers < > pgadmin-hackers@postgresql.org> > Subject: Re: [pgadmin-hackers] Patch : PGPASSFILE fix > > Hi Prasad, > > On Thu, Mar 5, 2015 at 4:20 AM, Prasad wrote: > > Hi, > > As mentioned in my earlier communication code calling this function is > checking for file existence. So if we decide to add code for creation of > full path, then similar code has to be removed from location of call to > this function. Otherwise, it will end up with multiple error messages. It's > wxWidget's wxFile that throws error. > > So, I've created two patches, and we can go with one of them. > 1. Let GetConfigFile function just read value from PGPASSFILE and return > as it is as like, similar to way it creates default path(It doesn't create > file in case of default path as well). And calling functions are taking > care of path validation and error messages. > This won't work. > We should create the file, if it does not exists (and, the path).2. Let > GetConfigFile function read value from PGPASSFILE and create file path ,it > will show error message in case it can't. In this case calling code only > should check existence of file before going ahead, and not try to create or > read file, otherwise , user will end up with multiple message boxes with > same error. > The patch, you shared, do not create the path (parent directories) for the > PGPASSFILE (if it does not exists). > You're only creating the file, which is not right. > > NOTE: > Please do not mix tabs and spaces in your patch. > I am still not able to apply the patch using 'git apply' utility. > > > -- > Thanks & Regards, > > Ashesh Vashi > EnterpriseDB INDIA: Enterprise PostgreSQL Company[ > http://www.enterprisedb.com] > > > http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi] > regards, > Prasad > > > > > > Sent: Wednesday, March 04, 2015 at 11:35 AM > From: "Ashesh Vashi" ashesh.va...@enterprisedb.com]>, func > To: "Dave Page" > Cc: Prasad , pgadmin-hackers < > pgadmin-hackers@postgresql.org[pgadmin-hackers@postgresql.org]> > Subject: Re: [pgadmin-hackers] Patch : PGPASSFILE fix > > On Wed, Mar 4, 2015 at 4:40 PM, Dave Page dp...@pgadmin.org]> wrote: > > > > On Wed, Mar 4, 2015 at 11:06 AM, Ashesh Vashi < > ashesh.va...@enterprisedb.com[ashesh.va...@enterprisedb.com][ > ashesh.va...@enterprisedb.com[ashesh.va...@enterprisedb.com]]> wrote: > On Wed, Mar 4, 2015 at 4:09 PM, Dave Page dp...@pgadmin.org][dp...@pgadmin.org[dp...@pgadmin.org]]> wrote: > > I think we should try to create the full path if necessary, and simply > throw an error if we can't. > And, I think - we should switch back to default pgpass configuration file. > > No, because that's a security risk (writing the password to a file that > wasn't what the user intended). > Agree. > > -- > Thanks & Regards, > > Ashesh Vashi > EnterpriseDB INDIA: Enterprise PostgreSQL Company[ > http://www.enterprisedb.com[http://www.enterprisedb.com]] > > > http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi][http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi]] > > > > > -- > Thanks & Regards, > > Ashesh Vashi > EnterpriseDB INDIA: Enterprise PostgreSQL Company[ > http://www.enterprisedb.com[http://www.enterprisedb.com]] > > > http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi][http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi]] > > On Wed, Mar 4, 2015 at 10:01 AM, Prasad prasa...@mail.com][prasa...@mail.com[prasa...@mail.com]]> wrote: > > Alright , I'll revert to PGPASS check. > > Existing function only creates folder containing file. With this case, > whats expected ? Reading value in PGPASSFILE and try to create folder > containing pgpass file (Assuming it's valid path)? Remember, it's > environment variable. User can specify anything in there. Some garbage > value as well. If we don't do any validation there, user will automatically > see error with complain about file ? > > > > thanks and regards, > > Prasad > > > > > > Sent: Wednesday, March 04, 2015 at 7:48 AM > > From: "Ashesh Vashi" ashesh.va...@enterprisedb.com][ashesh.va...@enterprisedb.com[ > ashesh.va...@enterprisedb.com]]> > > To: Prasad prasa...@mail.com]]> > > Cc: pgadmin-hackers pgadmin-hackers@postgresql.org][pgadmin-hackers@postgresql.org[ > pgadmin-hackers@postgresql.org]]> > > Subject: Re: [pgadmin-hackers] Patch : PGPASSFILE fix > > > > On Wed, Mar 4, 2015 at 8:44 AM, Prasad prasa...@mail.com][prasa...@
Re: [pgadmin-hackers] TODO list
2015-03-11 8:40 GMT+01:00 Prasad : > Hi, > > How up to date is this list ? > > Let's say it could use some updates :) > http://www.pgadmin.org/development/todo.php > > Had cursory glance, and second item in Essentials appear to be already > done. > > -- Guillaume. http://blog.guillaume.lelarge.info http://www.dalibo.com
Re: [pgadmin-hackers] TODO list
On Wed, Mar 11, 2015 at 7:40 AM, Prasad wrote: > Hi, > > How up to date is this list ? > > http://www.pgadmin.org/development/todo.php > > Had cursory glance, and second item in Essentials appear to be already done. I don't think it is done. We check against what we connected with, but not the database itself, and the password could have changed in the meantime. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers
[pgadmin-hackers] pgAdmin III commit: Minor doc build fix for Windows.
Minor doc build fix for Windows. Branch -- REL-1_20_0_PATCHES Details --- http://git.postgresql.org/gitweb?p=pgadmin3.git;a=commitdiff;h=b5ebe225a34544b63648de79c9400f8a8e0df9f0 Author: Sandeep Thakkar Modified Files -- docs/builddocs.bat |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers
[pgadmin-hackers] pgAdmin III commit: Minor doc build fix for Windows.
Minor doc build fix for Windows. Branch -- master Details --- http://git.postgresql.org/gitweb?p=pgadmin3.git;a=commitdiff;h=7e6944d13d395cd642243b885b9c88809c675be1 Author: Sandeep Thakkar Modified Files -- docs/builddocs.bat |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers
Re: [pgadmin-hackers] Patch : PGPASSFILE fix
Yes, it's not difficult. Just complicates things, wxWidgets has function to create directories in recursive fashion. but if we use it, we'll loose control about handling error condition and track of newly created folders. API returns if it's succesful ? But will not help with deletion in case of failure. Option will be to use source for that API as it is. regards, Prasad Sent: Wednesday, March 11, 2015 at 2:32 PM From: "Dave Page" To: "Ashesh Vashi" Cc: Prasad , pgadmin-hackers Subject: Re: [pgadmin-hackers] Peatch : PGPASSFILE fix On Wed, Mar 11, 2015 at 7:42 AM, Ashesh Vashi wrote: > On Wed, Mar 11, 2015 at 1:06 PM, Prasad wrote: >> >> I think, we need to agree what exactly solution should be. About creating >> parent directories.It's going to complicate solution, path can be of any >> depth. i.e. /a/b/c/d/e/.pgpass, and none of these folders could present. Are >> we going to keep on creating all folders ? h > > Agree - it's going to be complicated. It's not that hard - see http://nion.modprobe.de/blog/archives/357-Recursive-directory-creation.html for example. wx should make that even easier I expect. The only unhandled issue is what to do if we get an error on any of the directories. I would suggest just keeping an array of what we actually create, and removing any created prior to the error so we return the users filesystem to its original state. >> >> regards, >> Prasad >> >> >> >> Sent: Tuesday, March 10, 2015 at 7:09 AM >> From: "Ashesh Vashi" ut >> To: Prasad >> Cc: "Dave Page" , pgadmin-hackers >> >> Subject: Re: [pgadmin-hackers] Patch : PGPASSFILE fix >> >> Hi Prasad, >> >> On Thu, Mar 5, 2015 at 4:20 AM, Prasad wrote: >> >> Hi, >> >> As mentioned in my earlier communication code calling this function is >> checking for file existence. So if we decide to add code for creation of >> full path, then similar code has to be removed from location of call to this >> function. Otherwise, it will end up with multiple error messages. It's >> wxWidget's wxFile that throws error. >> >> So, I've created two patches, and we can go with one of them. >> 1. Let GetConfigFile function just read value from PGPASSFILE and return >> as it is as like, similar to way it creates default path(It doesn't create >> file in case of default path as well). And calling functions are taking care >> of path validation and error messages. >> This won't work. >> We should create the file, if it does not exists (and, the path).2. Let >> GetConfigFile function read value from PGPASSFILE and create file path ,it >> will show error message in case it can't. In this case calling code only >> should check existence of file before going ahead, and not try to create or >> read file, otherwise , user will end up with multiple message boxes with >> same error. >> The patch, you shared, do not create the path (parent directories) for the >> PGPASSFILE (if it does not exists). >> You're only creating the file, which is not right. >> >> NOTE: >> Please do not mix tabs and spaces in your patch. >> I am still not able to apply the patch using 'git apply' utility. >> >> >> -- >> Thanks & Regards, >> >> Ashesh Vashi >> EnterpriseDB INDIA: Enterprise PostgreSQL >> Company[http://www.enterprisedb.com[http://www.enterprisedb.com]] >> >> >> http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi][http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi]] >> regards, >> Prasad >> >> >> >> >> >> Sent: Wednesday, March 04, 2015 at 11:35 AM >> From: "Ashesh Vashi" >> , func >> To: "Dave Page" >> Cc: Prasad , pgadmin-hackers >> >> Subject: Re: [pgadmin-hackers] Patch : PGPASSFILE fix >> >> On Wed, Mar 4, 2015 at 4:40 PM, Dave Page >> wrote: >> >> >> >> On Wed, Mar 4, 2015 at 11:06 AM, Ashesh Vashi >> >> wrote: >> On Wed, Mar 4, 2015 at 4:09 PM, Dave Page >> >> wrote: >> >> I think we should try to create the full path if necessary, and simply >> throw an error if we can't. >> And, I think - we should switch back to default pgpass configuration >> file. >> >> No, because that's a security risk (writing the password to a file that >> wasn't what the user intended). >> Agree. >> >> -- >> Thanks & Regards, >> >> Ashesh Vashi >> EnterpriseDB INDIA: Enterprise PostgreSQL >> Company[http://www.enterprisedb.com[http://www.enterprisedb.com][http://www.enterprisedb.com[http://www.enterprisedb.com]]] >> >> >> http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi][http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi]][http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi][http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi]]] >> >> >> >> >> -- >> Thanks & Regards, >> >> Ashesh Vashi >> EnterpriseDB INDIA: Enterprise PostgreSQL >> Company[http://www.enterprisedb.com[http://www.enterprisedb.com][http://www.enterprisedb.com[http://www.enterprisedb.com]]] >> >> >> http://www.linkedin.com/in/asheshvashi[
