Re: Another review of URI for libpq, v7 submission

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Alex <ash(at)commandprompt(dot)com>
Cc: Marko Kreen <markokr(at)gmail(dot)com>, Daniel Farina <daniel(at)heroku(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Another review of URI for libpq, v7 submission
Date: 2012-03-27 13:03:30
Message-ID: 4F71BAA2.7080902@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2012-03-27 13:11:38 Re: Cross-backend signals and administration (Was: Re: pg_terminate_backend for same-role)
Previous Message Magnus Hagander 2012-03-27 12:58:26 Re: Cross-backend signals and administration (Was: Re: pg_terminate_backend for same-role)