Security fixes are back; other fixes can go in. Time for build 18?

Joe Darcy Joe.Darcy at Sun.COM
Mon Jan 11 17:09:42 PST 2010


On 01/11/10 05:03 PM, Andrew John Hughes wrote:
> 2010/1/12 Joe Darcy <Joe.Darcy at sun.com>:
>   
>> On 01/11/10 03:53 PM, Kelly O'Hair wrote:
>>     
>>> Andrew John Hughes wrote:
>>>       
>>>> 2010/1/11 Joseph D. Darcy <Joe.Darcy at sun.com>:
>>>>         
>>>>> Joseph D. Darcy wrote:
>>>>>           
>>>>>> Andrew John Hughes wrote:
>>>>>>             
>>>>>>> 2010/1/8 Joe Darcy <Joe.Darcy at sun.com>:
>>>>>>>
>>>>>>>               
>>>>>>>> Andrew John Hughes wrote:
>>>>>>>>
>>>>>>>>                 
>>>>>>>>> 2010/1/7 Joseph D. Darcy <Joe.Darcy at sun.com>:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                   
>>>>>>>>>> Andrew John Hughes wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                     
>>>>>>>>>>> 2009/12/25 Andrew John Hughes <gnu_andrew at member.fsf.org>:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>                       
>>>>>>>>>>>> 2009/12/24 Joseph D. Darcy <Joe.Darcy at sun.com>:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>                         
>>>>>>>>>>>>> Andrew John Hughes wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>                           
>>>>>>>>>> [big snip]
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                     
>>>>>>>>>>>>> The com.sun.java.swing package in OpenJDK should have the same
>>>>>>>>>>>>> effective
>>>>>>>>>>>>> compile-time visibility as in Sun JDK.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>                           
>>>>>>>>>>>> I don't know what that is; does this mean
>>>>>>>>>>>> com.sun.java.swing.plaf.nimbus is hidden in the proprietary JDK6?
>>>>>>>>>>>>  I
>>>>>>>>>>>> don't mind either way, it just seems that the other plaf packages
>>>>>>>>>>>> are
>>>>>>>>>>>> visible.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>                         
>>>>>>>>>>>>> I'm going to start taking my vacation in earnest now so we can
>>>>>>>>>>>>> finish
>>>>>>>>>>>>> working through this issue early in 2010.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Happy holidays,
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>                           
>>>>>>>>>>> Happy new year!  Any more thoughts on the above?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>                       
>>>>>>>>>> Yes, easing back from vacation and donning my fedora and bullwhip,
>>>>>>>>>> I've
>>>>>>>>>> dug
>>>>>>>>>> into what is going on here.
>>>>>>>>>>
>>>>>>>>>> In brief, make/common/Release.gmk has a list of packages to exclude
>>>>>>>>>> from
>>>>>>>>>> the
>>>>>>>>>> ct.sym warning (6476749: "Exclude Swing plaf classes from Sun
>>>>>>>>>> Proprietary
>>>>>>>>>> warning"); from
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> http://hg.openjdk.java.net/jdk6/jdk6/jdk/raw-file/c00f461c45bc/make/common/Release.gmk
>>>>>>>>>>
>>>>>>>>>> # The compiler should not issue a "Sun Propietary" warning when
>>>>>>>>>> compiling
>>>>>>>>>> # classes in the com.sun.java.swing.plaf packages, since we've
>>>>>>>>>> always
>>>>>>>>>> # allowed, and even advocated, extending them (see bug 6476749).
>>>>>>>>>> #
>>>>>>>>>> # This approach is NOT to be used as a general purpose way to avoid
>>>>>>>>>> such
>>>>>>>>>> # compiler warnings for non-core packages. The correct way is to
>>>>>>>>>> document
>>>>>>>>>> # the packages in NON_CORE_PKGS.gmk, and include them in the
>>>>>>>>>> NON_CORE_PKGS
>>>>>>>>>> # definition.
>>>>>>>>>> #
>>>>>>>>>> # Swing has taken this approach only as a temporary measure to
>>>>>>>>>> avoid
>>>>>>>>>> # the compiler warnings until we can properly document these
>>>>>>>>>> packages.
>>>>>>>>>> # This is covered under 6491853.
>>>>>>>>>> EXCLUDE_PROPWARN_PKGS = com.sun.java.swing.plaf          \
>>>>>>>>>>                    com.sun.java.swing.plaf.windows  \
>>>>>>>>>>                    com.sun.java.swing.plaf.motif    \
>>>>>>>>>>                    com.sun.java.swing.plaf.gtk
>>>>>>>>>>
>>>>>>>>>> In Sun's 6 update train, com.sun.java.swing.plaf.nimbus is included
>>>>>>>>>> on
>>>>>>>>>> that
>>>>>>>>>> package list.  Therefore, the test file in question compiles
>>>>>>>>>> without
>>>>>>>>>> warning
>>>>>>>>>> using Sun's 6 update release.  The corresponding addition to this
>>>>>>>>>> list
>>>>>>>>>> has
>>>>>>>>>> *not* been made in JDK 7, which is probably just an oversight.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                     
>>>>>>>>> In 7, it's now a standard API javax.swing.plaf.nimbus.  So you'll
>>>>>>>>> find
>>>>>>>>> it listed in make/docs/CORE_PKGS.gmk instead:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                   
>>>>>>>> Yes, for JDK 7 javax.swing.plaf.nimbus is a core package.
>>>>>>>>
>>>>>>>>
>>>>>>>>                 
>>>>>>>>>  javax.swing.plaf.basic                         \
>>>>>>>>>  javax.swing.plaf.metal                         \
>>>>>>>>>  javax.swing.plaf.multi                         \
>>>>>>>>>  javax.swing.plaf.nimbus                        \
>>>>>>>>>  javax.swing.plaf.synth                         \
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                   
>>>>>>>> However, javax.swing.plaf.nimbus being a core package in JDK 7 has no
>>>>>>>> direct
>>>>>>>> bearing how com.sun.java.swing.plaf.nimbus should be regarding in
>>>>>>>> either
>>>>>>>> OpenJDK 6 or the 6 update release.
>>>>>>>>
>>>>>>>> Generally, the "exported external" APIs in the JDK are in the java.*
>>>>>>>> and
>>>>>>>> javax.* namespaces.  These are the APIs that everyone is supposed to
>>>>>>>> call,
>>>>>>>> etc.  Nearly all of the sun.* and com.sun.* API are not in this
>>>>>>>> category;
>>>>>>>> rather they are only intended to be used by a restricted set of
>>>>>>>> parties,
>>>>>>>> such as the JDK implementation.  The Swing packages listed in
>>>>>>>> EXCLUDE_PROPWARN_PKGS are an exception to this rule.
>>>>>>>>
>>>>>>>> The ct.sym feature is a compile-time proto module system to add an
>>>>>>>> enforcement mechanism to this long-standing API policy.
>>>>>>>>
>>>>>>>>
>>>>>>>>                 
>>>>>>> Indeed.  I wasn't saying that it should be treated the same.  I was
>>>>>>> responding to your point that com.sun.java.swing.plaf.nimbus was not
>>>>>>> added to the exclusion list in OpenJDK7 i.e. it obviously hasn't
>>>>>>> because the package no longer exists in 7 and had to be recreated from
>>>>>>> 6.
>>>>>>>
>>>>>>>
>>>>>>>               
>>>>>>>>>> I'd support com.sun.java.swing.plaf.nimbus being included in this
>>>>>>>>>> list
>>>>>>>>>> in
>>>>>>>>>> OpenJDK 6 as long as
>>>>>>>>>>
>>>>>>>>>> * The API of the package is the same as in Sun's 6 update release
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                     
>>>>>>>>> We have no way of verifying what's in the 6 update release as it is
>>>>>>>>> proprietary software.
>>>>>>>>>
>>>>>>>>>                   
>>>>>>>> After hacking together a small annotation processor, I will send the
>>>>>>>> source
>>>>>>>> for the processor and a list of the signatures of the methods and
>>>>>>>> constructors of the types in the Nimbus package in the 6 update
>>>>>>>> train.
>>>>>>>>
>>>>>>>>
>>>>>>>>                 
>>>>>>> But what do we do if they don't match?  Are we supposed to start
>>>>>>> recreating missing methods from the proprietary 6 code when we have no
>>>>>>> idea how they function?
>>>>>>> The code from 7 is all we have.
>>>>>>>
>>>>>>>               
>>>>>> We can evaluate the difference once we see them.  Options include not
>>>>>> exposing the com.sun package in OpenJDK 6 and implementing any missing
>>>>>> functionality or everything might match and they'll be nothing to do
>>>>>> :-)
>>>>>>
>>>>>>             
>>>>>>>>>  The Nimbus code in OpenJDK6 is a backport of
>>>>>>>>> what's in 7, moved back to the com.sun.java.swing.plaf.nimbus
>>>>>>>>> namespace wihich I believe was used in the proprietary 6 update
>>>>>>>>> train.
>>>>>>>>>
>>>>>>>>> Remember that this is an open-source project; regardless of whether
>>>>>>>>> arbitrary bits of code are made invisible or not in a unpatched
>>>>>>>>> build
>>>>>>>>> of OpenJDK6, anyone can easily read the code and even rip the ct.sym
>>>>>>>>> hack right back out again.
>>>>>>>>>
>>>>>>>>>                   
>>>>>>>> Just because something is technically feasible does not mean it is
>>>>>>>> advisable.  Many technically possible changes are not done so that
>>>>>>>> various
>>>>>>>> other goals can be met, such as following the Java SE 6
>>>>>>>> specification,
>>>>>>>> etc.
>>>>>>>>
>>>>>>>> If one takes steps to circumvent ct.sym or other enforcement
>>>>>>>> mechanism,
>>>>>>>> it
>>>>>>>> is clear who should be responsible for any adverse consequences.  For
>>>>>>>> example, if someone circumvents ct.sym and then a type being accessed
>>>>>>>> is
>>>>>>>> changed in a subsequent release, that is a "close not a bug"
>>>>>>>> situation.
>>>>>>>>
>>>>>>>>
>>>>>>>>                 
>>>>>>> I didn't say it was advisable.  I'm just pointing out that it was a
>>>>>>> weak solution to begin with, and even weaker now that people can study
>>>>>>> and alter the source code rather than just having a binary blob to
>>>>>>> work with.
>>>>>>>
>>>>>>>               
>>>>>> Stronger mechanisms are planned for JDK internal implementation classes
>>>>>> new in JDK 7 where backwards compatibility is not a concern.
>>>>>>
>>>>>>             
>>>>>>> I'm a fan of warning about the use of this code so it's visibly a bug
>>>>>>> and something that should be fixed.  I also think it's perfectly
>>>>>>> legitimately to close bugs that refer to code that's not part of the
>>>>>>> public API (though Sun don't produce OpenJDK6 binaries anyway).
>>>>>>> Plenty of other open source projects do that without actively trying
>>>>>>> to stop people using it in the first place.
>>>>>>>
>>>>>>> I don't like the idea of code appearing to disappear as it just causes
>>>>>>> confusion, usually for end users who don't work on the code itself.
>>>>>>> It also impacts debugging, see Andrew Haley's blog on trying to track
>>>>>>> down an issue in javac: http://andrew-haley.livejournal.com/  The very
>>>>>>> fact that you term it 'an enforcement mechanism' suggests treating
>>>>>>> developers as potential criminals rather than as equals.
>>>>>>>
>>>>>>>               
>>>>>> There are many enforcement mechanism in the Java platform.  The class
>>>>>> file
>>>>>> verifier.  The type checker in javac.  The applet sandbox.  The
>>>>>> security
>>>>>> model.  No criminality need be inferred.
>>>>>>
>>>>>>             
>>>>>>>>>  Additionally, not doing this would make
>>>>>>>>> OpenJDK6 different from the proprietary release -- as you mention
>>>>>>>>> above, it doesn't mask this package.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                   
>>>>>>>> Because the proprietary release does not unmask this package, if
>>>>>>>> OpenJDK
>>>>>>>> 6
>>>>>>>> also unmasks this package, IMO it is important that the OpenJDK 6 API
>>>>>>>> match
>>>>>>>> the API exposed in the proprietary release.
>>>>>>>>
>>>>>>>>
>>>>>>>>                 
>>>>>>> I'm confused here -- does the proprietary JDK6 make this package
>>>>>>> visible or not?
>>>>>>>               
>>>>>> The com.sun.* nimbus package is visible in the proprietary JDK6
>>>>>> starting
>>>>>> (I assume) in 6u10 when Nimbus was added; it is visible in 6u17 where I
>>>>>> was
>>>>>> able to successfully compile your test program.
>>>>>>             
>>>>>>> You seemed to be saying earlier that it did and your
>>>>>>> idea of checking what the API is using an annotation processor also
>>>>>>> suggests this.
>>>>>>>
>>>>>>>               
>>>>>> Correct.
>>>>>>
>>>>>>             
>>>>>>> I agree it's important they match.  I just don't see what we can do if
>>>>>>> they don't.
>>>>>>>
>>>>>>>               
>>>>>> Let's see how far the two packages are from an alpha rename and
>>>>>> evaluate
>>>>>> our options.
>>>>>>
>>>>>>             
>>>>> Greetings.
>>>>>
>>>>> Before writing an annotation processor, I tried a simple diff.  Good
>>>>> news!
>>>>>  To make the API signatures of the com.sun.java.swing.plaf.nimbus
>>>>> packages
>>>>> match in OpenJDK 6 and the proprietary JDK the following two classes in
>>>>> OpenJDK 6 need to be made public:
>>>>>
>>>>> LoweredBorder.java
>>>>> TableScrollPaneCorner.java
>>>>>
>>>>> So I will approve the following change
>>>>>
>>>>> * The two classes above are changed to be public
>>>>> * The package in question is added to the EXCLUDE_PROPWARN_PKGS list in
>>>>> make/common/Release.gmk
>>>>>
>>>>> as long as Kelly also reviews any build impact.
>>>>>           
>>> Makefile change seems harmless.
>>>
>>> -kto
>>>
>>>       
>> With that, consider the change approved; please push using bug id 6915980
>> "Do not issue build warnings for use of com.sun.java.swing.plaf.nimbus."
>>
>> Thanks,
>>
>> -Joe
>>
>>     
>
> Pushed:http://hg.openjdk.java.net/jdk6/jdk6/jdk/rev/f6d8d0c0e6ad
>
> There's another Nimbus patch that should be backported before b18:
>
> # HG changeset patch
> # User peterz
> # Date 1258555006 -10800
> # Node ID fc3997fd1bcebf74031fdc43d44b371e4be3d29b
> # Parent  4f0275ea56fdb3f8199d16951954cfee80f931c2
> 6882917: Nimbus and DefaultTableCellRenderer: must start with normal background
> Reviewed-by: rupashka
> http://hg.openjdk.java.net/jdk7/jdk7/jdk/rev/fc3997fd1bcebf7403
>
>   

You're approved to push a port of this fix assuming the patch applies 
cleanly.

Thanks,

-Joe

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/jdk6-dev/attachments/20100111/1f3049fd/attachment-0001.html 


More information about the jdk6-dev mailing list