> On Jan 10, 2017, at 5:03 PM, Xueming Shen <xueming.s...@oracle.com> wrote: > Hi Mandy, > > I spent some time today for JDK-8171830. > > http://cr.openjdk.java.net/~sherman/8172432/webrev2/src/jdk.jartool/share/classes/sun/tools/jar/Main.java.sdiff.html > > <http://cr.openjdk.java.net/~sherman/8172432/webrev2/src/jdk.jartool/share/classes/sun/tools/jar/Main.java.sdiff.html> > --> Line#1851 checkModuleInfo() > > http://cr.openjdk.java.net/~sherman/8172432/webrev2/test/tools/jar/modularJar/Basic.java.sdiff.html > > <http://cr.openjdk.java.net/~sherman/8172432/webrev2/test/tools/jar/modularJar/Basic.java.sdiff.html> > --> Line#739 > > Only after I checked out the output bytes I realized that the current > implementation > for "checkServices", in which it creates a ModuleDescriptor from bytes as > > ModuleDescriptor md = ModuleDescriptor.read(ByteBuffer.wrap(moduleInfoBytes)); > > actually triggers the sanity check inside ModuleInfo.doRead(), which does > validate if all > exported/open packages are contained in the "packages" attribute. >
This is good. jar tool calls ModuleDescriptor.read after ModulePackages attribute is added/updated and this will leverage the ModuleDescriptor validation. > java.lang.module.InvalidModuleDescriptorException: Package jdk.test.bar > missing from ModulePackages attribute > at > java.base/jdk.internal.module.ModuleInfo.invalidModuleDescriptor(ModuleInfo.java:1078) > at java.base/jdk.internal.module.ModuleInfo.doRead(ModuleInfo.java:318) > at java.base/jdk.internal.module.ModuleInfo.read(ModuleInfo.java:141) > at > java.base/java.lang.module.ModuleDescriptor.read(ModuleDescriptor.java:2377) > at jdk.jartool/sun.tools.jar.Main.checkModuleInfo(Main.java:1843) > at jdk.jartool/sun.tools.jar.Main.run(Main.java:289) > at jdk.jartool/sun.tools.jar.Main.main(Main.java:1651) > > Just wanted to confirm with you that we actually don't need to do anything > for 8171830, > other than throwing in a test case (or wrap the > InvalidModuleDescriptorException into > a "jar" exception? > I agree that you can depend on the validation done by ModuleDescriptor::read as long as the jar tool reports the error message gracefully (probably catch InvalidModuleDescriptorException and output the error). It’d be good to add a comment to describe that exports/opens package will be validated. thanks Mandy
