From 90d4fb425a3e1db96c0387c3fed81f39ef34de89 Mon Sep 17 00:00:00 2001 From: melbiialy Date: Fri, 13 Mar 2026 21:56:53 +0200 Subject: [PATCH] fix(license): improve exception handling in uploadLicense stream cleanup The uploadLicense() method in Sw360LicenseService had a critical flaw where the original exception could be lost when stream cleanup failed. The finally block would throw a new IOException during cleanup, replacing and hiding the actual import failure exception, making debugging extremely difficult. Added null/empty file validation at the start of the method. Refactored exception handling to use a primaryException variable that preserves the original error. Extracted stream cleanup logic to a separate method closeInputMapStreams() that adds close failures as suppressed exceptions instead of replacing the primary exception. Changed from: finally { if (closeFailure != null) throw closeFailure; } To: catch (Exception e) { primaryException = e; throw e; } finally { closeInputMapStreams(inputMap, primaryException); } This ensures that if license import fails with "Invalid license format" and stream close fails with "Stream already closed", both errors are preserved with the import error as primary and close error as suppressed. This fix improves debuggability of license upload failures and follows Java exception handling best practices for resource cleanup. Signed-off-by: melbiialy --- .../license/Sw360LicenseService.java | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/license/Sw360LicenseService.java b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/license/Sw360LicenseService.java index 95c0355efd..34c717aa4f 100644 --- a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/license/Sw360LicenseService.java +++ b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/license/Sw360LicenseService.java @@ -271,34 +271,44 @@ private void copyDataStreamToResponse(HttpServletResponse response, ByteArrayInp } public void uploadLicense(User sw360User, MultipartFile file, boolean overwriteIfExternalIdMatches, boolean overwriteIfIdMatchesEvenWithoutExternalIdMatch) throws IOException, TException { - final HashMap inputMap = new HashMap<>(); - + if (file == null || file.isEmpty()) { + throw new BadRequestClientException("License file is missing or empty"); + } if (!PermissionUtils.isUserAtLeast(UserGroup.ADMIN, sw360User)) { throw new BadRequestClientException("Unable to upload license file. User is not admin"); } + + final HashMap inputMap = new HashMap<>(); + Exception primaryException = null; + try (InputStream inputStream = file.getInputStream()) { ZipTools.extractZipToInputStreamMap(inputStream, inputMap); LicenseService.Iface sw360LicenseClient = getThriftLicenseClient(); final LicsImporter licsImporter = new LicsImporter(sw360LicenseClient, overwriteIfExternalIdMatches, overwriteIfIdMatchesEvenWithoutExternalIdMatch); licsImporter.importLics(sw360User, inputMap); + } catch (Exception e) { + primaryException = e; + throw e; } finally { - IOException closeFailure = null; - for (InputStream in : inputMap.values()) { - try { + closeInputMapStreams(inputMap, primaryException); + } + } + + private void closeInputMapStreams(Map inputMap, Exception primaryException) { + for (InputStream in : inputMap.values()) { + try { + if (in != null) { in.close(); - } catch (IOException e) { - if (closeFailure == null) { - closeFailure = e; - } else { - closeFailure.addSuppressed(e); - } } - } - if (closeFailure != null) { - throw closeFailure; + } catch (IOException e) { + if (primaryException != null) { + primaryException.addSuppressed(e); + } else { + throw new RuntimeException("Failed to close input stream", e); + } } } - } + } public RequestSummary importOsadlInformation(User sw360User) throws TException { LicenseService.Iface sw360LicenseClient = getThriftLicenseClient();