Re: [BUGS]log can not be output when use DataSource - Mailing list pgsql-jdbc

Dave,

This new patch from Chen addresses 5. & 6. in my reivew. Those aspects
removed. The patch seems to address properly the BUG identified in the
logLevel original posting & other minor fixes as identified by review.

danap.

Chen Huajun wrote:

Hi

According the review result,and i modified the patch.
Deleted user & password from getUrl() and resumed getUrl() to private.

I think the following should be considered in the future, even if it 's
really needed.
 > The changing is just for testing.
 > It is also useful to compare two DataSource(for instance one is a
copy of another )
 > But if just for testing,now i have an another idea,
 > implementing toString() method which just call getURL()

Chen Huajun
(2013/02/04 10:28), Chen Huajun wrote:

danap,

> 5. getURL() - Changed from private to public, Why?, DriverManager
Contains
> no such public method nor Does the Java API define for interface
> DataSource,
> OK???

The changing is just for testing.
It is also useful to compare two DataSource(for instance one is a copy
of another )
But if just for testing,now i have an another idea,
implementing toString() method which just call getURL().
What about that?

> 6. user & password - Even if 5. approved public why would these be
open to
> a public interface for returning these values with URL. In a basic
> instinct I feel this is security risk.
> NOT OK

oh,they are not needed just as you said.
Sorry,i failed to understood your reply.
> in getURL(). The properties user & password are included in
getConnection()
> and therefore do not not need to be in getURL().

Chen Huajun
(2013/02/04 4:44), dmp wrote:

Review for patch:

org/postgresql/ds/common/BaseDataSource:

1. databaseName - Was null possibly so getURL(), getReference(), &
writeBaseObject() checked,
OK

2. binaryTransferEnable/Disable - Same as databaseName for
writeBaseObject(),
OK

3. logLevelSet - Needed to Address Bug this patch applies to directly,
OK

4. receiveBufferSize & sendBufferSize - Addresses lack of getter
Methods,
OK

5. getURL() - Changed from private to public, Why?, DriverManager
Contains
no such public method nor Does the Java API define for interface
DataSource,
OK???

6. user & password - Even if 5. approved public why would these be
open to
a public interface for returning these values with URL. In a basic
instinct I feel this is security risk.
NOT OK

7. sslFactory - Proper check for null in getReference(),
OK

8. applicationName - Proper check for null in getReference(),
OK

org/postgresql/test/jdbc2/optional/BaseDataSourceTest:

1. I'm not proficient with JUnit, but appears to add testing for
getURL()
in 5. above. If that is not approved then test suite addition is
not needed.
OK?

danap

Chen Huajun wrote:

danap,

Please use the new patch.
I add a test case,and fixed a mistake in the old one.

Chen Huajun
(2013/02/03 10:32), dmp wrote:

Hello,

I can review this tomorrow and get back by Monday, or sooner.

danap.

Chen Huajun wrote:

Hi

I have made a new patch for BaseDataSource ,please check it.
Some modifications in getReference() is just for keeping
the same style to other String parameters .

Chen Huajun

--
Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-jdbc