libpq parameter parsing problem

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
22 messages Options
12
Reply | Threaded
Open this post in threaded view
|

libpq parameter parsing problem

Jobin Augustine
Hello hackers,
User can pass session-level settings as a parameter in the connection string like:
psql "host=localhost user=postgres options='-c synchronous_commit=off'"
Which sets the synchronous_commit off for the session.

However, URI spec is not allowing it,
psql postgresql://postgres@localhost:5432/postgres?options="-c synchronous_commit=off"
psql: error: could not connect to server: extra key/value separator "=" in URI query parameter: "options"

psql postgresql://postgres@localhost:5432/postgres?options="-c synchronous_commit off"
psql: error: could not connect to server: FATAL:  -c synchronous_commit requires a value

Moreover session just hangs forever:
psql postgresql://postgres@localhost:5432/postgres?application_name=hello&options='-c synchronous_commit=off'
provided that the connection works without the 'options' parameter specification

Thanks and regards,
Jobin.
Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Heikki Linnakangas
On 07/01/2020 15:17, Jobin Augustine wrote:

> Hello hackers,
> User can pass session-level settings as a parameter in the connection
> string like:
> psql "host=localhost user=postgres options='-c synchronous_commit=off'"
> Which sets the synchronous_commit off for the session.
>
> However, URI spec is not allowing it,
> psql postgresql://postgres@localhost:5432/postgres?options="-c
> synchronous_commit=off"
> psql: error: could not connect to server: extra key/value separator "="
> in URI query parameter: "options"

The "-c synchronous_commit=off" string is part of the value for the
"options" keyword. So it needs to be URI encoded:

psql
"postgresql://postgres@localhost:5432/postgres?options=-c%20synchronous_commit%3Doff"

- Heikki


Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Shulgin, Oleksandr
In reply to this post by Jobin Augustine
On Tue, Jan 7, 2020 at 2:17 PM Jobin Augustine <[hidden email]> wrote:
Hello hackers,
User can pass session-level settings as a parameter in the connection string like:
psql "host=localhost user=postgres options='-c synchronous_commit=off'"
Which sets the synchronous_commit off for the session.

However, URI spec is not allowing it,
psql postgresql://postgres@localhost:5432/postgres?options="-c synchronous_commit=off"
psql: error: could not connect to server: extra key/value separator "=" in URI query parameter: "options"

psql postgresql://postgres@localhost:5432/postgres?options="-c synchronous_commit off"
psql: error: could not connect to server: FATAL:  -c synchronous_commit requires a value

Jobin,

As already pointed out by Heikki, and per documentation:

Percent-encoding may be used to include symbols with special meaning in any of the URI parts, e.g. replace = with %3D.

Moreover session just hangs forever:
psql postgresql://postgres@localhost:5432/postgres?application_name=hello&options='-c synchronous_commit=off'
provided that the connection works without the 'options' parameter specification

What you observe here is most likely the result of your shell interpreting the ampersand sign (&) in the middle of the URI and triggering asynchronous execution of the command before the sign.  Please try escaping the sign or using appropriate quoting, e.g. extend the single quote to start before the URI:

psql 'postgresql://postgres@localhost:5432/postgres?application_name=hello&options=-c synchronous_commit%3Doff'

Cheers,
--
Alex
Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Jobin Augustine
Thanks Alex and Heikki,

Encoding works and it solved the problem. I find the documentation is not very clear about it.
Heikki's single line comment was more clear to me than documentation :)
A statement like  "PostgreSQL URI string need to be encoded with Percent-encoding " would have been much better


On Tue, Jan 7, 2020 at 7:54 PM Oleksandr Shulgin <[hidden email]> wrote:
On Tue, Jan 7, 2020 at 2:17 PM Jobin Augustine <[hidden email]> wrote:
Hello hackers,
User can pass session-level settings as a parameter in the connection string like:
psql "host=localhost user=postgres options='-c synchronous_commit=off'"
Which sets the synchronous_commit off for the session.

However, URI spec is not allowing it,
psql postgresql://postgres@localhost:5432/postgres?options="-c synchronous_commit=off"
psql: error: could not connect to server: extra key/value separator "=" in URI query parameter: "options"

psql postgresql://postgres@localhost:5432/postgres?options="-c synchronous_commit off"
psql: error: could not connect to server: FATAL:  -c synchronous_commit requires a value

