Horton

Re: Another review of URI for libpq, v7 submission

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>

View thread
From: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
To: Alex <ash@commandprompt.com>
Cc: Marko Kreen <markokr@gmail.com>, Daniel Farina <daniel@heroku.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2012-03-27T13:03:30Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Refactor to eliminate duplicate copies of conninfo default-finding code.

On 22.03.2012 23:42, Alex wrote:
> Okay, at last here's v9, rebased against current master branch.

Some quick comments on this patch:

I see a compiler warning:
fe-connect.c: In function ‘conninfo_parse’:
fe-connect.c:4113: warning: unused variable ‘option’

Docs are missing.

I wonder if you should get an error if you try specify the same option 
multiple times. At least the behavior needs to be documented.

Should %00 be forbidden?

The error message is a bit confusing for 
"postgres://localhost?dbname=%XXfoo":
WARNING: ignoring unrecognized URI query parameter: dbname
There is in fact nothing wrong with "dbname", it's the %XX in the value 
that's bogus.

Looking at conninfo_uri_decode(), I think it's missing a bounds check, 
and peeks at two bytes beyond the end of string if the input ends in a '%'.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com