#17 supportsMethod check not strict enough

closed
nobody
None
5
2006-01-25
2005-08-22
Anonymous
No

C3P0ImplUtils.supportsMethod(...) returning true does
not guarantee that the method is supported. In
particular, when using the jconn2.jar (5.5) and Sybase
12.5, setTypeMap() is not actually supported and will
throw an UnsupportedOperationException.

I have attached my fix, which attempts to call the
getter and setter of the property. Note that it hasn't
been rigorously tested, and I'm not sure what the
performance penalties are with this kind of checking.

As a design note, the the supportsMethod information
probably shouldn't be associated with the connection
(as in NewPooledConnection) but should be associated
with the pool of connections - so you only need to
establish method support per pool, rather than per
physical connection.

Discussion

  • Steve Waldman

    Steve Waldman - 2005-12-22

    Logged In: YES
    user_id=175530

    I'm sorry for the very long delay, c3p0 fixes had to take a hiatus. You are very
    right that the supportsXXX methods only do a trivial check -- they test whether
    an object claims to support, not whether it actually supports the method. I
    prefer to leave this convention alone, but I've added extra testing elsewhere to
    prevent c3p0 from erroneously calling commonly unsupported methods. There's
    something of a fix to this in 0.9.0.2, but that introduced problems for some
    users, a hopefully better resolution will be in 0.9.0.3 very shortly.

     
  • Steve Waldman

    Steve Waldman - 2006-01-25

    Logged In: YES
    user_id=175530

    although the supportsXXX methods are still a trivial test (does the method
    exist?) c3p0 no longer calls the problematic methods even where drivers appear
    to implement them. thanks again, very much, for the report and proposed fix.

     
  • Steve Waldman

    Steve Waldman - 2006-01-25
    • status: open --> closed
     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks