Skip to content

[fix][tls] remove throw exception in getProvider#15414

Closed
nodece wants to merge 1 commit intoapache:masterfrom
nodece:remove_throw_ex_get_provider
Closed

[fix][tls] remove throw exception in getProvider#15414
nodece wants to merge 1 commit intoapache:masterfrom
nodece:remove_throw_ex_get_provider

Conversation

@nodece
Copy link
Member

@nodece nodece commented May 2, 2022

Signed-off-by: Zixuan Liu nodeces@gmail.com

Motivation

When cannot find the BouncyCastleFipsProvider or BouncyCastleProvider, we shouldn't throw any exception, it is unnecessary.

Modifications

  • Remove throw exception in getProvider

Documentation

Need to update docs?

  • no-need-doc

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 2, 2022
@poorbarcode
Copy link
Contributor

poorbarcode commented May 3, 2022

I think: this patch is to solve the boot failure.

If return null, this public constant also will be null, I see this static constant is not used, can we deleted it ?

public static final Provider BC_PROVIDER = getProvider();

public static boolean isBCFIPS() {
return BC_PROVIDER.getClass().getCanonicalName().equals(BC_FIPS_PROVIDER_CLASS);
}

If we deleted the static constant, so we don't have to change this part:

try {
return getBCProviderFromClassPath();
} catch (Exception e) {
log.warn("Not able to get Bouncy Castle provider for both FIPS and Non-FIPS from class path:", e);
throw new RuntimeException(e);
}

Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nodece
Copy link
Member Author

nodece commented May 3, 2022

@poorbarcode We cannot delete these, it is used to load the Bouncy Castle provider.

@poorbarcode
Copy link
Contributor

@poorbarcode We cannot delete these, it is used to load the Bouncy Castle provider.

Received, thank you.

Copy link
Member

@mattisonchao mattisonchao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, left a trivial comment.

log.warn("Not able to get Bouncy Castle provider for both FIPS and Non-FIPS from class path:", e);
throw new RuntimeException(e);
}
return null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional will be better than null.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use this TLS provider, the purpose of this is to add a BC provider to the JDK.

@Technoboy- Technoboy- closed this May 27, 2022
@Technoboy- Technoboy- reopened this May 27, 2022
@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label Jun 27, 2022
@nodece nodece closed this Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants