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

Chen, Dana,

Applied and thanks for all your efforts

Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca

On Mon, Feb 4, 2013 at 12:11 PM, dmp <danap(at)ttc-cmc(dot)net> wrote:

> 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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/**mailpref/pgsql-jdbc<http://www.postgresql.org/mailpref/pgsql-jdbc>
>