|
Here is a fix. The fix uses two techniques to detect the case where the DB2Driver class did not come from the expected jar file in the classpath, but rather was loaded from some location (e.g., JAVA_HOME/jre/lib/ext):
1) Using the JavaWorld Tip 105 technique, we ask the DB2Driver classfile itself for the location from which it was loaded, and we print that location, rather than just assuming that the class was loaded from the db2jcc.jar in the application class path. 2) If the DB2Driver class has a different class loader than the sysinfo Main class's class loader, we print a warning. To see the effects of this patch, you can run the following experiment: a) First, run sysinfo with db2jcc.jar in your classpath as normal. b) Second, copy db2jcc.jar into JAVA_HOME/jre/lib/ext and re-run sysinfo with the same classpath. Note that it detects where DB2Driver was *really* loaded from, and displays that information. Hi Brian. I'd like to clarify the scope of what you are trying to do with this patch. Is it only to correctly report the location of db2jcc.jar if it is both in the classpath and loaded by a different classloader or should it cover 1) other jars such as derby.jar derbyclient.jar etc..? 2) The case where there is no jar in the classpath at all, but only loaded by a classloader? It seems that if the jar is loaded by the classloader but is not in the classpath it would not report at all, but that perhaps is not the problem you are trying to solve. On the patch I think the System.out can be removed. Probably just the odd location reporting would be good enough. I did not quite understand this condition" if (loc != null && loc.startsWith("file:") && loc.indexOf("!") > 0) What does the occurance of ! mean? Thanks for looking at this issue. Having sysinfo properly report jar locations when jars are not loaded from the classpath is really important. Kathey Hi Kathey, thanks for the feedback.
I think we can make sysinfo do a variety of things; I was treading cautiously because I'm unsure of the requirements. The current implementation in the reportCloudscape() and getAllInfo() methods is basically a "for each entry in the classpath, print some info about it", and so I was trying to fit into that implementation without perturbing it very much. Therefore, as you noticed, my change only affects the case where db2jcc.jar is in the classpath but the DB2Driver class turns out to get loaded from some other location. The code that I altered is triggered by finding db2jcc.jar in the classpath. It would be possible to write an alternate implementation, such as the following: - Construct a list of "sentinel" classes, and the jar files that we expect such classes to be loaded from - For each sentinel class, load it, then look to see from which jar it was loaded. - Print the information about the jar, and decorate that message in a special way if the class was not loaded from the expected jar. It sort of seems like that is the intent of the useMe() and tryAllClasspaths() methods, which appear to be run only if you pass the "-cp" argument to sysinfo (http://db.apache.org/derby/docs/10.1/tools/ctoolssysinfo1002931.html), which I admit I had overlooked. I'll take a look at this part of sysinfo, and investigate how I could improve it, as well. The business about if (loc != null && loc.startsWith("file:") && loc.indexOf("!") > 0) is as follows: when we call getResource(x.y.class).getFile(), the string that we get back looks something like:
file://path/to/the/jar!x.y.class so I'm just checking for a string of that format and sucking the file name out of it. I'll add a comment to make that more clear. Hi Kathey,
I re-worked my patch based on your comments, and I think it is more useful now. I abstracted the code out into a subroutine so that it could be shared, added some comments, and, most importantly, used the code in the "sysinfo -cp" code path, where it actually makes a lot more sense and is more useful. The net effect of this patch is: 1) If you run "sysinfo -cp" (and any of its variants), sysinfo now reports the actual location where the class was loaded from, in addition to telling you whether or not it loaded the class successfully. 2) If you run just "sysinfo", when it is processing the db2jcc.jar file in your classpath, it now reports the actual location where the DB2Driver class was loaded from, which may not be the db2jcc.jar file in your classpath (if you've placed db2jcc.jar somewhere else). Please have a look at this new patch when you get a chance and tell me what you think. I was thinking I would go ahead and commit this patch since I know it has been sitting for a while, but I ran sysinfo quickly with the patch and even though I have both db2jcc.jar and db2jcc_license_c.jar in my classpath, sysinfo shows me two db2jcc.jars and no db2jcc_license_c.jar
With the patch: --------- Derby Information -------- JRE - JDBC: J2SE 1.4.2 - JDBC 3.0 [D:\p4\marsden_patch\classes] 10.2.0.0 alpha - (1) [/D:/p4/marsden_patch/drda/jcc/2.4/db2jcc.jar] 2.4 - (17) [/D:/p4/marsden_patch/drda/jcc/2.4/db2jcc.jar] 2.4 - (17) ------------------------------------------------------ Without the patch: --------- Derby Information -------- JRE - JDBC: J2SE 1.4.2 - JDBC 3.0 [D:\p4\marsden_patch\classes] 10.2.0.0 alpha - (1) [D:\p4\marsden_patch\drda\jcc\2.4\db2jcc.jar] 2.4 - (17) [D:\p4\marsden_patch\drda\jcc\2.4\db2jcc_license_c.jar] 2.4 - (17) ------------------------------------------------------ That's an interesting point. I think that the line in question was rather misleading, though:
[/D:/p4/marsden_patch/drda/jcc/2.4/db2jcc.jar] 2.4 - (17) The "2.4 - (17)" in this line is printing the results of calling DB2Driver.getMajorVersion() (2), DB2Driver.getMinorVersion (4), and DB2Driver.getJCCBuildNumber() (17). But, we're calling these methods on the DB2Driver which is in db2jcc.jar! That is, from what I can tell, sysinfo is claiming to report that db2jcc_license_c.jar is version 2.4, build 17, but it's not actually using any information from db2jcc_license_c.jar to derive that information. I don't know that db2jcc_license_c.jar even has a concept of a version number. From what I can see, it has only a MANIFEST, and a single file, namely com.ibm.db2.jcc.licenses.DB2J. Is there any documentation available on the DB2J class? It would be easy to alter the proposed patch so that sysinfo reverts back to its current behavior (that is: print the filename of the db2jcc_license_j jar file, and the version information from the DB2Driver which is in the classpath); my question is: is that really a useful behavior, or is there something more useful that we could print about the db2jcc_license_c.jar file specifically? There is no special version information for the license file or documentation for the DB2J class. It has never really changed from release to release so version information is not really relevant. It is very important that it be listed in sysinfo because I would say its absence is the #1 reported reason JCC doesn't work. It's mere existance seems to be the key.
As long as it prints the filename it is fine for it to print the DB2Driver version or even no version at IMO Hi Kathey, please consider this most recent patch. I've reverted the handling of db2jcc_license_c.jar back to the pre-patch behavior, to wit: if we find db2jcc_license_c.jar in the classpath, and if we are also able to load some version of the DB2Driver class or the DB2Version class, then we display the db2jcc_license_c.jar file that we found in the classpath, and we also display the version number information that we fetched from DB2Driver/DB2Version.
Here's a bunch of the various tests I ran; in all the cases, I think that the output of the new SysInfo is correct, but I'd like you to confirm my results, please :) 1, 2) java -cp derby.jar:derbytools.jar sysinfo [-cp] 3, 4) java -cp derby.jar:derbytools.jar:db2jcc.jar sysinfo [-cp] 5, 6) java -cp derby.jar:derbytools.jar:db2jcc.jar:db2_license_c.jar sysinfo [-cp] 7, 8) java -cp derby.jar:derbytools.jar:db2_license_c.jar sysinfo [-cp] 9, 10) (with db2jcc.jar in $JAVA_HOME/jre/lib/ext), java -cp derby.jar:derbytools.jar:db2_license_c.jar sysinfo [-cp] Cases (7) and (8) are particularly interesting. Both the current sysinfo, and the one with my patch, print no information about db2jcc_license_c.jar if we cannot load any version of the DB2Driver or DB2Version classes. I think this is reasonable, but I'd like to hear your opinion on this. An alternate implementation would be, as you suggested in your comment, to print a line for db2jcc_license_c.jar in this case, but don't print any version information. I guess I don't know how important this is. In your comment, you noted that the primary user mistake was to have db2jcc.jar, but not db2jcc_license_c.jar, rather than vice-versa, so I don't know if it's worth adding a bunch more code to try to do something useful about the case where we have db2jcc_license_c.jar in the classpath, but NOT db2jcc.jar. Please tell me what you think. thanks, bryan Hi Bryan,
I took a look at version 3 of your patch. It looks good, just a couple comments: - On Windows you get a leading slash before the drive spec (i.e. /C:/mydir/jars/...). Might be worth fixing, but it is just a 'polish' issue. - In test case 9 above, db2jcc_license_c.jar shows up in the output, but db2jcc.jar does not. It does appear as found in testcase 10. This behavior is the same as the current sysinfo. It would be nice if this were fixed, but this could be treated as a separate issue. - My opinion is that it would be nice to have db2jcc_license_c.jar appear in the output if db2jcc.jar is not present. That can also be fixed as a separate issue, though. I also stumbled across some really bizarre behavior (already present - not introduced by your patch), if you put derby.jar or one of the other product jars into the extensions directory, while there are others on the classpath. To see what I mean, try putting just derbynet.jar in the extensions, or putting derby.jar into the extensions dir while you have another on your classpath. Hi Andrew, Thanks for reviewing my changes.
Your point about the Windows file syntax is a really good one, and I think it's worth fixing. Unfortunately, the best thing I've been able to think of so far is the rather crude: // On Windows, file: scheme URLs look like file:/c:/derby/..., so strip that leading slash, too: if (loc.length() > 3 && loc.charAt(0) == '/' && Character.isLetter(loc.charAt(1)) && loc.charAt(2) == ':') loc = loc.substring(1); Can you think of a cleaner way to code this? Hi Bryan, here's an alternative:
if (System.getProperty("os.name").startsWith("Windows")) loc = loc.substring(1); I'll leave it up to you. Your method is just as good. Is this code used?
+ // Given a loaded class, and the name by which we loaded that class, this + // routine asks the class's class loader for information about where the + // class was loaded from. Typically, this is a file, which might be + // either a class file or a jar file. The routine figures that out, and + // returns the name of the file. If it can't figure it out, it returns null + private static String getFileWhichLoadedClass(Class c, String cName) + { + String className = "/" + cName.replace('.', '/') + ".class"; + java.net.URL url = c.getResource(className); because I thought a class loader was not allowed to return the .class file as a resource for security reasons. Class.getResource() doesn't return the .class file itself, it returns the URL of the location from which the class in question has already been loaded. This is used to determine the actual location of the loaded class, whether it was loaded by the extensions mechanism or from the classpath.
Because we're just asking for the location of the file, I don't believe there are any security manager issues. If we tried to load the class from the URL, that might be a problem if we don't have sufficient permissions to access the class. The comment for ClassLoader.getResource says:
a URL for reading the resource, or null if the resource could not be found or the caller doesn't have adequate privileges to get the resource. Thus if you don't have permission to read the class you should get null back. Not sure how this affects this patch, I know in the class loader that loads from the database I implemented to return null for any request for a file ending in .class, I'd just assumed most class loaders did that. Come to think of it, has any work been done to sysinfo to make it run properly with the security manager enabled? It definitely accesses System properties and such without the requisite doPrivileged() blocks.
Answered my own question: java -cp jars/insane/derby.jar -Djava.security.manager org.apache.derby.tools.sysinfo ------------------ Java Information ------------------ Java Version: 1.4.2_09 Java Vendor: Apple Computer, Inc. Java home: Security Exception: java.security.AccessControlException: access denied (java.util.PropertyPermission java.home read) Java classpath: Security Exception: java.security.AccessControlException: access denied (java.util.PropertyPermission java.class.path read) OS name: Mac OS X OS architecture: ppc OS version: 10.4.3 Java user name: Security Exception: java.security.AccessControlException: access denied (java.util.PropertyPermission user.name read) Java user home: Security Exception: java.security.AccessControlException: access denied (java.util.PropertyPermission user.home read) Java user dir: Security Exception: java.security.AccessControlException: access denied (java.util.PropertyPermission user.dir read) java.specification.name: Java Platform API Specification java.specification.version: 1.4 --------- Derby Information -------- JRE - JDBC: J2SE 1.4.2 - JDBC 3.0 [/org/apache/derby/info/DBMS.properties] 10.2.0.0 alpha - (233223M) ------------------------------------------------------ ----------------- Locale Information ----------------- ------------------------------------------------------ So that would be a 'no'. :-) Hi Dan, I was looking at the 1.3.1 javadoc at the time, it's not very specific about the security ramifications of the methods in question. oh well.
Anyway, if we didn't have permissions to access the class, we'd already be in trouble by the time this code was called, since we would have already attempted to load the class to pass it into the getFileWhichLoadedClass method. Attached is an updated version of the patch incorporating Andrew's suggestion to clean up the display on Windows.
I haven't done anything about the security manager issues raised by Dan because I don't understand them well enough to know what to do. Andrew McIntyre suggested that the getCodeSource() method included in the patch for
I will pursue Andrew's suggestion, and will investigate using that code in sysinfo to improve its behavior. I changed the title of this issue to more accurately depict the seriousness of the problem.
Currently sysinfo will print the wrong information if Derby is loaded with a custom ClassLoader. See this thread on the derby-user list http://www.nabble.com/How-can-I-get-sysinfo-for-a-custom-Derby-classloader--t1171044.html I am changing this to Critical as its impact is apparent even when users are just using classpath and derby automatically loads jars. For example amidst much confusion about
>derbytools.jar will automatically put derbyclient.jar in the >classpath. Since you have derbytools.jar from trunk before >derbyclient.jar version 10.1 in your classpath, derbyclient.jar will >be shadowed. Sysinfo will however report that you are using the 10.1 >version of derbyclient.jar, since it only sees the CLASSPATH variable. It seems to me perhaps this particular autoload is not a good idea but I will post separately about that but this bug seems to make sysinfo not very useful for finding out jar versions. Here's another version of the patch for review.
The primary difference between this version and the previous 30-Nov-2005 version is that I've switched to using the getProtectionDomain().getCodeSource() technique proposed by David vanCouvering. I picked up the code from a patch proposal to Here's a few remarks: - I've tested this code on Linux and Windows, with various combinations of jars and classes in the classpath and in the JRE/lib/ext, with and without a Security Manager in place, using various combinations of sysinfo arguments. Unfortunately, there are a *lot* of permutations here, and I probably missed some. Still, for the cases I tested, it behaved well. - Running sysinfo -cp works pretty well with a Security Manager, but running sysinfo *without* the -cp argument fails pretty badly under a Security Manager, as Andrew commented earlier in this issue. I didn't try to fix those problems, though I added a print statement for one particularly egregious case in which a security exception while fetching the CLASSPATH value was quietly swallowed with no output. Now it will at least tell you why it failed to process your CLASSPATH. - To run sysinfo -cp with a Security manager, you need a trivial policy file. I used: grant { permission java.lang.RuntimePermission "getProtectionDomain"; }; which obviously isn't ideal, but it was sufficient for testing. - I also did some testing of the case raised by Knut Anders, in which having derbyclient.jar in the manifest of derbytools.jar causes derbyclient.jar to be pulled from the same directory where derbytools.jar was loaded, even if you have some other derbyclient.jar in your classpath. In my testing, "sysinfo -cp" behaves correctly in this case, and reports the *real* derbyclient.jar that is in use, which is good. Unfortunately, the code which explicitly parses the classpath (sysinfo with no arguments) is still fooled by this case and gives a misleading response. Fundamentally, as I said in my 23-Nov-2005 comment, I think that the algorithm used by Main.getAllInfo() is just an inferior algorithm. Textual parsing of the CLASSPATH entry is just too easily fooled by a variety of modern class loading situations. But I think that the algorithm used by "sysinfo -cp" is fundamentally sound, and I think that this patch improves it to make sysinfo better than it was, even if it's not where we want it to be, yet. Please have a renewed look at this patch, and tell me what you think. thanks, bryan Hi Bryan,
This looks good to me. Dan would need to comment on the security ramifications of needing the getProtectionDomain permission. If you have a chance, take a look at my patch for Minor comments on the code
- would be good to make the comment for the new method getFileWhichLoadedClass a javadoc method (use /** ... */) show that it shows up in the generated javadoc and IDE's. - why check getProtectionDomain() for a null return when the javadoc does not indicate it can return null? I personally don't like this coding style as it add code for no value, should we check all methods that return references for null, even ones like String.trim()? It can make the reader have to look to the javadoc to see in what situations the method can return null, to understand the code, only to find out it can't. I'm trying to think through the ramifications of using getProtectionDomain() and hence possibly needing the associated permission in the policy file. One issue is that this permission might be needed for all derby jar files, since sysinfo is in all jars. It probably comes down to what situations would we expect sysinfo to be executed with a security manager present, in those situations how likely is the policy file going to be configured to support sysinfo? Since sysinfo is really meant to be a quick check of the classpath, not an integral part of an application. On the "windows specific" code for formatting the output of the URL, would this be needed if instead you did something like
+ URL result = cs.getLocation (); String loc; if ("file".equals(result.getProtocol)) loc = result.getPath(); else loc = result.toString(): Regarding Dan's most recent comment, about the Windows-specific path formatting, the suggestion does not seem to work for me, unfortunately:
C:\bryan\src\java\urlloc>type x.java public class x { public static void main(String []args) throws Exception { java.net.URL result = (new x()).getClass(). getProtectionDomain().getCodeSource().getLocation(); System.out.println("proto=" + result.getProtocol()); System.out.println("path=" + result.getPath()); System.out.println("toString=" + result.toString()); } } C:\bryan\src\java\urlloc>java -cp . x proto=file path=/C:/bryan/src/java/urlloc/ toString=file:/C:/bryan/src/java/urlloc/ Sysinfo_Feb28_2006.diff is a revised version of the sysinfo patch proposal incorporating comments from reviewers. There are two changes as compared to yesterday's patch proposal:
1) The method comments have been reformatted to better match javadoc conventions 2) The unnecessary and confusing check of the getProtectionDomain return value has been removed. A few small things:
1 - the class path becoming null issue is probably caused by the reportCloudscape method nulling out the local field classpath on a SecurityException. 2 - Instead of hardcoding the "Unable to analyze class path" message, could you please add a new message to sysinfomessages.properties and use LocalizedResource to get the localized value? 3 - Whether or not allowing getProtectionDomain is an appropriate permission to add, perhaps the getProtectionDomain() call should be wrapped in a try/catch block for a SecurityException so that sysinfo can gracefully fail out of getFileWhichLoadedClass()? At least then in an environment where this is not allowed, we can report the problem with a meaningful error message instead of just falling over. A localized message may be needed here as well. Patch "with_andrews_feedback.diff" incorporates the suggestions from Andrew's review of March 8 (thanks Andrew):
- The message when java.class.path cannot be read is fetched from a translatable property - The code now catches the security exception when getProtectionDomain is disallowed and displays a friendly message fetched from a translatable property. Hi Bryan, the latest patch for this looks good to me. I would say go ahead and commit when you have a chance.
It turns out that there are several tests which run the 'sysinfo' code in situations in which a Java security manager is in place, and in these tests the code is not currently being granted the permissions necessary to display the new detailed information which is added by this patch.
The tests are: - derbynet/sysinfo.java - derybnet/sysinfo_withproperties.java - demo/RunClassPathTester.java The two sysinfo tests run the Network Server under a security manager, and then call the 'sysinfo' command in client/server mode to print the sysinfo output from that Network Server. But the Network Server is not granted the permission to read java.class.path, so that part of the sysinfo output is blank. The RunClassPathTester.java test runs the standalone sysinfo tool under a security manager, passing it the '-cp' flag to print information about Database.class and SimpleApp.class. These classes are indeed in the class path, but the test is not granted the permission to get the ProtectionDomain and CodeSource for them. The result, right now, is that the two Sysinfo tests now produce a new line of output stating that permission has not been granted to read the java.class.path property, and the RunClassPathTester test now produces a new line of output stating that permission has not been granted to get the ProtectionDomain. I think that output is correct, and so I am tempted to update the master output files to reflect this new correct output. Alternatively, I could modify derby_tests.policy to grant additional permissions to the Derby codebase(s). What is the better path to pursue? I believe the goal is to run the tests with as few permissions as possible. Granting additional permissions for these tests might mask security bugs elsewhere in the code.
So, I agree, I think the thing to do is add master files that include the output of the caught SecurityExceptions. Myrna had mentioned enhancing the test harness to be able to provide specific permissions to individual tests. Once that work had been completed, we could add the necessary permissions to these tests only. I thought a JIRA had been field for that, but I wasn't able to find it at the moment. Thanks, Andrew. I agree with you.
I've committed this change: http://svn.apache.org/viewcvs?rev=387599&view=rev I added a brief release note to the CHANGES file mentioning the new behavior and the new security requirement.
Here's a release note:
PROBLEM: Sysinfo classpath information was insufficiently detailed SYMPTOM: Sometimes it was hard to tell where the Derby classes were actually being loaded from in the JVM CAUSE: the algorithm that sysinfo used for analyzing and reporting on the application classpath was not robust. SOLUTION: The sysinfo tool now prints additional information about the origin of the classes and jars that it examines. The origin of a class might be: an entry in the application classpath, an entry in a class loader location list, a jar fetched due to being listed in the manifest entry of another jar, a standard extension in the JRE's extensions directory, a jar installed into the application server, or any of various other possibilities. Note that when sysinfo runs under a Java security manager, it may need special permissions to access this additional information, including the permission to read the java.class.path property, and the permission to call getProtectionDomain on a class. If sysinfo is not granted these permissions, it will display an error message about the security problem in place of displaying the class origin information. SECURITY NOTE: The new permissions are optional. If you do not provide them, you will see a message such as the following in your sysinfo output: Unable to analyze class path: access denied (java.util.PropertyPermission java.class.path read). Such a message does not indicate an error; it simply means that the sysinfo tool was unable to provide detailed classpath information because it did not have permission to read the classpath. In prior releases, under these circumstances, sysinfo would silently print nothing, now it prints an informational message about the reason that it couldn't provide any classpath information. Linking this issue to DERBY-1273.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Closed
Critical
SysInfo does not print the right information when Derby is not loaded through the classpath.
In particular, I think that the following code, if inserted at the right place, enables the sysinfo utility to do a better job of printing out *exactly* the location where the DB2 JDBC driver was loaded from:
this.getClass().getResource("/com/ibm/db2/jcc/DB2Driver.class").getFile()
When I compile and run this code in a standalone Java program, experimenting with various combinations of db2jcc.jar in the classpath versus db2jcc.jar in $JAVA_HOME/jre/lib/ext, the code appears to reliably figure out the actual location from which DB2Driver.class is found.
So I think it should be possible to use this code in derby/impl/tools/sysinfo/Main.java to print a better message about the location from which various classes are loaded.
However, I'm getting a little lost trying to figure out sysinfo/Main.java and sysinfo/ZipInfoProperties.java. :(
It seems to me like the current SysInfo program basically takes the approach of parsing the CLASSPATH variable and printing out information about the things it finds there.
That algorithm works well, except that it is defeated by "magic" locations like $JAVA_HOME/jre/lib/ext.
Therefore, it seems like SysInfo needs to be enhanced to use an additional algorithm like the one I propose above, which is to ask the classloader itself for a file-scheme URL to the actual location of the critical class files, and then to print that information out.
Anyway, that's as far as I got today, and I wanted to record that investigation so I wouldn't lose it.