mirror of
https://github.com/Frooodle/Stirling-PDF.git
synced 2025-11-16 01:21:16 +01:00
refactor(common, core, proprietary): migrate boxed Booleans to primitive booleans and adopt is* accessors to reduce null checks/NPE risk (#4153)
# Description of Changes **What was changed** - Switched multiple nullable `Boolean` fields to primitive `boolean` in `ApplicationProperties`: - `Security.enableLogin`, `Security.csrfDisabled` - `System.googlevisibility`, `System.showUpdateOnlyAdmin`, `System.enableAlphaFunctionality`, `System.disableSanitize`, `System.enableUrlToPDF` - `Metrics.enabled` - Updated all consumers to use Lombok’s `is*` accessors instead of `get*`: - `AppConfig`, `PostHogService`, `CustomHtmlSanitizer`, `EndpointConfiguration`, `InitialSetup`, `OpenApiConfig`, `ConvertWebsiteToPDF`, `HomeWebController`, `MetricsController`, proprietary `SecurityConfiguration`, `AccountWebController` - Tests adjusted to mock `isDisableSanitize()` instead of `getDisableSanitize()` - Logic simplifications: - Removed redundant null-handling/ternaries now that primitives have defaults (e.g., `enableAlphaFunctionality` bean) - Replaced `Boolean.TRUE.equals(...)` with direct primitive checks - Used constant-first `equals` for NPE safety in string comparisons **Why the change was made** - Primitive booleans eliminate ambiguity, cut down on `NullPointerException` risks, and simplify conditions - Aligns with Java/Lombok conventions (`isX()` for `boolean`) for clearer, more consistent APIs - Spring provides sane defaults for missing booleans (`false`), reducing boilerplate and cognitive load --- ## Checklist ### General - [x] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [x] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [x] I have performed a self-review of my own code - [x] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md#6-testing) for more details.
This commit is contained in:
Ludy
GitHub
parent
57eb6dbed9
commit
e932ca01f3
@ -70,7 +70,7 @@ public class AppConfig {
|
||||
|
||||
@Bean(name = "loginEnabled")
|
||||
public boolean loginEnabled() {
|
||||
return applicationProperties.getSecurity().getEnableLogin();
|
||||
return applicationProperties.getSecurity().isEnableLogin();
|
||||
}
|
||||
|
||||
@Bean(name = "appName")
|
||||
@ -120,9 +120,7 @@ public class AppConfig {
|
||||
|
||||
@Bean(name = "enableAlphaFunctionality")
|
||||
public boolean enableAlphaFunctionality() {
|
||||
return applicationProperties.getSystem().getEnableAlphaFunctionality() != null
|
||||
? applicationProperties.getSystem().getEnableAlphaFunctionality()
|
||||
: false;
|
||||
return applicationProperties.getSystem().isEnableAlphaFunctionality();
|
||||
}
|
||||
|
||||
@Bean(name = "rateLimit")
|
||||
|
||||
@ -112,8 +112,8 @@ public class ApplicationProperties {
|
||||
|
||||
@Data
|
||||
public static class Security {
|
||||
private Boolean enableLogin;
|
||||
private Boolean csrfDisabled;
|
||||
private boolean enableLogin;
|
||||
private boolean csrfDisabled;
|
||||
private InitialLogin initialLogin = new InitialLogin();
|
||||
private OAUTH2 oauth2 = new OAUTH2();
|
||||
private SAML2 saml2 = new SAML2();
|
||||
@ -295,8 +295,8 @@ public class ApplicationProperties {
|
||||
throw new UnsupportedProviderException(
|
||||
"Logout from the provider "
|
||||
+ registrationId
|
||||
+ " is not supported. "
|
||||
+ "Report it at https://github.com/Stirling-Tools/Stirling-PDF/issues");
|
||||
+ " is not supported. Report it at"
|
||||
+ " https://github.com/Stirling-Tools/Stirling-PDF/issues");
|
||||
};
|
||||
}
|
||||
}
|
||||
@ -314,19 +314,19 @@ public class ApplicationProperties {
|
||||
@Data
|
||||
public static class System {
|
||||
private String defaultLocale;
|
||||
private Boolean googlevisibility;
|
||||
private boolean googlevisibility;
|
||||
private boolean showUpdate;
|
||||
private Boolean showUpdateOnlyAdmin;
|
||||
private boolean showUpdateOnlyAdmin;
|
||||
private boolean customHTMLFiles;
|
||||
private String tessdataDir;
|
||||
private Boolean enableAlphaFunctionality;
|
||||
private boolean enableAlphaFunctionality;
|
||||
private Boolean enableAnalytics;
|
||||
private Boolean enablePosthog;
|
||||
private Boolean enableScarf;
|
||||
private Datasource datasource;
|
||||
private Boolean disableSanitize;
|
||||
private boolean disableSanitize;
|
||||
private int maxDPI;
|
||||
private Boolean enableUrlToPDF;
|
||||
private boolean enableUrlToPDF;
|
||||
private Html html = new Html();
|
||||
private CustomPaths customPaths = new CustomPaths();
|
||||
private String fileUploadLimit;
|
||||
@ -453,10 +453,10 @@ public class ApplicationProperties {
|
||||
@Override
|
||||
public String toString() {
|
||||
return """
|
||||
Driver {
|
||||
driverName='%s'
|
||||
}
|
||||
"""
|
||||
Driver {
|
||||
driverName='%s'
|
||||
}
|
||||
"""
|
||||
.formatted(driverName);
|
||||
}
|
||||
}
|
||||
@ -491,7 +491,7 @@ public class ApplicationProperties {
|
||||
|
||||
@Data
|
||||
public static class Metrics {
|
||||
private Boolean enabled;
|
||||
private boolean enabled;
|
||||
}
|
||||
|
||||
@Data
|
||||
|
||||
@ -253,11 +253,11 @@ public class PostHogService {
|
||||
addIfNotEmpty(
|
||||
properties,
|
||||
"security_enableLogin",
|
||||
applicationProperties.getSecurity().getEnableLogin());
|
||||
applicationProperties.getSecurity().isEnableLogin());
|
||||
addIfNotEmpty(
|
||||
properties,
|
||||
"security_csrfDisabled",
|
||||
applicationProperties.getSecurity().getCsrfDisabled());
|
||||
applicationProperties.getSecurity().isCsrfDisabled());
|
||||
addIfNotEmpty(
|
||||
properties,
|
||||
"security_loginAttemptCount",
|
||||
@ -302,13 +302,13 @@ public class PostHogService {
|
||||
addIfNotEmpty(
|
||||
properties,
|
||||
"system_googlevisibility",
|
||||
applicationProperties.getSystem().getGooglevisibility());
|
||||
applicationProperties.getSystem().isGooglevisibility());
|
||||
addIfNotEmpty(
|
||||
properties, "system_showUpdate", applicationProperties.getSystem().isShowUpdate());
|
||||
addIfNotEmpty(
|
||||
properties,
|
||||
"system_showUpdateOnlyAdmin",
|
||||
applicationProperties.getSystem().getShowUpdateOnlyAdmin());
|
||||
applicationProperties.getSystem().isShowUpdateOnlyAdmin());
|
||||
addIfNotEmpty(
|
||||
properties,
|
||||
"system_customHTMLFiles",
|
||||
@ -320,7 +320,7 @@ public class PostHogService {
|
||||
addIfNotEmpty(
|
||||
properties,
|
||||
"system_enableAlphaFunctionality",
|
||||
applicationProperties.getSystem().getEnableAlphaFunctionality());
|
||||
applicationProperties.getSystem().isEnableAlphaFunctionality());
|
||||
addIfNotEmpty(
|
||||
properties,
|
||||
"system_enableAnalytics",
|
||||
@ -337,7 +337,7 @@ public class PostHogService {
|
||||
|
||||
// Capture Metrics properties
|
||||
addIfNotEmpty(
|
||||
properties, "metrics_enabled", applicationProperties.getMetrics().getEnabled());
|
||||
properties, "metrics_enabled", applicationProperties.getMetrics().isEnabled());
|
||||
|
||||
// Capture EnterpriseEdition properties
|
||||
addIfNotEmpty(
|
||||
|
||||
@ -62,8 +62,7 @@ public class CustomHtmlSanitizer {
|
||||
.and(new HtmlPolicyBuilder().disallowElements("noscript").toFactory());
|
||||
|
||||
public String sanitize(String html) {
|
||||
boolean disableSanitize =
|
||||
Boolean.TRUE.equals(applicationProperties.getSystem().getDisableSanitize());
|
||||
boolean disableSanitize = applicationProperties.getSystem().isDisableSanitize();
|
||||
return disableSanitize ? html : POLICY.sanitize(html);
|
||||
}
|
||||
}
|
||||
|
||||
@ -36,7 +36,7 @@ class CustomHtmlSanitizerTest {
|
||||
// strict-stubbing failures when individual tests bypass certain branches.
|
||||
lenient().when(ssrfProtectionService.isUrlAllowed(anyString())).thenReturn(true);
|
||||
lenient().when(applicationProperties.getSystem()).thenReturn(systemProperties);
|
||||
lenient().when(systemProperties.getDisableSanitize()).thenReturn(false);
|
||||
lenient().when(systemProperties.isDisableSanitize()).thenReturn(false);
|
||||
|
||||
customHtmlSanitizer = new CustomHtmlSanitizer(ssrfProtectionService, applicationProperties);
|
||||
}
|
||||
@ -374,7 +374,7 @@ class CustomHtmlSanitizerTest {
|
||||
"<p>ok</p><script>alert('XSS')</script><img src=\"http://blocked.local/a.png\">";
|
||||
|
||||
// For this test, disable sanitize
|
||||
when(systemProperties.getDisableSanitize()).thenReturn(true);
|
||||
when(systemProperties.isDisableSanitize()).thenReturn(true);
|
||||
|
||||
// Also ensure SSRF would block it if sanitization were enabled (to prove bypass)
|
||||
lenient().when(ssrfProtectionService.isUrlAllowed(anyString())).thenReturn(false);
|
||||
|
||||
@ -48,7 +48,7 @@ class EmlToPdfTest {
|
||||
when(mockSsrfProtectionService.isUrlAllowed(org.mockito.ArgumentMatchers.anyString()))
|
||||
.thenReturn(true);
|
||||
when(mockApplicationProperties.getSystem()).thenReturn(mockSystem);
|
||||
when(mockSystem.getDisableSanitize()).thenReturn(false);
|
||||
when(mockSystem.isDisableSanitize()).thenReturn(false);
|
||||
|
||||
customHtmlSanitizer =
|
||||
new CustomHtmlSanitizer(mockSsrfProtectionService, mockApplicationProperties);
|
||||
|
||||
@ -29,7 +29,7 @@ public class FileToPdfTest {
|
||||
when(mockSsrfProtectionService.isUrlAllowed(org.mockito.ArgumentMatchers.anyString()))
|
||||
.thenReturn(true);
|
||||
when(mockApplicationProperties.getSystem()).thenReturn(mockSystem);
|
||||
when(mockSystem.getDisableSanitize()).thenReturn(false);
|
||||
when(mockSystem.isDisableSanitize()).thenReturn(false);
|
||||
|
||||
customHtmlSanitizer =
|
||||
new CustomHtmlSanitizer(mockSsrfProtectionService, mockApplicationProperties);
|
||||
|
||||
@ -475,7 +475,7 @@ public class EndpointConfiguration {
|
||||
disableGroup("enterprise");
|
||||
}
|
||||
|
||||
if (!applicationProperties.getSystem().getEnableUrlToPDF()) {
|
||||
if (!applicationProperties.getSystem().isEnableUrlToPDF()) {
|
||||
disableEndpoint("url-to-pdf");
|
||||
}
|
||||
}
|
||||
|
||||
@ -61,11 +61,9 @@ public class InitialSetup {
|
||||
public void initEnableCSRFSecurity() throws IOException {
|
||||
if (GeneralUtils.isVersionHigher(
|
||||
"0.46.0", applicationProperties.getAutomaticallyGenerated().getAppVersion())) {
|
||||
Boolean csrf = applicationProperties.getSecurity().getCsrfDisabled();
|
||||
boolean csrf = applicationProperties.getSecurity().isCsrfDisabled();
|
||||
if (!csrf) {
|
||||
GeneralUtils.saveKeyToSettings("security.csrfDisabled", false);
|
||||
GeneralUtils.saveKeyToSettings("system.enableAnalytics", true);
|
||||
applicationProperties.getSecurity().setCsrfDisabled(false);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -50,7 +50,7 @@ public class OpenApiConfig {
|
||||
.url("https://www.stirlingpdf.com")
|
||||
.email("contact@stirlingpdf.com"))
|
||||
.description(DEFAULT_DESCRIPTION);
|
||||
if (!applicationProperties.getSecurity().getEnableLogin()) {
|
||||
if (!applicationProperties.getSecurity().isEnableLogin()) {
|
||||
return new OpenAPI().components(new Components()).info(info);
|
||||
} else {
|
||||
SecurityScheme apiKeyScheme =
|
||||
|
||||
@ -71,7 +71,7 @@ public class ConvertWebsiteToPDF {
|
||||
URI location = null;
|
||||
HttpStatus status = HttpStatus.SEE_OTHER;
|
||||
|
||||
if (!applicationProperties.getSystem().getEnableUrlToPDF()) {
|
||||
if (!applicationProperties.getSystem().isEnableUrlToPDF()) {
|
||||
location =
|
||||
uriComponentsBuilder
|
||||
.queryParam("error", "error.endpointDisabled")
|
||||
|
||||
@ -84,8 +84,8 @@ public class HomeWebController {
|
||||
@ResponseBody
|
||||
@Hidden
|
||||
public String getRobotsTxt() {
|
||||
Boolean allowGoogle = applicationProperties.getSystem().getGooglevisibility();
|
||||
if (Boolean.TRUE.equals(allowGoogle)) {
|
||||
boolean allowGoogle = applicationProperties.getSystem().isGooglevisibility();
|
||||
if (allowGoogle) {
|
||||
return "User-agent: Googlebot\nAllow: /\n\nUser-agent: *\nAllow: /";
|
||||
} else {
|
||||
return "User-agent: Googlebot\nDisallow: /\n\nUser-agent: *\nDisallow: /";
|
||||
|
||||
@ -42,9 +42,7 @@ public class MetricsController {
|
||||
|
||||
@PostConstruct
|
||||
public void init() {
|
||||
Boolean metricsEnabled = applicationProperties.getMetrics().getEnabled();
|
||||
if (metricsEnabled == null) metricsEnabled = true;
|
||||
this.metricsEnabled = metricsEnabled;
|
||||
metricsEnabled = applicationProperties.getMetrics().isEnabled();
|
||||
}
|
||||
|
||||
@GetMapping("/status")
|
||||
|
||||
@ -119,7 +119,7 @@ class HomeWebControllerTest {
|
||||
@Test
|
||||
@DisplayName("googlevisibility=true -> allow all agents")
|
||||
void robots_allow() throws Exception {
|
||||
when(applicationProperties.getSystem().getGooglevisibility()).thenReturn(Boolean.TRUE);
|
||||
when(applicationProperties.getSystem().isGooglevisibility()).thenReturn(true);
|
||||
|
||||
mockMvc.perform(get("/robots.txt"))
|
||||
.andExpect(status().isOk())
|
||||
@ -136,7 +136,7 @@ class HomeWebControllerTest {
|
||||
@Test
|
||||
@DisplayName("googlevisibility=false -> disallow all agents")
|
||||
void robots_disallow() throws Exception {
|
||||
when(applicationProperties.getSystem().getGooglevisibility()).thenReturn(Boolean.FALSE);
|
||||
when(applicationProperties.getSystem().isGooglevisibility()).thenReturn(false);
|
||||
|
||||
mockMvc.perform(get("/robots.txt"))
|
||||
.andExpect(status().isOk())
|
||||
@ -151,9 +151,9 @@ class HomeWebControllerTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("googlevisibility=null -> disallow all (default branch)")
|
||||
void robots_disallowWhenNull() throws Exception {
|
||||
when(applicationProperties.getSystem().getGooglevisibility()).thenReturn(null);
|
||||
@DisplayName("googlevisibility not set (default false) -> disallow all")
|
||||
void robots_disallowWhenNotSet() throws Exception {
|
||||
when(applicationProperties.getSystem().isGooglevisibility()).thenReturn(false);
|
||||
|
||||
mockMvc.perform(get("/robots.txt"))
|
||||
.andExpect(status().isOk())
|
||||
|
||||
@ -126,7 +126,7 @@ public class AccountWebController {
|
||||
SAML2 saml2 = securityProps.getSaml2();
|
||||
|
||||
if (securityProps.isSaml2Active()
|
||||
&& applicationProperties.getSystem().getEnableAlphaFunctionality()
|
||||
&& applicationProperties.getSystem().isEnableAlphaFunctionality()
|
||||
&& applicationProperties.getPremium().isEnabled()) {
|
||||
String samlIdp = saml2.getProvider();
|
||||
String saml2AuthenticationPath = "/saml2/authenticate/" + saml2.getRegistrationId();
|
||||
|
||||
@ -125,7 +125,7 @@ public class SecurityConfiguration {
|
||||
|
||||
@Bean
|
||||
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
|
||||
if (securityProperties.getCsrfDisabled() || !loginEnabledValue) {
|
||||
if (securityProperties.isCsrfDisabled() || !loginEnabledValue) {
|
||||
http.csrf(CsrfConfigurer::disable);
|
||||
}
|
||||
|
||||
@ -146,7 +146,7 @@ public class SecurityConfiguration {
|
||||
.addFilterAfter(rateLimitingFilter(), UserAuthenticationFilter.class)
|
||||
.addFilterAfter(firstLoginFilter, UsernamePasswordAuthenticationFilter.class);
|
||||
|
||||
if (!securityProperties.getCsrfDisabled()) {
|
||||
if (!securityProperties.isCsrfDisabled()) {
|
||||
CookieCsrfTokenRepository cookieRepo =
|
||||
CookieCsrfTokenRepository.withHttpOnlyFalse();
|
||||
CsrfTokenRequestAttributeHandler requestHandler =
|
||||
|
||||
@ -27,7 +27,7 @@ class AppUpdateAuthService implements ShowAdminInterface {
|
||||
if (!showUpdate) {
|
||||
return showUpdate;
|
||||
}
|
||||
boolean showUpdateOnlyAdmin = applicationProperties.getSystem().getShowUpdateOnlyAdmin();
|
||||
boolean showUpdateOnlyAdmin = applicationProperties.getSystem().isShowUpdateOnlyAdmin();
|
||||
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
|
||||
if (authentication == null || !authentication.isAuthenticated()) {
|
||||
return !showUpdateOnlyAdmin;
|
||||
|
||||
Loading…
Reference in New Issue
Block a user