JGSS Enhancements (contribution by Two Sigma Open Source)

[Weijun Wang]

> On Oct 5, 2018, at 1:15 AM, Nico Williams <Nico.Williams at twosigma.com> wrote:
> 
> On Thu, Oct 04, 2018 at 11:19:06AM +0100, Alan Bateman wrote:
>> On 03/10/2018 21:49, Nico Williams wrote:
>>> :
>>> A lot of these changes are interrelated.  Reviewing them in order of
>>> size might require rebasing our stack of commits, and may not be
>>> entirely possible.
>>> 
>>> We're extremely familiar with this code as we have been patching the
>>> JGSS stack this way for years (we have developed these patches for JDKs
>>> 7, 8, 9, 10, 11, and the current 12 master), and we have been running
>>> this in production (with JDKs 7, 8, and 9, and soon 11)
>> Just a few high-level points on the patches that you attached:
>> 
>> 1. It's important to take sponsor/Reviewer effort into account. I
>> skimmed through some of the 25 patches and they lack a detailed
>> description on what the issue is about. JGSS gurus might recognize
>> some of these issues from the diffs but I suspect you (or Victor)
>> will need to match the patches to existing issues in JIRA or else
>> get bugs submitted so that there is a description for each issue in
>> the bug database.
> 
> We've pointed out that we need bugs filed.  We'll work with Weijun to
> get them filed when he's back from vacation.

I'm just back. Will start working tomorrow.

> 
>> 2. I skimmed the patches and didn't see any tests or changes to
>> existing tests. This may come up in the discussion of each change as
>> the default position is for all bug fixes should have tests where
>> feasible.
> 
> That's an existing problem in the codebase.  The tests are very limited
> as it is, and testing GSS/Kerberos requires mocking up a KDC, which
> requires more complex test infrastructure.

I do have a simple one at

  http://hg.openjdk.java.net/jdk/jdk/file/tip/test/jdk/sun/security/krb5/auto/KDC.java

you can read OneKDC.java, Context.java and Basic.java in the same directory.

--Max

> 
>> 3. I see the patches include at least some API changes so the
>> sponsor will need to submit a CSR for approval. API changes are only
>> allowed in feature releases.
> 
> That's fine.
> 
> Nico
> --