Re: WIP: URI connection string support for libpq

On 12/13/2011 05:45 PM, Alexander Shulgin wrote:
> Before that, why don't also accept "psql://", "pgsql://", "postgre://" 
> and anything else? Or wait, aren't we adding to the soup again (or 
> rather putting the soup right into libpq?)

There are multiple URI samples within PostgreSQL drivers in the field, 
here are two I know of what I believe to be a larger number of samples 
that all match in this regard:

http://sequel.rubyforge.org/rdoc/files/doc/opening_databases_rdoc.html
http://www.rmunn.com/sqlalchemy-tutorial/tutorial.html

These two are using "postgres".  One of the hopes in adding URI support 
was to make it possible for the libpq spec to look similar to the ones 
already floating around, so that they'd all converge.  Using a different 
prefix than the most popular ones have already adopted isn't a good way 
to start that.  Now, whenever the URI discussion wanders off into 
copying the JDBC driver I wonder again why that's relevant.  But making 
the implementation look like what people have already deployed surely 
is, particularly if there's no downside to doing that.

Initial quick review of your patch:  you suggested this as the general form:

psql -d postgresql://user@pw:host:port/dbname?param1=value1&param2=value2...

That's presumably supposed to be:

psql -d postgresql://user:pw@host:port/dbname?param1=value1&param2=value2...

This variation worked here:

$ psql -d postgresql://gsmith@localhost:5432/gsmith

If we had to pick one URI prefix, it should be "postgres".  But given 
the general name dysfunction around this project, I can't see how anyone 
would complain if we squat on "postgresql" too.  Attached patch modifies 
yours to prove we can trivially support both, in hopes of detonating 
this argument before it rages on further.  Tested like this:

$ psql -d postgres://gsmith@localhost:5432/gsmith

And that works too now.  I doubt either of us like what I did to the 
handoff between conninfo_uri_parse and conninfo_uri_parse_options to 
achieve that, but this feature is still young.

After this bit of tinkering with the code, it feels to me like this 
really wants a split() function to break out the two sides of a string 
across a delimiter, eating it in the process.  Adding the level of 
paranoia I'd like around every bit of code I see that does that type of 
operation right now would take a while.  Refactoring in terms of split 
and perhaps a few similarly higher-level string parsing operations, 
targeted for this job, might make it easier to focus on fortifying those 
library routines instead.  For example, instead of the gunk I just added 
that moves past either type of protocol prefix, I'd like to just say 
"split(buf,"://",&left,&right) and then move on with processing the 
right side.

I agree with your comment that we need to add some sort of regression 
tests for this.  Given how the parsing is done right now, we'd want to 
come up with some interesting invalid strings too.  Making sure this 
fails gracefully (and not in a buffer overflow way) might even use 
something like fuzz testing too.  Around here we've just been building 
some Python scripting to do that sort of thing, tests that aren't 
practical to do with pg_regress.  Probably be better from the project's 
perspective if such things were in Perl instead; so far no one has ever 
paid me enough to stomach writing non-trivial things in Perl.  Perhaps 
you are more diverse.

-- 
Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us