Security fixes are back; other fixes can go in. Time for build 18?
Kelly O'Hair
Kelly.Ohair at Sun.COM
Mon Jan 11 15:53:32 PST 2010
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
>>
>> Thanks,
>>
>> -Joe
>>
>
> Great news! Here's a webrev for the revised fix:
> http://cr.openjdk.java.net/~andrew/nimbus/webrev.05/
More information about the jdk6-dev
mailing list