RFR: 8345139: Fix bugs and inconsistencies in the Provider services map

Francisco Ferrari Bihurriet fferrari at openjdk.org
Fri Dec 6 19:49:16 UTC 2024 

Hi, this pull request implements the fixes for bugs and inconsistencies described in [JDK-8345139](https://bugs.openjdk.org/browse/JDK-8345139 "Fix bugs and inconsistencies in the Provider services map").

#### New services map design

Here is the high-level hierarchy of the new services map design:

* `servicesMap` (`ServicesMap`)
    * Instances
        * `impl` (`ServicesMapImpl`)
            * `services` (`Map<ServiceKey, Service>`): unifies the previous `serviceMap` and `legacyMap`
            * `legacySvcKeys` (`Set<ServiceKey>`): set indicating which keys in `services` correspond to the Legacy API
            * `serviceProps` (`Map<ServiceKey, String>`): keeps track of the _provider Hashtable entries_ that originated services entries <sup>(1)</sup>
            * `serviceAttrProps` (`Map<ServiceKey, Map<UString, String>>`): keeps track of the _provider Hashtable entries_ that originated service attributes <sup>(1)</sup>
        * `serviceSet` (`AtomicReference<Set<Service>>`): part of a lock-free mechanism to implement a consistent version of the `getServices()` readers method
    * Writers' methods (for providers registration)
        * `Current asCurrent()`: returns `impl` seen as a `Current` interface implementer
            * `putService(Service svc)`
            * `removeService(Service svc)`
        * `Legacy asLegacy()`: returns `impl` seen as a `Legacy` interface implementer
            * `putClassName(ServiceKey key, String className, String propKey)`
            * `putAlias(ServiceKey key, ServiceKey aliasKey, String propKey)`
            * `putAttribute(ServiceKey key, String attrName, String attrValue, String propKey)`
            * `remove(ServiceKey key, String className)`
            * `removeAlias(ServiceKey key, ServiceKey aliasKey)`
            * `removeAttribute(ServiceKey key, String attrName, String attrValue)`
    * Readers' methods (for services users and `GetInstance` APIs)
        * `Set<Service> getServices()`
        * `Service getService(ServiceKey key)`
    * Other methods: default and copy constructors, `void clear()`

(1): to maintain the consistency between the provider's `servicesMap` and its _Hashtable entries_, even if Legacy API updates occur through _properties_ with different casing, or aliases instead of main algorithms.

#### Testing

As part of our testing, we observed all the tests pass in the following categories:

* `jdk:tier1` (see [GitHub Actions run](https://github.com/franferrax/jdk/actions/runs/12193211398))
* `jdk/com/sun/crypto`
* `jdk/java/security`
    * Including the new `jdk/java/security/Provider/ServicesConsistency.java`

Additionally, we found no regressions with respect to this pull request baseline (bf0debc023a42ccdf2f589039e4d98e11424b4dd) in the following category:

* `jdk/sun/security`
    * Same results in both codebases

      | Results                 |     |
      |-------------------------|----:|
      | Tests that passed       | 797 |
      | Tests that failed       |  15 |
      | Tests that had errors   |   2 |
      | Tests that were not run |  35 |
      | Total                   | 849 |

##### New `ServicesConsistency.java` test

The new `ServicesConsistency` test checks several of the cases described in the JBS issue, plus other similar variants. Here is a mapping between some of the test cases and their corresponding JBS issue section (using [text fragments](https://developer.mozilla.org/en-US/docs/Web/URI/Fragment/Text_fragments) links), known to describe the tested problem.

| Test case(s) | JBS section |
|--------------|:-----------:|
| [`ServicesConsistency::testInvalidGetServicesRemoval`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L1040-L1042) | [JDK-8345139, section 1.1](https://bugs.openjdk.org/browse/JDK-8345139#:~:text=1.1,.,-1.2) |
| [`ServicesConsistency::testInvalidGetServiceRemoval`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L1036-L1038) | [JDK-8345139, section 1.2](https://bugs.openjdk.org/browse/JDK-8345139#:~:text=1.2,.,-1.3) |
| [`ServicesConsistency::testPutAllIsAtomic`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L957-L971)<br>[`ServicesConsistency::testReplaceAllIsAtomic`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L930-L955) | [JDK-8345139, section 1.4](https://bugs.openjdk.org/browse/JDK-8345139#:~:text=1.4,.,-1.5) |
| [`ServicesConsistency::testInvalidCachedClass`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L892-L912)<br>[`ServicesConsistency::testInvalidCachedHasKeyAttributes`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L841-L843)<br>[`ServicesConsistency::testInvalidCachedSupportedKeyFormats`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L845-L848) | [JDK-8345139, section 1.5](https://bugs.openjdk.org/browse/JDK-8345139#:~:text=1.5,.,-2) |
| [`ServicesConsistency::testSerializationClassnameConsistency`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L1062-L1082) | [JDK-8345139, section 2.1](https://bugs.openjdk.org/browse/JDK-8345139#:~:text=2.1,.,-2.2) |
| [`ServicesConsistency::testCurrentAPIServicesOverride`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L624-L626) | [JDK-8345139, section 2.3](https://bugs.openjdk.org/browse/JDK-8345139#:~:text=2.3,.,-2.4) |
| [`ServicesConsistency::testLegacyAPIServicesOverride`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L450-L452) | [JDK-8345139, section 2.4](https://bugs.openjdk.org/browse/JDK-8345139#:~:text=2.4,.,-2.5) |
| [`ServicesConsistency::testLegacyAPIAliasCannotBeAlgorithm`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L611-L622) | [JDK-8345139, section 2.5](https://bugs.openjdk.org/browse/JDK-8345139#:~:text=2.5,.,-3) |
| [`ServicesConsistency::testInvalidServiceNotReturned`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L835-L839) | [JDK-8345139, section 3.1](https://bugs.openjdk.org/browse/JDK-8345139#:~:text=3.1,.,-3.2) |
| [`ServicesConsistency::testComputeDoesNotThrowNPE`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L436-L441)<br>[`ServicesConsistency::testMergeDoesNotThrowNPE`](https://github.com/openjdk/jdk/blob/95364f86f4cdfe787ba203d3dc346a10822054a5/test/jdk/java/security/Provider/ServicesConsistency.java#L443-L448) | [JDK-8345139, section 3.2](https://bugs.openjdk.org/browse/JDK-8345139#:~:text=3.2,.,-Note) |

This contribution is co-authored by @franferrax and @martinuy.

-------------

Commit messages:
 - 8345139: Fix bugs and inconsistencies in the Provider services map

Changes: https://git.openjdk.org/jdk/pull/22613/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=22613&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8345139
  Stats: 2473 lines in 2 files changed: 2085 ins; 226 del; 162 mod
  Patch: https://git.openjdk.org/jdk/pull/22613.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/22613/head:pull/22613

PR: https://git.openjdk.org/jdk/pull/22613

More information about the security-dev mailing list