Skip to content

Fix stream leaks and default-charset use in CLI tools #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bact wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix stream leaks and default-charset use in CLI tools #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c89bb93
Fix stream leaks and default-charset use in CLI tools
bact Sep 18, 2026
841a939
Fix encoding
bact Sep 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 33 additions & 23 deletions src/main/java/org/spdx/tools/CompareSpdxDocs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,16 @@
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.UUID;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.spdx.core.InvalidSPDXAnalysisException;
import org.spdx.library.ModelCopyManager;
import org.spdx.library.model.v2.SpdxDocument;
import org.spdx.spreadsheetstore.SpreadsheetException;
import org.spdx.storage.IModelStore;
import org.spdx.storage.simple.InMemSpdxStore;
import org.spdx.tools.compare.MultiDocumentSpreadsheet;
import org.spdx.utility.compare.SpdxCompareException;
import org.spdx.utility.compare.SpdxComparer;
Expand Down Expand Up @@ -84,7 +82,7 @@ static int run(String[] args) {
return ExitCode.USAGE_ERROR;
}
if (args.length > MAX_ARGS) {
System.out.println("Too many SPDX documents specified. Must be less than "+String.valueOf(MAX_ARGS-1)+" document filenames");
System.out.println("Too many SPDX documents specified. Must be at most "+String.valueOf(MAX_ARGS-1)+" document filenames");
usage();
return ExitCode.USAGE_ERROR;
}
Expand Down Expand Up @@ -117,18 +115,20 @@ public static void onlineFunction(String[] args) throws OnlineToolException {
for (int i = 1; i < args.length; i++) {
try {
addDocToComparer(compareDocs, args[i], docNames, verificationErrors);
} catch (InvalidSPDXAnalysisException | IOException | InvalidFileNameException e) {
} catch (InvalidSPDXAnalysisException | IOException | InvalidFileNameException | RuntimeException e) {
throw new OnlineToolException("Error opening SPDX document "+args[i]+": "+e.getMessage());
}
}
List<String> normalizedDocNames = normalizeDocNames(docNames);
MultiDocumentSpreadsheet outSheet = null;
boolean success = false;
try {
outSheet = new MultiDocumentSpreadsheet(outputFile, true, false);
outSheet.importVerificationErrors(verificationErrors, normalizedDocNames);
SpdxComparer comparer = new SpdxComparer();
comparer.compare(compareDocs);
outSheet.importCompareResults(comparer, normalizedDocNames);
success = true;
} catch (SpreadsheetException e) {
throw new OnlineToolException("Unable to create output spreadsheet: "+e.getMessage());
} catch (InvalidSPDXAnalysisException e) {
Expand All @@ -143,10 +143,16 @@ public static void onlineFunction(String[] args) throws OnlineToolException {
logger.warn("Warning - error closing spreadsheet: "+e.getMessage());
}
}
if (!success) {
// don't leave a partial workbook that blocks a retry
try {
Files.deleteIfExists(outputFile.toPath());
} catch (IOException e) {
logger.warn("Warning - unable to delete incomplete output file: "+e.getMessage());
}
}
}
}



