Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Excerpts from Pavel Stehule's message of dom jun 26 13:10:13 -0400 2011: Hello I have touched next_token() and next_token_expand() a bit more, because it seemed to me that they could be simplified further (the bit about returning the comma in the token, instead of being a boolean return, seemed strange). Please let me know what you think. I am thinking, so it is ok. Thanks, I have pushed it. Brendan: thanks for the patch. I am not sure, if load_ident is correct. In load_hba you clean a previous context after successful processing. In load_ident you clean data on start. Is it ok? Yeah, they are a bit inconsistent. I am uninclined to mess with it, though. Right now it seems to me that the only way it could fail is if you hit an out-of-memory or a similar problem. And if you hit that at postmaster startup ... well ... I guess you have A Problem. If somebody wants to submit a patch to fix that, be my guest. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/28 Alvaro Herrera alvhe...@commandprompt.com: Excerpts from Pavel Stehule's message of dom jun 26 13:10:13 -0400 2011: Hello I have touched next_token() and next_token_expand() a bit more, because it seemed to me that they could be simplified further (the bit about returning the comma in the token, instead of being a boolean return, seemed strange). Please let me know what you think. I am thinking, so it is ok. Thanks, I have pushed it. Brendan: thanks for the patch. I am not sure, if load_ident is correct. In load_hba you clean a previous context after successful processing. In load_ident you clean data on start. Is it ok? Yeah, they are a bit inconsistent. I am uninclined to mess with it, though. Right now it seems to me that the only way it could fail is if you hit an out-of-memory or a similar problem. And if you hit that at postmaster startup ... well ... I guess you have A Problem. there should be a format (syntax) error. If somebody breaks a pg_ident and will do a reload, then all ident mapping is lost. This is not related to topic or doesn't modify current behave, so should not be repaired now Pavel If somebody wants to submit a patch to fix that, be my guest. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Excerpts from Pavel Stehule's message of mar jun 28 16:19:02 -0400 2011: there should be a format (syntax) error. If somebody breaks a pg_ident and will do a reload, then all ident mapping is lost. No, the file is not actually parsed until the auth verification runs. The incorrect tokens are happily stored in memory by the tokeniser. In my view that's the wrong way to do it, but changing it seems to be a lot of work (I didn't try). -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/28 Alvaro Herrera alvhe...@commandprompt.com: Excerpts from Pavel Stehule's message of mar jun 28 16:19:02 -0400 2011: there should be a format (syntax) error. If somebody breaks a pg_ident and will do a reload, then all ident mapping is lost. No, the file is not actually parsed until the auth verification runs. The incorrect tokens are happily stored in memory by the tokeniser. In my view that's the wrong way to do it, but changing it seems to be a lot of work (I didn't try). ok -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Hello I have touched next_token() and next_token_expand() a bit more, because it seemed to me that they could be simplified further (the bit about returning the comma in the token, instead of being a boolean return, seemed strange). Please let me know what you think. I am thinking, so it is ok. I am not sure, if load_ident is correct. In load_hba you clean a previous context after successful processing. In load_ident you clean data on start. Is it ok? Regards Pavel -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On 24 June 2011 03:48, Alvaro Herrera alvhe...@commandprompt.com wrote: I have touched next_token() and next_token_expand() a bit more, because it seemed to me that they could be simplified further (the bit about returning the comma in the token, instead of being a boolean return, seemed strange). Please let me know what you think. Cool. I didn't like the way it returned the comma either, but I thought it would be best if I kept the changes in my patch to a minimum. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011:
yes - it has a sense. Quoting changes sense from keyword to literal.
But then I see a significant inconsistency - every know keywords
should be only tokens.
else if (strcmp(token, pamservice) == 0)
- {
- REQUIRE_AUTH_OPTION(uaPAM, pamservice, pam);
- parsedline-pamservice = pstrdup(c);
- }
because pamservice - is known keyword, but 'pamservice' is some
literal without any mean. You should to use a makro token_is_keyword
more often.
Yeah, I wondered about this too (same with auth types, i.e. do we accept
quoted hostssl and so on or should that by rejected?). I opted for
leaving it alone, but maybe this needs to be fixed. (Now that I think
about it, what we should do first is verify whether it works with quotes
in the unpatched code).
--
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/21 Alvaro Herrera alvhe...@commandprompt.com:
Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011:
yes - it has a sense. Quoting changes sense from keyword to literal.
But then I see a significant inconsistency - every know keywords
should be only tokens.
else if (strcmp(token, pamservice) == 0)
- {
- REQUIRE_AUTH_OPTION(uaPAM, pamservice, pam);
- parsedline-pamservice = pstrdup(c);
- }
because pamservice - is known keyword, but 'pamservice' is some
literal without any mean. You should to use a makro token_is_keyword
more often.
Yeah, I wondered about this too (same with auth types, i.e. do we accept
quoted hostssl and so on or should that by rejected?). I opted for
leaving it alone, but maybe this needs to be fixed. (Now that I think
about it, what we should do first is verify whether it works with quotes
in the unpatched code).
It's question about compatibility - sure. But a description inside
pg_hba.conf speaks cleanly - quoting means a lost of original
semantic.
And if we allow a quoting somewhere, then I can't imagine a
description - somewhere quoting means a string literal, somewhere
have not impact?
Regards
Pavel Stehule
--
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Excerpts from Pavel Stehule's message of mar jun 21 10:04:26 -0400 2011:
2011/6/21 Alvaro Herrera alvhe...@commandprompt.com:
Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011:
yes - it has a sense. Quoting changes sense from keyword to literal.
But then I see a significant inconsistency - every know keywords
should be only tokens.
else if (strcmp(token, pamservice) == 0)
- {
- REQUIRE_AUTH_OPTION(uaPAM, pamservice, pam);
- parsedline-pamservice = pstrdup(c);
- }
because pamservice - is known keyword, but 'pamservice' is some
literal without any mean. You should to use a makro token_is_keyword
more often.
Yeah, I wondered about this too (same with auth types, i.e. do we accept
quoted hostssl and so on or should that by rejected?). I opted for
leaving it alone, but maybe this needs to be fixed. (Now that I think
about it, what we should do first is verify whether it works with quotes
in the unpatched code).
I tested it and it works: This line
local @dbs +b trust
is accepted and it works in the unpatched code. I don't think we want
to break people's existing pg_hba.conf files for no reason. I doubt
that many people are using pg_hba.conf tokens with quotes, mind you, but
there might be some ...
In any case, if people here thinks we should tighten this, it's easy to
do on top of this patch by changing the strcmp() calls to
token_is_keyword, as you say. Let's not burden this patch with the
responsibility of doing so, because that's likely to get it punted.
--
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/21 Alvaro Herrera alvhe...@commandprompt.com:
Excerpts from Pavel Stehule's message of mar jun 21 10:04:26 -0400 2011:
2011/6/21 Alvaro Herrera alvhe...@commandprompt.com:
Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011:
yes - it has a sense. Quoting changes sense from keyword to literal.
But then I see a significant inconsistency - every know keywords
should be only tokens.
else if (strcmp(token, pamservice) == 0)
- {
- REQUIRE_AUTH_OPTION(uaPAM, pamservice, pam);
- parsedline-pamservice = pstrdup(c);
- }
because pamservice - is known keyword, but 'pamservice' is some
literal without any mean. You should to use a makro token_is_keyword
more often.
Yeah, I wondered about this too (same with auth types, i.e. do we accept
quoted hostssl and so on or should that by rejected?). I opted for
leaving it alone, but maybe this needs to be fixed. (Now that I think
about it, what we should do first is verify whether it works with quotes
in the unpatched code).
I tested it and it works: This line
local @dbs +b trust
is accepted and it works in the unpatched code. I don't think we want
to break people's existing pg_hba.conf files for no reason. I doubt
that many people are using pg_hba.conf tokens with quotes, mind you, but
there might be some ...
In any case, if people here thinks we should tighten this, it's easy to
do on top of this patch by changing the strcmp() calls to
token_is_keyword, as you say. Let's not burden this patch with the
responsibility of doing so, because that's likely to get it punted.
It is time to discuss about it.
I thinking so current behave is strange and should be fixed - it
doesn't respect a description stored in pg_hba.conf. I agree, so this
will have impact on compatibility, but pg_hba is config file so this
impact is not too hard. The cleaning now can carry a benefit in
future, when pg_hba can be more complex.
Regards
Pavel
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On Tue, Jun 21, 2011 at 10:15:50AM -0400, Alvaro Herrera wrote:
Excerpts from Pavel Stehule's message of mar jun 21 10:04:26 -0400 2011:
2011/6/21 Alvaro Herrera alvhe...@commandprompt.com:
Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011:
yes - it has a sense. Quoting changes sense from keyword to literal.
But then I see a significant inconsistency - every know keywords
should be only tokens.
else if (strcmp(token, pamservice) == 0)
- {
- REQUIRE_AUTH_OPTION(uaPAM, pamservice, pam);
- parsedline-pamservice = pstrdup(c);
- }
because pamservice - is known keyword, but 'pamservice' is some
literal without any mean. You should to use a makro token_is_keyword
more often.
Yeah, I wondered about this too (same with auth types, i.e. do we accept
quoted hostssl and so on or should that by rejected?). I opted for
leaving it alone, but maybe this needs to be fixed. (Now that I think
about it, what we should do first is verify whether it works with quotes
in the unpatched code).
I tested it and it works: This line
local @dbs +b trust
is accepted and it works in the unpatched code. I don't think we want
to break people's existing pg_hba.conf files for no reason. I doubt
that many people are using pg_hba.conf tokens with quotes, mind you, but
there might be some ...
In any case, if people here thinks we should tighten this, it's easy to
do on top of this patch by changing the strcmp() calls to
token_is_keyword, as you say. Let's not burden this patch with the
responsibility of doing so, because that's likely to get it punted.
Hmm, would it be possible to add some deprecation warnings for this case
without making the code too messy? Perhaps with a macro
token_should_be_keyword. That's the usual path to tightening syntax.
Ross
--
Ross Reedstrom, Ph.D. reeds...@rice.edu
Systems Engineer Admin, Research Scientistphone: 713-348-6166
Connexions http://cnx.orgfax: 713-348-3665
Rice University MS-375, Houston, TX 77005
GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E F888 D3AE 810E 88F0 BEDE
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011: because pamservice - is known keyword, but 'pamservice' is some literal without any mean. You should to use a makro token_is_keyword more often. Yeah, I wondered about this too (same with auth types, i.e. do we accept quoted hostssl and so on or should that by rejected?). I opted for leaving it alone, but maybe this needs to be fixed. (Now that I think about it, what we should do first is verify whether it works with quotes in the unpatched code). AFAICS, this is only important in places where the syntax allows either a keyword or an identifier. If only a keyword is possible, there is no value in rejecting it because it's quoted. And, when you do the test, I think you'll find that it would be breaking hba files that used to work (though admittedly, it's doubtful that there are any such in the field). regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/21 Tom Lane t...@sss.pgh.pa.us: Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011: because pamservice - is known keyword, but 'pamservice' is some literal without any mean. You should to use a makro token_is_keyword more often. Yeah, I wondered about this too (same with auth types, i.e. do we accept quoted hostssl and so on or should that by rejected?). I opted for leaving it alone, but maybe this needs to be fixed. (Now that I think about it, what we should do first is verify whether it works with quotes in the unpatched code). AFAICS, this is only important in places where the syntax allows either a keyword or an identifier. If only a keyword is possible, there is no value in rejecting it because it's quoted. And, when you do the test, I think you'll find that it would be breaking hba files that used to work (though admittedly, it's doubtful that there are any such in the field). It should be better documented. I don't think so this is good solution, but this is not too important. regards Pavel regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Excerpts from Pavel Stehule's message of mar jun 21 11:04:11 -0400 2011: 2011/6/21 Tom Lane t...@sss.pgh.pa.us: AFAICS, this is only important in places where the syntax allows either a keyword or an identifier. If only a keyword is possible, there is no value in rejecting it because it's quoted. And, when you do the test, I think you'll find that it would be breaking hba files that used to work (though admittedly, it's doubtful that there are any such in the field). It should be better documented. I don't think so this is good solution, but this is not too important. On the contrary -- we should support it but not document it. I mean, what good would that do? If someone is so silly to uselessly quote keywords, let them do it, but let's not encourage it. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/21 Alvaro Herrera alvhe...@commandprompt.com: Excerpts from Pavel Stehule's message of mar jun 21 11:04:11 -0400 2011: 2011/6/21 Tom Lane t...@sss.pgh.pa.us: AFAICS, this is only important in places where the syntax allows either a keyword or an identifier. If only a keyword is possible, there is no value in rejecting it because it's quoted. And, when you do the test, I think you'll find that it would be breaking hba files that used to work (though admittedly, it's doubtful that there are any such in the field). It should be better documented. I don't think so this is good solution, but this is not too important. On the contrary -- we should support it but not document it. I mean, what good would that do? If someone is so silly to uselessly quote keywords, let them do it, but let's not encourage it. it is argument too :) It has not good solution - one break compatibility, second is strange and undocumented :( Actually I don't remember a issues about pg_hba.conf - probably 99% users work with default configuration, so we can leave this file in current state. I am thinking so a notice in pg_hba.conf can be redesigned - almost all people don't read it, but if someone read it, then he needs a correct information - in sense, so on quotes works only where literal or known literal can be entered. Regards Pavel Stehule -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On Jun 21, 2011, at 12:41 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: On the contrary -- we should support it but not document it. +1. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On 22 June 2011 00:47, Tom Lane t...@sss.pgh.pa.us wrote: Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011: because pamservice - is known keyword, but 'pamservice' is some literal without any mean. You should to use a makro token_is_keyword more often. Yeah, I wondered about this too (same with auth types, i.e. do we accept quoted hostssl and so on or should that by rejected?). I opted for leaving it alone, but maybe this needs to be fixed. (Now that I think about it, what we should do first is verify whether it works with quotes in the unpatched code). AFAICS, this is only important in places where the syntax allows either a keyword or an identifier. If only a keyword is possible, there is no value in rejecting it because it's quoted. And, when you do the test, I think you'll find that it would be breaking hba files that used to work (though admittedly, it's doubtful that there are any such in the field). Yes, I agree, and this was my thinking when I came up against this while writing the original patch. It doesn't help to treat hostssl differently than hostssl, because quoted identifiers are meaningless in that context. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/22 Brendan Jurd dire...@gmail.com: On 22 June 2011 00:47, Tom Lane t...@sss.pgh.pa.us wrote: Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011: because pamservice - is known keyword, but 'pamservice' is some literal without any mean. You should to use a makro token_is_keyword more often. Yeah, I wondered about this too (same with auth types, i.e. do we accept quoted hostssl and so on or should that by rejected?). I opted for leaving it alone, but maybe this needs to be fixed. (Now that I think about it, what we should do first is verify whether it works with quotes in the unpatched code). AFAICS, this is only important in places where the syntax allows either a keyword or an identifier. If only a keyword is possible, there is no value in rejecting it because it's quoted. And, when you do the test, I think you'll find that it would be breaking hba files that used to work (though admittedly, it's doubtful that there are any such in the field). Yes, I agree, and this was my thinking when I came up against this while writing the original patch. It doesn't help to treat hostssl differently than hostssl, because quoted identifiers are meaningless in that context. ook, now is clean, so this is majority opinion. Please, can you send a final patch. Regards Pavel Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On 22 June 2011 14:01, Pavel Stehule pavel.steh...@gmail.com wrote: ook, now is clean, so this is majority opinion. Please, can you send a final patch. I don't have any further changes to add to Alvaro's version 3, which is already up on the CF app. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Hello Brendan, I checked your patch, it is applied cleanly and I don't see any mayor problem. This patch does all what is expected. I have two minor comments a) you don't use macro token_matches consistently func: parse_hba_line --if (strcmp(token-string, local) == 0) should be if (token_is_keyword(token, local)) ... I don't see any sense when somebody use a quotes there. b) probably you can simplify a memory management using own two persistent memory context - and you can swap it. Then functions like free_hba_record, clean_hba_list, free_lines should be removed. Regards Pavel Stehule 2011/6/18 Brendan Jurd dire...@gmail.com: On 18 June 2011 13:43, Alvaro Herrera alvhe...@commandprompt.com wrote: Is this really a WIP patch? I'm playing a bit with it currently, seems fairly sane. In this case, the WIP designation is meant to convey warning: only casual testing has beeen done. I tried it out with various permutations of pg_hba.conf, and it worked as advertised in those tests, but I have not made any attempt to formulate a more rigorous testing regimen. In particular I haven't tested that the more exotic authentication methods still work properly, and I can't recall whether I tested recursive file inclusion and group membership. Is that a wrongful use of the WIP designation? Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
sorry a) you don't use macro token_matches consistently should be a) you don't use macro token_is_keyword consistently it should be used for all keywords Regards Pavel Stehule -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Excerpts from Pavel Stehule's message of lun jun 20 11:34:25 -0400 2011: b) probably you can simplify a memory management using own two persistent memory context - and you can swap it. Then functions like free_hba_record, clean_hba_list, free_lines should be removed. Yeah, I reworked the patch with something like that over the weekend. Not all of it though. I'll send an updated version shortly. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On 21 June 2011 06:06, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Alvaro Herrera's message of lun jun 20 12:19:37 -0400 2011: Excerpts from Pavel Stehule's message of lun jun 20 11:34:25 -0400 2011: b) probably you can simplify a memory management using own two persistent memory context - and you can swap it. Then functions like free_hba_record, clean_hba_list, free_lines should be removed. Yeah, I reworked the patch with something like that over the weekend. Not all of it though. I'll send an updated version shortly. Here it is, please let me know what you think. I took the liberty of cleaning up some things that were clearly historical leftovers. Okay, yeah, the MemoryContext approach is far more elegant than what I had. Boy was I ever barking up the wrong tree. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Excerpts from Brendan Jurd's message of lun jun 20 20:06:39 -0400 2011: On 21 June 2011 06:06, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Alvaro Herrera's message of lun jun 20 12:19:37 -0400 2011: Excerpts from Pavel Stehule's message of lun jun 20 11:34:25 -0400 2011: b) probably you can simplify a memory management using own two persistent memory context - and you can swap it. Then functions like free_hba_record, clean_hba_list, free_lines should be removed. Yeah, I reworked the patch with something like that over the weekend. Not all of it though. I'll send an updated version shortly. Here it is, please let me know what you think. I took the liberty of cleaning up some things that were clearly historical leftovers. Okay, yeah, the MemoryContext approach is far more elegant than what I had. Boy was I ever barking up the wrong tree. Eh, whoever wrote the original code was barking up the same tree, so I don't blame you for following the well-trodden path. I realize I took out most of the fun of this patch from you, but -- are you still planning to do some more exhaustive testing of it? I checked some funny scenarios (including include files and groups) but it's not all that unlikely that I missed some cases. I also didn't check any auth method other than ident, md5 and trust, 'cause I don't have what's required for anything else. (It's a pity that the regression tests don't exercise anything else.) -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On 21 June 2011 11:11, Alvaro Herrera alvhe...@commandprompt.com wrote: I realize I took out most of the fun of this patch from you, but -- are you still planning to do some more exhaustive testing of it? I checked some funny scenarios (including include files and groups) but it's not all that unlikely that I missed some cases. I also didn't check any auth method other than ident, md5 and trust, 'cause I don't have what's required for anything else. (It's a pity that the regression tests don't exercise anything else.) I've pretty much tested all the things I can think to test, and I'm in the same boat as you re auth methods. If there are bugs, I think they will be of the obscure kind, that only reveal themselves under peculiar circumstances. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/20 Alvaro Herrera alvhe...@commandprompt.com: Excerpts from Alvaro Herrera's message of lun jun 20 12:19:37 -0400 2011: Excerpts from Pavel Stehule's message of lun jun 20 11:34:25 -0400 2011: b) probably you can simplify a memory management using own two persistent memory context - and you can swap it. Then functions like free_hba_record, clean_hba_list, free_lines should be removed. Yeah, I reworked the patch with something like that over the weekend. Not all of it though. I'll send an updated version shortly. Here it is, please let me know what you think. I took the liberty of cleaning up some things that were clearly historical leftovers. I have one question. I can't find any rules for work with tokens, etc, where is quotes allowed and disallowed? I don't see any other issues. Regards Pavel Stehule -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On 21 June 2011 13:51, Pavel Stehule pavel.steh...@gmail.com wrote: I have one question. I can't find any rules for work with tokens, etc, where is quotes allowed and disallowed? I don't see any other issues. I'm not sure I understand your question, but quotes are allowed anywhere and they always act to remove any special meaning the token might otherwise have had. It's much like quoting a column name in SQL. I didn't change any of the hba parsing rules, so escaping and whatnot should all work the way it did before. The only difference should be that after parsing, keywords will only be evaluated for fields where they matter. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/21 Brendan Jurd dire...@gmail.com: On 21 June 2011 13:51, Pavel Stehule pavel.steh...@gmail.com wrote: I have one question. I can't find any rules for work with tokens, etc, where is quotes allowed and disallowed? I don't see any other issues. I'm not sure I understand your question, but quotes are allowed anywhere and they always act to remove any special meaning the token might otherwise have had. It's much like quoting a column name in SQL. I don't understand to using a macro #define token_is_keyword(t, k) (!t-quoted strcmp(t-string, k) == 0) because you disallowed a quoting? Regards Pavel I didn't change any of the hba parsing rules, so escaping and whatnot should all work the way it did before. The only difference should be that after parsing, keywords will only be evaluated for fields where they matter. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On 21 June 2011 14:34, Pavel Stehule pavel.steh...@gmail.com wrote: I don't understand to using a macro #define token_is_keyword(t, k) (!t-quoted strcmp(t-string, k) == 0) because you disallowed a quoting? Well, a token can only be treated as a special keyword if it is unquoted. As an example, in the 'database' field, the bare token 'replication' is a keyword meaning the pseudo-database for streaming rep. Whereas the quoted token replication would mean a real database which is called 'replication'. Likewise, the bare token 'all' in the username field is a keyword -- it matches any username. Whereas the quoted token all would only match a user named 'all'. Therefore, token_is_keyword only returns true where the token matches the given string as is also unquoted. Does that make sense? Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/21 Brendan Jurd dire...@gmail.com:
On 21 June 2011 14:34, Pavel Stehule pavel.steh...@gmail.com wrote:
I don't understand to using a macro
#define token_is_keyword(t, k) (!t-quoted strcmp(t-string, k) == 0)
because you disallowed a quoting?
Well, a token can only be treated as a special keyword if it is unquoted.
As an example, in the 'database' field, the bare token 'replication'
is a keyword meaning the pseudo-database for streaming rep. Whereas
the quoted token replication would mean a real database which is
called 'replication'.
Likewise, the bare token 'all' in the username field is a keyword --
it matches any username. Whereas the quoted token all would only
match a user named 'all'.
Therefore, token_is_keyword only returns true where the token matches
the given string as is also unquoted.
Does that make sense?
yes - it has a sense. Quoting changes sense from keyword to literal.
But then I see a significant inconsistency - every know keywords
should be only tokens.
else if (strcmp(token, pamservice) == 0)
- {
- REQUIRE_AUTH_OPTION(uaPAM, pamservice, pam);
- parsedline-pamservice = pstrdup(c);
- }
because pamservice - is known keyword, but 'pamservice' is some
literal without any mean. You should to use a makro token_is_keyword
more often.
??
Regards
Pavel
Cheers,
BJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On 16 June 2011 00:22, Pavel Stehule pavel.steh...@gmail.com wrote: I try to apply your patch, but it is finished with some failed hinks. Please, can you refresh your patch Hi Pavel, Thanks for taking a look. I have attached v2 of the patch, as against current HEAD. I've also added the new patch to the CF app. I look forward to your comments. Cheers, BJ hba-keywords-v2.diff.bz2 Description: BZip2 compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Excerpts from Brendan Jurd's message of vie jun 17 19:31:41 -0400 2011: On 16 June 2011 00:22, Pavel Stehule pavel.steh...@gmail.com wrote: I try to apply your patch, but it is finished with some failed hinks. Please, can you refresh your patch Hi Pavel, Thanks for taking a look. I have attached v2 of the patch, as against current HEAD. I've also added the new patch to the CF app. I look forward to your comments. Is this really a WIP patch? I'm playing a bit with it currently, seems fairly sane. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On 18 June 2011 13:43, Alvaro Herrera alvhe...@commandprompt.com wrote: Is this really a WIP patch? I'm playing a bit with it currently, seems fairly sane. In this case, the WIP designation is meant to convey warning: only casual testing has beeen done. I tried it out with various permutations of pg_hba.conf, and it worked as advertised in those tests, but I have not made any attempt to formulate a more rigorous testing regimen. In particular I haven't tested that the more exotic authentication methods still work properly, and I can't recall whether I tested recursive file inclusion and group membership. Is that a wrongful use of the WIP designation? Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
