From fae09a2a05dcd787f7438d5de293630f342e8d4f Mon Sep 17 00:00:00 2001 From: Sven Strittmatter Date: Fri, 12 Jul 2024 16:53:55 +0200 Subject: [PATCH 1/3] #127 Improve Error Handling for Failed DefectDojo API Requests Two Problems: 1. We do not pass consequently the causing exception to our own custom exception. 2. We do not catch all possible REST client runtime exceptions. Fixed by consequent logging and passing ofthe origin exception as cause of our own excpetion. Also catching a more generic exception type totach all possible errors. Signed-off-by: Sven Strittmatter --- .../service/DefaultImportScanService.java | 4 ++-- .../service/GenericDefectDojoService.java | 23 ++++++++++++------- .../service/ImportScanService2.java | 6 ++--- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/main/java/io/securecodebox/persistence/defectdojo/service/DefaultImportScanService.java b/src/main/java/io/securecodebox/persistence/defectdojo/service/DefaultImportScanService.java index 00bb63bd..b17b012b 100644 --- a/src/main/java/io/securecodebox/persistence/defectdojo/service/DefaultImportScanService.java +++ b/src/main/java/io/securecodebox/persistence/defectdojo/service/DefaultImportScanService.java @@ -31,7 +31,7 @@ import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestTemplate; import java.nio.charset.StandardCharsets; @@ -124,7 +124,7 @@ public String getFilename() { final var payload = new HttpEntity>(body, headers); return exchangeRequest(endpoint, payload); - } catch (HttpClientErrorException e) { + } catch (RestClientException e) { log.error("Exception while attaching findings to engagement: {}", e.getMessage()); throw new PersistenceException("Failed to attach findings to engagement.", e); } diff --git a/src/main/java/io/securecodebox/persistence/defectdojo/service/GenericDefectDojoService.java b/src/main/java/io/securecodebox/persistence/defectdojo/service/GenericDefectDojoService.java index 2119c618..7b5cf7f6 100644 --- a/src/main/java/io/securecodebox/persistence/defectdojo/service/GenericDefectDojoService.java +++ b/src/main/java/io/securecodebox/persistence/defectdojo/service/GenericDefectDojoService.java @@ -12,6 +12,7 @@ import com.fasterxml.jackson.databind.cfg.CoercionInputShape; import io.securecodebox.persistence.defectdojo.config.Config; import io.securecodebox.persistence.defectdojo.exception.LoopException; +import io.securecodebox.persistence.defectdojo.exception.PersistenceException; import io.securecodebox.persistence.defectdojo.http.Foo; import io.securecodebox.persistence.defectdojo.http.ProxyConfigFactory; import io.securecodebox.persistence.defectdojo.model.Engagement; @@ -29,6 +30,7 @@ import org.springframework.http.converter.StringHttpMessageConverter; import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; import org.springframework.util.LinkedMultiValueMap; +import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestTemplate; import org.springframework.web.util.UriComponentsBuilder; @@ -73,14 +75,19 @@ public final T get(long id) { final var url = this.config.getUrl() + API_PREFIX + this.getUrlPath() + "/" + id; log.debug("Requesting URL: {}", url); - ResponseEntity response = restTemplate.exchange( - url, - HttpMethod.GET, - payload, - getModelClass() - ); - - return response.getBody(); + try { + ResponseEntity response = restTemplate.exchange( + url, + HttpMethod.GET, + payload, + getModelClass() + ); + + return response.getBody(); + } catch (RestClientException e) { + log.error("Exception while getting data: {}", e.getMessage()); + throw new PersistenceException("Failed to get data.", e); + } } @Override diff --git a/src/main/java/io/securecodebox/persistence/defectdojo/service/ImportScanService2.java b/src/main/java/io/securecodebox/persistence/defectdojo/service/ImportScanService2.java index 28d809b3..0df013f6 100644 --- a/src/main/java/io/securecodebox/persistence/defectdojo/service/ImportScanService2.java +++ b/src/main/java/io/securecodebox/persistence/defectdojo/service/ImportScanService2.java @@ -22,7 +22,7 @@ import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestTemplate; import java.nio.charset.StandardCharsets; @@ -94,8 +94,8 @@ public String getFilename() { var payload = new HttpEntity<>(mvn, headers); return restTemplate.exchange(config.getUrl() + "/api/v2/" + endpoint + "/", HttpMethod.POST, payload, ImportScanResponse.class).getBody(); - } catch (HttpClientErrorException e) { - throw new PersistenceException("Failed to attach findings to engagement."); + } catch (RestClientException e) { + throw new PersistenceException("Failed to attach findings to engagement.", e); } } From ba2db23f4121e4626a13bee04baf4064708d1dea Mon Sep 17 00:00:00 2001 From: Sven Strittmatter Date: Fri, 26 Jul 2024 11:49:56 +0200 Subject: [PATCH 2/3] #127 Use Parameterized Log to Prevent Performance Penalties The conatenation will be done always, also if debug level is not logged. So we should alwyas use parameterized log statements. Signed-off-by: Sven Strittmatter --- .../defectdojo/service/GenericDefectDojoService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/securecodebox/persistence/defectdojo/service/GenericDefectDojoService.java b/src/main/java/io/securecodebox/persistence/defectdojo/service/GenericDefectDojoService.java index 7b5cf7f6..0da77c6c 100644 --- a/src/main/java/io/securecodebox/persistence/defectdojo/service/GenericDefectDojoService.java +++ b/src/main/java/io/securecodebox/persistence/defectdojo/service/GenericDefectDojoService.java @@ -219,7 +219,7 @@ protected PaginatedResult internalSearch(Map queryParams, lon } var url = new URI(this.config.getUrl() + API_PREFIX + this.getUrlPath() + "/"); - log.debug("Requesting URL: " + url); + log.debug("Requesting URL: {}", url); var uriBuilder = UriComponentsBuilder.fromUri(url).queryParams(multiValueMap); ResponseEntity responseString = restTemplate.exchange( From 0ff32f38b8d1971ec278d6cfc16c4ce4ba4c7a2f Mon Sep 17 00:00:00 2001 From: Sven Strittmatter Date: Fri, 26 Jul 2024 15:12:44 +0200 Subject: [PATCH 3/3] #127 Also Handle HTTP Client Exception for All Other HTTP Methods In previous commits only GET requests were covered with proper exception handling. This adds rethrow of our own custom exception with the originating error as cause, like already implemented for GET requests. Signed-off-by: Sven Strittmatter --- .../service/GenericDefectDojoService.java | 62 ++++++++++++++----- 1 file changed, 47 insertions(+), 15 deletions(-) diff --git a/src/main/java/io/securecodebox/persistence/defectdojo/service/GenericDefectDojoService.java b/src/main/java/io/securecodebox/persistence/defectdojo/service/GenericDefectDojoService.java index 0da77c6c..05316690 100644 --- a/src/main/java/io/securecodebox/persistence/defectdojo/service/GenericDefectDojoService.java +++ b/src/main/java/io/securecodebox/persistence/defectdojo/service/GenericDefectDojoService.java @@ -75,6 +75,7 @@ public final T get(long id) { final var url = this.config.getUrl() + API_PREFIX + this.getUrlPath() + "/" + id; log.debug("Requesting URL: {}", url); + try { ResponseEntity response = restTemplate.exchange( url, @@ -85,8 +86,8 @@ public final T get(long id) { return response.getBody(); } catch (RestClientException e) { - log.error("Exception while getting data: {}", e.getMessage()); - throw new PersistenceException("Failed to get data.", e); + log.error("Exception while doing a GET request to DefectDojo API: {}", e.getMessage()); + throw new PersistenceException("Failed to do a GET to DefectDojo API!", e); } } @@ -140,8 +141,17 @@ public final T create(@NonNull T object) { var restTemplate = this.getRestTemplate(); HttpEntity payload = new HttpEntity<>(object, getDefectDojoAuthorizationHeaders()); - ResponseEntity response = restTemplate.exchange(this.config.getUrl() + API_PREFIX + getUrlPath() + "/", HttpMethod.POST, payload, getModelClass()); - return response.getBody(); + try { + ResponseEntity response = restTemplate.exchange( + this.config.getUrl() + API_PREFIX + getUrlPath() + "/", + HttpMethod.POST, + payload, + getModelClass()); + return response.getBody(); + } catch (RestClientException e) { + log.error("Exception while doing a POST request to DefectDojo API: {}", e.getMessage()); + throw new PersistenceException("Failed to do a POST to DefectDojo API!", e); + } } @Override @@ -149,7 +159,16 @@ public final void delete(long id) { var restTemplate = this.getRestTemplate(); HttpEntity payload = new HttpEntity<>(getDefectDojoAuthorizationHeaders()); - restTemplate.exchange(this.config.getUrl() + API_PREFIX + getUrlPath() + "/" + id + "/", HttpMethod.DELETE, payload, String.class); + try { + restTemplate.exchange( + this.config.getUrl() + API_PREFIX + getUrlPath() + "/" + id + "/", + HttpMethod.DELETE, + payload, + String.class); + } catch (RestClientException e) { + log.error("Exception while doing a DELETE request to DefectDojo API: {}", e.getMessage()); + throw new PersistenceException("Failed to do a DELETE to DefectDojo API!", e); + } } @Override @@ -157,10 +176,18 @@ public final T update(@NonNull T object, long id) { var restTemplate = this.getRestTemplate(); HttpEntity payload = new HttpEntity<>(object, getDefectDojoAuthorizationHeaders()); - ResponseEntity response = restTemplate.exchange(this.config.getUrl() + API_PREFIX + getUrlPath() + "/" + id + "/", HttpMethod.PUT, payload, getModelClass()); - return response.getBody(); + try { + ResponseEntity response = restTemplate.exchange(this.config.getUrl() + API_PREFIX + getUrlPath() + "/" + id + "/", + HttpMethod.PUT, + payload, + getModelClass()); + return response.getBody(); + } catch (RestClientException e) { + log.error("Exception while doing a PUT request to DefectDojo API: {}", e.getMessage()); + throw new PersistenceException("Failed to do a PUT to DefectDojo API!", e); + } } - + /** * Get the URL path for the REST endpoint relative to {@link #API_PREFIX} * @@ -222,13 +249,18 @@ protected PaginatedResult internalSearch(Map queryParams, lon log.debug("Requesting URL: {}", url); var uriBuilder = UriComponentsBuilder.fromUri(url).queryParams(multiValueMap); - ResponseEntity responseString = restTemplate.exchange( - uriBuilder.build(mutableQueryParams), - HttpMethod.GET, - payload, - String.class - ); + try { + ResponseEntity responseString = restTemplate.exchange( + uriBuilder.build(mutableQueryParams), + HttpMethod.GET, + payload, + String.class + ); - return deserializeList(responseString.getBody()); + return deserializeList(responseString.getBody()); + } catch (RestClientException e) { + log.error("Exception while doing a GET request to DefectDojo API: {}", e.getMessage()); + throw new PersistenceException("Failed to do a GET to DefectDojo API!", e); + } } }