/**
* Adds all SPDX documents found in the file or directory to the compareDocs list
Expand Down Expand Up @@ -174,14 +180,8 @@ private static void addDocToComparer(List<SpdxDocument> compareDocs,
}
}
if (dupDocUri) {
// Make a unique URI by appending a UUID
String newUri = doc.getDocumentUri() + UUID.randomUUID();
warnings.add("Duplicate Document URI: " + doc.getDocumentUri() + " changed to " + newUri);
IModelStore newStore = new InMemSpdxStore();
ModelCopyManager copyManager = new ModelCopyManager();
SpdxDocument newDoc = new SpdxDocument(newStore, newUri, copyManager, false);
newDoc.copyFrom(doc);
doc = newDoc;
warnings.add("Duplicate Document URI: " + doc.getDocumentUri()
+ ". Document namespaces should be unique.");
}
compareDocs.add(doc);
if (!warnings.isEmpty()) {
Expand All @@ -190,10 +190,16 @@ private static void addDocToComparer(List<SpdxDocument> compareDocs,
verificationErrors.add(warnings);
docNames.add(filePath);
} else if (spdxDocOrDir.isDirectory()) {
for (File file:spdxDocOrDir.listFiles()) {
File[] files = spdxDocOrDir.listFiles();
if (files == null) {
throw new IOException("Unable to list the files in directory "+filePath);
}
// listFiles order is filesystem dependent
Arrays.sort(files);
for (File file:files) {
try {
addDocToComparer(compareDocs, file.getPath(), docNames, verificationErrors);
} catch (InvalidSPDXAnalysisException | IOException | InvalidFileNameException e) {
} catch (InvalidSPDXAnalysisException | IOException | InvalidFileNameException | RuntimeException e) {
System.out.println("Error deserializing "+file+". Skipping.");
continue;
}
Expand All @@ -207,7 +213,7 @@ private static void addDocToComparer(List<SpdxDocument> compareDocs,
* @param uriFilePaths Un-normalized file paths or URIs
* @return List of normalized doc names
*/
private static List<String> normalizeDocNames(List<String> uriFilePaths) {
static List<String> normalizeDocNames(List<String> uriFilePaths) {
List<String> docNames = new ArrayList<>();
if (uriFilePaths.size() < 1) {
return docNames;
Expand All @@ -228,13 +234,17 @@ private static List<String> normalizeDocNames(List<String> uriFilePaths) {
}
}
}
// Back up looking for the first path separator
for (int i = commonPrefixIndex; i >= 0; i--) {
if (uriFilePaths.get(0).charAt(i) == '/' || uriFilePaths.get(0).charAt(i) == '\\') {
commonPrefixIndex = i+1;
// Back up to just after the last path separator in the common prefix,
// so a name is never cut in the middle of a path segment
int lastSeparator = -1;
for (int i = commonPrefixIndex - 1; i >= 0; i--) {
char ch = uriFilePaths.get(0).charAt(i);
if (ch == '/' || ch == '\\') {
lastSeparator = i;
break;
}
}
commonPrefixIndex = lastSeparator + 1;
for (String uriFilePath:uriFilePaths) {
docNames.add(uriFilePath.substring(commonPrefixIndex).replace("\\", "/"));
}
Expand Down
43 changes: 20 additions & 23 deletions src/main/java/org/spdx/tools/GenerateVerificationCode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@
package org.spdx.tools;

import java.io.File;
import java.nio.file.Path;
import java.io.IOException;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;

import javax.annotation.Nullable;

Expand Down Expand Up @@ -81,8 +83,11 @@ static int run(String[] args) {
SpdxPackageVerificationCode verificationCode = generateVerificationCode(directoryPath, skippedRegex);
printVerificationCode(verificationCode);
return ExitCode.SUCCESS;
} catch (PatternSyntaxException ex) {
error("Invalid regular expression for the skipped files: "+ex.getMessage());
return ExitCode.USAGE_ERROR;
} catch (Exception ex) {
error("Error creating verification code: "+ex.getMessage());
System.out.println("Error creating verification code: "+ex.getMessage());
return ExitCode.ERROR;
}
}
Expand Down Expand Up @@ -115,46 +120,38 @@ public static SpdxPackageVerificationCode generateVerificationCode(String direct

/**
* Collect files to be skipped
* @param skippedRegex Regular Expression for file paths to be skipped
* @param skippedRegex Regular Expression for file paths to be skipped. It is applied against the
* path relative to the directory, with '/' as the separator on every platform
* @param dir Directory to scan for collecting skipped files
* @return
*/
private static File[] collectSkippedFiles(String skippedRegex, File dir) {
Pattern skippedPattern = Pattern.compile(skippedRegex);
List<File> skippedFiles = new ArrayList<>();
collectSkippedFiles(skippedPattern, skippedFiles, dir.getPath(), dir);
File[] retval = new File[skippedFiles.size()];
retval = skippedFiles.toArray(retval);
return retval;
collectSkippedFiles(skippedPattern, skippedFiles, dir.toPath(), dir);
return skippedFiles.toArray(new File[0]);
}

/**
* Internal method to recurse through the source directory collecting files to skip
* @param skippedPattern
* @param skippedFiles
* @param rootPath
* @param dir
* @return
* @param fileOrDir
*/
private static void collectSkippedFiles(Pattern skippedPattern,
List<File> skippedFiles, String rootPath, File dir) {
if (dir.isFile()) {
String relativePath = dir.getPath().substring(rootPath.length()+1);
List<File> skippedFiles, Path rootPath, File fileOrDir) {
if (fileOrDir.isFile()) {
String relativePath = rootPath.relativize(fileOrDir.toPath()).toString()
.replace(File.separatorChar, '/');
if (skippedPattern.matcher(relativePath).matches()) {
skippedFiles.add(dir);
skippedFiles.add(fileOrDir);
}
} else if (dir.isDirectory()) {
File[] children = dir.listFiles();
} else if (fileOrDir.isDirectory()) {
File[] children = fileOrDir.listFiles();
if (children != null) {
for (int i = 0; i < children.length; i++) {
if (children[i].isFile()) {
String relativePath = children[i].getPath().substring(rootPath.length()+1);
if (skippedPattern.matcher(relativePath).matches()) {
skippedFiles.add(children[i]);
}
} else if (children[i].isDirectory()) {
collectSkippedFiles(skippedPattern, skippedFiles, rootPath, children[i]);
}
for (File child : children) {
collectSkippedFiles(skippedPattern, skippedFiles, rootPath, child);
}
}
}
Expand Down
60 changes: 55 additions & 5 deletions src/main/java/org/spdx/tools/MatchingStandardLicenses.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,15 @@

import java.io.File;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.charset.CharacterCodingException;
import java.nio.charset.Charset;
import java.nio.charset.CodingErrorAction;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.util.List;

import org.apache.commons.io.ByteOrderMark;
import org.spdx.core.InvalidSPDXAnalysisException;
import org.spdx.utility.compare.LicenseCompareHelper;
import org.spdx.utility.compare.SpdxCompareException;
Expand Down Expand Up @@ -53,6 +58,10 @@ private MatchingStandardLicenses() {
static int MIN_ARGS = 1;
static int MAX_ARGS = 1;

/** UTF-32 first: the UTF-32LE BOM begins with the UTF-16LE BOM. */
private static final ByteOrderMark[] BOMS = {ByteOrderMark.UTF_32LE, ByteOrderMark.UTF_32BE,
ByteOrderMark.UTF_8, ByteOrderMark.UTF_16LE, ByteOrderMark.UTF_16BE};

/**
* Main entry point for the MatchingStandardLicenses tool.
* Delegates to {@link #run(String[])} and terminates the JVM with its exit status.
Expand Down Expand Up @@ -123,12 +132,53 @@ static int run(String[] args) {
}

/**
* @param textFile
* @return
* @throws IOException
* Reads a text file with the platform default charset as the fallback, see
* {@link #readAll(File, Charset)}.
* @param textFile file to read
* @return the file content, without any byte order mark
* @throws IOException on read error
*/
private static String readAll(File textFile) throws IOException {
return new String(Files.readAllBytes(textFile.toPath()), Charset.defaultCharset());
static String readAll(File textFile) throws IOException {
return readAll(textFile, Charset.defaultCharset());
}

/**
* Reads a text file. A UTF-8, UTF-16 or UTF-32 byte order mark selects the encoding and
* is removed. Otherwise the bytes are decoded as UTF-8, or with {@code fallback} if they
* are not valid UTF-8 (e.g. a legacy Windows ANSI file).
* @param textFile file to read
* @param fallback charset for content that has no byte order mark and is not valid UTF-8
* @return the file content, without any byte order mark
* @throws IOException on read error
*/
static String readAll(File textFile, Charset fallback) throws IOException {
byte[] bytes = Files.readAllBytes(textFile.toPath());
for (ByteOrderMark bom : BOMS) {
if (startsWith(bytes, bom)) {
return new String(bytes, bom.length(), bytes.length - bom.length(),
Charset.forName(bom.getCharsetName()));
}
}
try {
return StandardCharsets.UTF_8.newDecoder()
.onMalformedInput(CodingErrorAction.REPORT)
.onUnmappableCharacter(CodingErrorAction.REPORT)
.decode(ByteBuffer.wrap(bytes)).toString();
} catch (CharacterCodingException e) {
return new String(bytes, fallback);
}
}

private static boolean startsWith(byte[] bytes, ByteOrderMark bom) {
if (bytes.length < bom.length()) {
return false;
}
for (int i = 0; i < bom.length(); i++) {
if (bytes[i] != (byte) bom.get(i)) {
return false;
}
}
return true;
}

private static void usage() {
Expand Down
Loading
Loading