On Mon, 26 Jan 2026 16:41:44 GMT, Matthew Donovan <[email protected]> wrote:

>> Mikhail Yankelevich has refreshed the contents of this pull request, and 
>> previous commits have been removed. The incremental views will show 
>> differences compared to the previous content of the PR. The pull request 
>> contains one new commit since the last revision:
>> 
>>   JDK-8376218: Improve KeyUtil::getKeySize coverage
>
> test/jdk/sun/security/util/KeyUtilTests.java line 84:
> 
>> 82:         params.init(ecPrivateKey.getParams());
>> 83: 
>> 84:         int keySizeResult = KeyUtil.getKeySize(params);
> 
> If the `CustomSunEC` provider uses all of the same Service classes as SunEC, 
> what exactly is this test exercising? Seems like it's duplicating 
> `testGetKeySizeSunEc`

It tests 
[this](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/security/util/KeyUtil.java#L137-L165)
 block specifically. We already test SunEC, but EC which is not SunEC is not 
covered at all hence this test. 

  public static final int getKeySize(AlgorithmParameters parameters) {

        switch (parameters.getAlgorithm()) {
            case "EC":
                // ECKeySizeParameterSpec is SunEC internal only
                if (parameters.getProvider().getName().equals("SunEC")) {
                    try {
                        ECKeySizeParameterSpec ps = parameters.getParameterSpec(
                            ECKeySizeParameterSpec.class);
                        if (ps != null) {
                            return ps.getKeySize();
                        }
                    } catch (InvalidParameterSpecException ipse) {
                        // ignore
                    }
                }

                try {
                    ECParameterSpec ps = parameters.getParameterSpec(
                            ECParameterSpec.class);
                    if (ps != null) {
                        return ps.getOrder().bitLength();
                    }
                } catch (InvalidParameterSpecException ipse) {
                    // ignore
                }

                // Note: the ECGenParameterSpec case should be covered by the
                // ECParameterSpec case above.
                // See ECUtil.getECParameterSpec(String).

                break;

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29389#discussion_r2731417895

Reply via email to