Re: [BUGS]log can not be output when use DataSource

From: Chen Huajun <chenhj(at)cn(dot)fujitsu(dot)com>
To: dmp <danap(at)ttc-cmc(dot)net>
Cc: pgsql-jdbc(at)postgresql(dot)org
Subject: Re: [BUGS]log can not be output when use DataSource
Date: 2013-02-04 03:56:25
Message-ID: 510F3169.3030507@cn.fujitsu.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-jdbc

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
>>
>>
>
>
>
>

Attachment Content-Type Size
pgjdbc_BaseDataSourceV3.patch text/plain 5.5 KB

In response to

Responses

Browse pgsql-jdbc by date

  From Date Subject
Next Message Ian Pilcher 2013-02-04 04:27:25 setTimestamp(int, Timestamp, Calendar) ignoring time zone?
Previous Message Chen Huajun 2013-02-04 02:28:33 Re: [BUGS]log can not be output when use DataSource