Jobin,

As already pointed out by Heikki, and per documentation:

Percent-encoding may be used to include symbols with special meaning in any of the URI parts, e.g. replace = with %3D.

Moreover session just hangs forever:
psql postgresql://postgres@localhost:5432/postgres?application_name=hello&options='-c synchronous_commit=off'
provided that the connection works without the 'options' parameter specification

What you observe here is most likely the result of your shell interpreting the ampersand sign (&) in the middle of the URI and triggering asynchronous execution of the command before the sign.  Please try escaping the sign or using appropriate quoting, e.g. extend the single quote to start before the URI:

psql 'postgresql://postgres@localhost:5432/postgres?application_name=hello&options=-c synchronous_commit%3Doff'

Yes, That was too silly. my bad 

Cheers,
--
Alex

Thanks and Regards,
Jobin 
Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Heikki Linnakangas
On 08/01/2020 10:48, Jobin Augustine wrote:
> Thanks Alex and Heikki,
>
> Encoding works and it solved the problem. I find the documentation is
> not very clear about it.
> Heikki's single line comment was more clear to me than documentation :)
> A statement like  "PostgreSQL URI string need to be encoded with
> Percent-encoding <https://en.wikipedia.org/wiki/Percent-encoding> "
> would have been much better

If you could propose a concrete patch to add something like that to the
docs, that'd be great. I think adding an example connection URI like
that in the docs would be really helpful.

- Heikki


Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Michael Paquier-2
On Wed, Jan 08, 2020 at 11:06:35AM +0200, Heikki Linnakangas wrote:
> If you could propose a concrete patch to add something like that to the
> docs, that'd be great. I think adding an example connection URI like that in
> the docs would be really helpful.

+1.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Jobin Augustine
Hi Team,
Sorry for the late reply. Attaching a patch.

On Thu, Jan 9, 2020 at 10:08 AM Michael Paquier <[hidden email]> wrote:
On Wed, Jan 08, 2020 at 11:06:35AM +0200, Heikki Linnakangas wrote:
> If you could propose a concrete patch to add something like that to the
> docs, that'd be great. I think adding an example connection URI like that in
> the docs would be really helpful.

+1.
--
Michael 
Jobin 

libpqURIconnection.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Michael Paquier-2
On Mon, Jan 13, 2020 at 08:07:06PM +0530, Jobin Augustine wrote:
> Sorry for the late reply. Attaching a patch.

>     <para>
> -    Percent-encoding may be used to include symbols with special meaning in any
> -    of the <acronym>URI</acronym> parts, e.g. replace <literal>=</literal> with
> -    <literal>%3D</literal>.
> -
> +    Connection <acronym>URI</acronym> need to be encoded with
> +    <ulink url="https://en.wikipedia.org/wiki/Percent-encoding">Percent-encoding</ulink>
> +    if it includes symbols with special meaning in any of its parts.
> +    For example:
> +<programlisting>
> +postgresql://postgres@localhost:5432/postgres?options=-c%20synchronous_commit%3Doff%20-c%20geqo%3Doff
> +</programlisting>
> +    where all <literal>=</literal> are replaced with <literal>%3D</literal> and
> +    space character with <literal>%20</literal>
>     </para>
The reference to wikipedia is nice to have.  A small nit from me is
that I would group the last sentence with the "For example", to give:
"Here is an example where all = are replaced.."

The first sentence sounds good to me.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Jobin Augustine
Hi Michael,

On Tue, Jan 14, 2020 at 11:33 AM Michael Paquier <[hidden email]> wrote:
On Mon, Jan 13, 2020 at 08:07:06PM +0530, Jobin Augustine wrote:
> Sorry for the late reply. Attaching a patch.

>     <para>
> -    Percent-encoding may be used to include symbols with special meaning in any
> -    of the <acronym>URI</acronym> parts, e.g. replace <literal>=</literal> with
> -    <literal>%3D</literal>.
> -
> +    Connection <acronym>URI</acronym> need to be encoded with
> +    <ulink url="https://en.wikipedia.org/wiki/Percent-encoding">Percent-encoding</ulink>
> +    if it includes symbols with special meaning in any of its parts.
> +    For example:
> +<programlisting>
> +postgresql://postgres@localhost:5432/postgres?options=-c%20synchronous_commit%3Doff%20-c%20geqo%3Doff
> +</programlisting>
> +    where all <literal>=</literal> are replaced with <literal>%3D</literal> and
> +    space character with <literal>%20</literal>
>     </para>

