[
http://issues.apache.org/jira/browse/DERBY-1675?page=comments#action_12433535 ]
Francois Orsini commented on DERBY-1675:
----------------------------------------
Here are my comments.
Very good changes indeed Sunitha. My comments are ONLY minor ones. Mostly
recommendations with some of the comments, etc.
1) java/drda/org/apache/derby/impl/drda/NetworkServerControlImpl.java
A) Static block:
+ // Sun JCE does not have support for EUSRIDPWD, whereas
+ // most versions of IBM JCE have support for this. Hence
+ // find out if the server can support EUSRIDPWD.
Minor nit but EUSRIDPWD is not part of the JVM - It is just that some JCE
provider(s) may not have DH support with a Prime of 32-bytes. I
would have phrased this differently, such as:
// DRDA Specification for the EUSRIDPWD security mechanism
// requires DH algorithm support with a 32-byte prime to be
// used. Not all JCE implementations have support for this.
// Hence here we need to find out if EUSRIDPWD can be used
// with the current JVM.
Again, this is just my recommendation to phrase this differently - Take it or
leave it :)
+ /**
+ * EUSRIDPWD support depends on the availability of the
+ * algorithm in the JCE implementation in the classpath
+ * of the server. At runtime, information about this
+ * capability is figured out.
+ * @return whether EUSRIDPWD is supported or not
+ */
B) supportsEUSRIDPWD():
Here I would have just put the following comments:
/**
* This method returns whether EUSRIDPWD security mechanism
* is supported or not. See class static block for more
* info.
* @return true if EUSRIDPWD is supported, falso otherwise
*/
I think you need 1 more CR (line space before the next getIntPropVal())
2) +++
java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/protocol.tests
-readScalar2Bytes SECMEC 3// SECMEC
-readScalar2Bytes SECMEC 9// SECMEC
-readScalar2Bytes SECMEC 4// SECMEC
-readScalar2Bytes SECMEC 8// SECMEC
-readScalar1Byte SECCHKCD 1// SECMEC
+readSecMecAndSECCHKCD // read secmec values and secchkcd
We're loosing the returned list to check here but I guess it is not obvious to
have this protocol test driver behave conditionally based on the JVM being run,
etc...since there is no canon's for this test if am not mistaken...Anyway, too
much of a big deal I'd say.
I have also tested the patch with Solaris / jdk 1.5 ...
Cheers.
> Network Server should not send to client that it supports EUSRIDPWD when
> running against Sun JVM
> ------------------------------------------------------------------------------------------------
>
> Key: DERBY-1675
> URL: http://issues.apache.org/jira/browse/DERBY-1675
> Project: Derby
> Issue Type: Improvement
> Components: Network Server
> Affects Versions: 10.0.2.0, 10.0.2.1, 10.1.1.0, 10.2.1.0, 10.1.2.1,
> 10.1.3.0, 10.1.3.1
> Reporter: Sunitha Kambhampati
> Assigned To: Sunitha Kambhampati
> Priority: Minor
> Attachments: derby1675.diff.txt, derby1675.stat.txt
>
>
> As part of ACCSECRD, if the server does not accept the security mechanism
> sent by the client, the server will send a list of security mechanism that
> it supports. Currently even when the server is running with sun jvm, it will
> still send EUSRIDPWD as a sec mec that it supports, which is incorrect. The
> server should test if it can support EUSRIDPWD dynamically and if it does,
> only then send EURRIDPWD as an option that it supports.
> see DRDAConnThread.writeACCSECRD(int)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