The reference to wikipedia is nice to have.  A small nit from me is
that I would group the last sentence with the "For example", to give:
"Here is an example where all = are replaced.."
Yes, agree, readability is better with that modification.
Attaching a modified patch.
The first sentence sounds good to me.
--
Michael
 Jobin.

libpqURIconnection_v2.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Michael Paquier-2
On Tue, Jan 14, 2020 at 12:01:43PM +0530, Jobin Augustine wrote:
> Yes, agree, readability is better with that modification.
> Attaching a modified patch.

(Please always top-post, this is the style of the PostgreSQL community
mailing lists).  The new patch looks good to me, let's see if others
have extra thoughts to share.  If not, I'll try to commit it.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Alvaro Herrera-9
On 2020-Jan-14, Michael Paquier wrote:

> On Tue, Jan 14, 2020 at 12:01:43PM +0530, Jobin Augustine wrote:
> > Yes, agree, readability is better with that modification.
> > Attaching a modified patch.
>
> (Please always top-post, this is the style of the PostgreSQL community
> mailing lists).  The new patch looks good to me, let's see if others
> have extra thoughts to share.  If not, I'll try to commit it.

The thought I had when I first saw this was that it would be better to
have a minimal, coherent set of useful examples all together in a
subsection called Examples.  But then I realized that random examples
are already scattered here and there in the libpq page, so while I still
think that that would be better, I no longer think it's this patch's
responsibility to make it so.

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

David G Johnston
In reply to this post by Jobin Augustine
On Mon, Jan 13, 2020 at 11:32 PM Jobin Augustine <[hidden email]> wrote:
Hi Michael,

On Tue, Jan 14, 2020 at 11:33 AM Michael Paquier <[hidden email]> wrote:
On Mon, Jan 13, 2020 at 08:07:06PM +0530, Jobin Augustine wrote:
> Sorry for the late reply. Attaching a patch.

>     <para>
> -    Percent-encoding may be used to include symbols with special meaning in any
> -    of the <acronym>URI</acronym> parts, e.g. replace <literal>=</literal> with
> -    <literal>%3D</literal>.
> -
> +    Connection <acronym>URI</acronym> need to be encoded with
> +    <ulink url="https://en.wikipedia.org/wiki/Percent-encoding">Percent-encoding</ulink>
> +    if it includes symbols with special meaning in any of its parts.
> +    For example:
> +<programlisting>
> +postgresql://postgres@localhost:5432/postgres?options=-c%20synchronous_commit%3Doff%20-c%20geqo%3Doff
> +</programlisting>
> +    where all <literal>=</literal> are replaced with <literal>%3D</literal> and
> +    space character with <literal>%20</literal>
>     </para>

The reference to wikipedia is nice to have.  A small nit from me is
that I would group the last sentence with the "For example", to give:
"Here is an example where all = are replaced.."
Yes, agree, readability is better with that modification.
Attaching a modified patch.
The first sentence sounds good to me.


Should start out: "The Connection URI needs to be encoded with (confirm OK-ness of wikipedia direct link in docs) Percent-encoding..." (mainly "need => needs", the "the" reads better to me though)

I would probably choose to move the example for the options parameters to the "Parameter Key Words" options section:

...represent a literal backslash.</p>

<p>When specifying options as part of a percent-encoded Connection URI the structural space(s) and equal sign(s) "=" need to be encoded as %20 and %3D respectively (in addition to any value-specific encoding needs) while the escaping back-slash "\" does not.  For example:
<programlisting>postgresql:///mydb?options=-c%20synchronous_commit%3Doff%20-c%20geqo%3Doff</programlisting>

<p>For a detailed...

If you'd like an example for the first section maybe something like having a space in your database name:

postgresql://user@localhost:5432/my%20database

Also, regardless of where it is placed having both the username and database both be named "postgres" in an example just adds unnecessary mental effort to understanding the example.  One that none of the existing examples do.  Name your user "user" and database "mydb" unless, as with the desire to include a space, there is a meaningful reason not to.

Also, in my modified example (for options) above since everything is optional omitting everything except the database and parameters removes noise (though keeping them may increase comfort).

David J.

Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Michael Paquier-2
On Tue, Jan 14, 2020 at 06:52:07PM -0700, David G. Johnston wrote:
> I would probably choose to move the example for the options parameters to
> the "Parameter Key Words" options section:

I think that this would be inconsistent with the rest, as that's a URI
and all the other examples are there.  I agree with the feeling of
Alvaro upthread that we could do a better effort with the handling of
the examples in this section, but it is quite unclear to me if that
would actually bring more clarity to the whole, and that's not really
the job of this patch.

> Also, regardless of where it is placed having both the username and
> database both be named "postgres" in an example just adds unnecessary
> mental effort to understanding the example.  One that none of the existing
> examples do.  Name your user "user" and database "mydb" unless, as with the
> desire to include a space, there is a meaningful reason not to.

Good point.  Using "mydb" or "user" instead of "postgres" in the new
example would be less confusing.  Another question, would be it better
to use "5433" instead of "5432" for the port number.  That's a nit,
but as we are on that stuff let's be right..
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

David G Johnston
On Tue, Jan 14, 2020 at 7:40 PM Michael Paquier <[hidden email]> wrote:
On Tue, Jan 14, 2020 at 06:52:07PM -0700, David G. Johnston wrote:
> I would probably choose to move the example for the options parameters to
> the "Parameter Key Words" options section:

I think that this would be inconsistent with the rest, as that's a URI
and all the other examples are there.  I agree with the feeling of
Alvaro upthread that we could do a better effort with the handling of
the examples in this section, but it is quite unclear to me if that
would actually bring more clarity to the whole, and that's not really
the job of this patch.


My rationale is more since none of the other options have structural parts that require escaping, and rarely do the values themselves require escaping, that tossing that single example for a seldom-used option into the middle of the "usage examples" section doesn't really fit.  What the example does is clarify a specific combination of factors, URI and "options", that require special attention.  I'd rather bury that special case in the documentation for options then explain it in detail in the generic URI section - the structural elements involved are already mentioned in the options section and this just clarifies how they are written in the URI situation.  Its not a strong opinion but I don't think adding it there while leaving the other common compound usage examples as a whole above is a misplacement - "options" is special and can very well have special treatment.  It will be found by those that need to know about it.

In any case I do think that calling out the fact that "structural" pieces of the option parameter need to be encoded should happen regardless of the placement of the example that demonstrates those structural elements being encoded.  For the rest some people using unusual values may have to deal with escaping - for users of "options" they are going to and their attention should be drawn to that fact to help avoid the confusion that prompted this patch.

David J.

Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Michael Paquier-2
On Tue, Jan 14, 2020 at 08:54:41PM -0700, David G. Johnston wrote:

> My rationale is more since none of the other options have structural parts
> that require escaping, and rarely do the values themselves require
> escaping, that tossing that single example for a seldom-used option into
> the middle of the "usage examples" section doesn't really fit.  What the
> example does is clarify a specific combination of factors, URI and
> "options", that require special attention.  I'd rather bury that special
> case in the documentation for options then explain it in detail in the
> generic URI section - the structural elements involved are already
> mentioned in the options section and this just clarifies how they are
> written in the URI situation.  Its not a strong opinion but I don't think
> adding it there while leaving the other common compound usage examples as a
> whole above is a misplacement - "options" is special and can very well have
> special treatment.  It will be found by those that need to know about it.
Fair point.  Now, replacing a special character applies to more than
"options", because it can be used for any values.  For example:
postgresql:///mydb?host=localhost&application_name=hoge%20%3D%20foo
(This generates "hoge = foo" as application_name as you can guess.)

And the part of the docs for connection URIs describes only how to
percent-encode a path.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Peter Eisentraut-6
In reply to this post by Michael Paquier-2
On 2020-01-14 07:03, Michael Paquier wrote:

>>      <para>
>> -    Percent-encoding may be used to include symbols with special meaning in any
>> -    of the <acronym>URI</acronym> parts, e.g. replace <literal>=</literal> with
>> -    <literal>%3D</literal>.
>> -
>> +    Connection <acronym>URI</acronym> need to be encoded with
>> +    <ulink url="https://en.wikipedia.org/wiki/Percent-encoding">Percent-encoding</ulink>
>> +    if it includes symbols with special meaning in any of its parts.
>> +    For example:
>> +<programlisting>
>> +postgresql://postgres@localhost:5432/postgres?options=-c%20synchronous_commit%3Doff%20-c%20geqo%3Doff
>> +</programlisting>
>> +    where all <literal>=</literal> are replaced with <literal>%3D</literal> and
>> +    space character with <literal>%20</literal>
>>      </para>
>
> The reference to wikipedia is nice to have.

Let's please not put links to Wikipedia into the main body of the
documentation.  If people want to look up something, they know where to
find it.

Links to authoritative sources (perhaps an RFC in this case) would be
better.

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Michael Paquier-2
On Wed, Jan 15, 2020 at 04:42:28PM +0100, Peter Eisentraut wrote:
> Let's please not put links to Wikipedia into the main body of the
> documentation.  If people want to look up something, they know where to find
> it.

That's not really a new concept:
$ cd doc/ && git grep wikipedia\.org | wc -l
55

> Links to authoritative sources (perhaps an RFC in this case) would be
> better.

No objections to your suggestion in this case though.  I can see
Section 2.1 from RFC3986 for that:
https://tools.ietf.org/html/rfc3986#section-2.1

So, gathering all the feedback and my own tweaks, I finish with the
attached (merely simplified).
--
Michael

libpqURIconnection_v3.diff (1K) Download Attachment
signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Jobin Augustine

> Links to authoritative sources (perhaps an RFC in this case) would be
> better.

No objections to your suggestion in this case though.  I can see
Section 2.1 from RFC3986 for that:
https://tools.ietf.org/html/rfc3986#section-2.1

So, gathering all the feedback and my own tweaks, I finish with the
attached (merely simplified).
--
Michael
+1
As an end-user,  It it is very clear for me now.

Verified the new example :
$ psql "postgresql://user@localhost:5433/mydb?options=-c%20synchronous_commit%3Doff"
Password for user user:
Pager usage is off.
psql (12.0 (Ubuntu 12.0-2.pgdg18.04+1), server 11.5)
Type "help" for help.

mydb=> show synchronous_commit;
 synchronous_commit
--------------------
 off
(1 row)



Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Shulgin, Oleksandr
In reply to this post by Michael Paquier-2
On Thu, Jan 16, 2020 at 4:49 AM Michael Paquier <[hidden email]> wrote:

> Links to authoritative sources (perhaps an RFC in this case) would be
> better.

No objections to your suggestion in this case though.  I can see
Section 2.1 from RFC3986 for that:
https://tools.ietf.org/html/rfc3986#section-2.1

Yes, this makes more sense, especially since we already refer to that same RFC earlier on the page:

> URIs generally follow <ulink url="https://tools.ietf.org/html/rfc3986">RFC 3986</ulink>,...

For a moment I thought we could improve further if we rephrase "symbols with special meaning" as "reserved characters", which are defined in section 2.2 of the RFC.  But that set is broader than what actually needs to be encoded for correct interpretation by our parser.

At the same time, we could still be more specific if we would say "delimiters" instead of the generic "special meaning".  Should we then provide an exhaustive list of delimiters or is it clear enough like that?  For example, the whitespace doesn't need to be percent-encoded (it doesn't hurt as you might be able to spare the quoting if using it as an argument to a shell command), while the "equal sign", when used in the query string part, does need encoding.

--
Alex

Reply | Threaded
Open this post in threaded view
|

Re: libpq parameter parsing problem

Michael Paquier-2
On Thu, Jan 16, 2020 at 09:17:23AM +0100, Oleksandr Shulgin wrote:
> At the same time, we could still be more specific if we would say
> "delimiters" instead of the generic "special meaning".  Should we then
> provide an exhaustive list of delimiters or is it clear enough like that?
> For example, the whitespace doesn't need to be percent-encoded (it doesn't
> hurt as you might be able to spare the quoting if using it as an argument
> to a shell command), while the "equal sign", when used in the query string
> part, does need encoding.

This term comes from you, as of 2d612ab, and that does not look like
something to change because reserved characters have "sometimes" a
special meaning in this context.  A reference to the RFC is sufficient
IMO, readers could always look at that reference for a precise list
and that would bloat the docs more than necessary.
--
Michael

signature.asc (849 bytes) Download Attachment
12