From 8a701e5895a339add83f0fd674f3c884bfc8caae Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Sat, 22 Feb 2025 13:35:27 +0100 Subject: [PATCH 1/2] Add nullability annotations This change adds nullability annotations ([JSpecify](https://jspecify.dev)) to `packageurl-java` and modifies somes behavior. Specifically: - It marks with `@Nullable` all public methods that can return `null`. - It marks with `@Nullable` all method parameters that can have a `null` value **without** throwing any exception. - It modifies the behavior of the remaining parameters: passing `null` to those parameters will result in an NPE, instead of `MalformedPackageURLException`. This is at least a behavior change (minor version bump), but it _might_ be a breaking change. --- pom.xml | 5 + .../MalformedPackageURLException.java | 4 +- .../com/github/packageurl/PackageURL.java | 218 +++++++++--------- .../github/packageurl/PackageURLBuilder.java | 69 +++--- .../com/github/packageurl/package-info.java | 23 ++ .../PackageURLConstraintValidator.java | 3 +- .../packageurl/validator/package-info.java | 25 ++ .../packageurl/PackageURLBuilderTest.java | 22 +- .../com/github/packageurl/PackageURLTest.java | 10 +- 9 files changed, 221 insertions(+), 158 deletions(-) create mode 100644 src/main/java/com/github/packageurl/validator/package-info.java diff --git a/pom.xml b/pom.xml index d1e6d30..0d9fb94 100644 --- a/pom.xml +++ b/pom.xml @@ -93,6 +93,11 @@ true provided + + org.jspecify + jspecify + 1.0.0 + junit junit diff --git a/src/main/java/com/github/packageurl/MalformedPackageURLException.java b/src/main/java/com/github/packageurl/MalformedPackageURLException.java index 47f6f72..56782ac 100644 --- a/src/main/java/com/github/packageurl/MalformedPackageURLException.java +++ b/src/main/java/com/github/packageurl/MalformedPackageURLException.java @@ -21,6 +21,8 @@ */ package com.github.packageurl; +import org.jspecify.annotations.Nullable; + /** * Exception class intended to be used for PackageURL parsing exceptions. * @@ -43,7 +45,7 @@ public MalformedPackageURLException() { * * @param msg the detail message. */ - public MalformedPackageURLException(String msg) { + public MalformedPackageURLException(@Nullable String msg) { super(msg); } diff --git a/src/main/java/com/github/packageurl/PackageURL.java b/src/main/java/com/github/packageurl/PackageURL.java index 4824b42..2046347 100644 --- a/src/main/java/com/github/packageurl/PackageURL.java +++ b/src/main/java/com/github/packageurl/PackageURL.java @@ -21,10 +21,11 @@ */ package com.github.packageurl; +import static java.util.Objects.requireNonNull; + import java.io.Serializable; import java.net.URI; import java.net.URISyntaxException; -import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; @@ -33,6 +34,7 @@ import java.util.TreeMap; import java.util.regex.Pattern; import java.util.stream.Collectors; +import org.jspecify.annotations.Nullable; /** *

Package-URL (aka purl) is a "mostly universal" URL to describe a package. A purl is a URL composed of seven components:

@@ -54,16 +56,18 @@ public final class PackageURL implements Serializable { private static final long serialVersionUID = 3243226021636427586L; private static final Pattern PATH_SPLITTER = Pattern.compile("/"); + private static final String SCHEME = "pkg"; /** * Constructs a new PackageURL object by parsing the specified string. * * @param purl a valid package URL string to parse * @throws MalformedPackageURLException if parsing fails + * @throws NullPointerException if {@code purl} is {@code null} * @since 1.0.0 */ public PackageURL(final String purl) throws MalformedPackageURLException { - parse(purl); + parse(requireNonNull(purl, "purl")); } /** @@ -82,34 +86,29 @@ public PackageURL(final String type, final String name) throws MalformedPackageU /** * Constructs a new PackageURL object. * - * @param type the type of package (i.e. maven, npm, gem, etc) + * @param type the type of package (i.e. maven, npm, gem, etc), not {@code null} * @param namespace the name prefix (i.e. group, owner, organization) - * @param name the name of the package + * @param name the name of the package, not {@code null} * @param version the version of the package * @param qualifiers an array of key/value pair qualifiers * @param subpath the subpath string * @throws MalformedPackageURLException if parsing fails + * @throws NullPointerException if {@code type} or {@code name} are {@code null} * @since 1.0.0 */ - public PackageURL(final String type, final String namespace, final String name, final String version, - final TreeMap qualifiers, final String subpath) + public PackageURL(final String type, final @Nullable String namespace, final String name, final @Nullable String version, + final @Nullable TreeMap qualifiers, final @Nullable String subpath) throws MalformedPackageURLException { - this.scheme = validateScheme("pkg"); - this.type = validateType(type); + this.type = validateType(requireNonNull(type, "type")); this.namespace = validateNamespace(namespace); - this.name = validateName(name); + this.name = validateName(requireNonNull(name, "name")); this.version = validateVersion(version); this.qualifiers = validateQualifiers(qualifiers); - this.subpath = validatePath(subpath, true); - verifyTypeConstraints(this.type, this.namespace, this.name); + this.subpath = validateSubPath(subpath); + verifyTypeConstraints(this.type, this.namespace); } - /** - * The PackageURL scheme constant - */ - private String scheme; - /** * The package "type" or package "protocol" such as maven, npm, nuget, gem, pypi, etc. * Required. @@ -120,7 +119,7 @@ public PackageURL(final String type, final String namespace, final String name, * The name prefix such as a Maven groupid, a Docker image owner, a GitHub user or organization. * Optional and type-specific. */ - private String namespace; + private @Nullable String namespace; /** * The name of the package. @@ -132,19 +131,19 @@ public PackageURL(final String type, final String namespace, final String name, * The version of the package. * Optional. */ - private String version; + private @Nullable String version; /** * Extra qualifying data for a package such as an OS, architecture, a distro, etc. * Optional and type-specific. */ - private Map qualifiers; + private @Nullable Map qualifiers; /** * Extra subpath within a package, relative to the package root. * Optional. */ - private String subpath; + private @Nullable String subpath; public PackageURLBuilder toBuilder() { @@ -170,7 +169,7 @@ public PackageURLBuilder toBuilder() { * @since 1.0.0 */ public String getScheme() { - return scheme; + return SCHEME; } /** @@ -189,7 +188,7 @@ public String getType() { * @return the namespace * @since 1.0.0 */ - public String getNamespace() { + public @Nullable String getNamespace() { return namespace; } @@ -209,7 +208,7 @@ public String getName() { * @return the version of the package * @since 1.0.0 */ - public String getVersion() { + public @Nullable String getVersion() { return version; } @@ -219,7 +218,7 @@ public String getVersion() { * @return qualifiers * @since 1.0.0 */ - public Map getQualifiers() { + public @Nullable Map getQualifiers() { return (qualifiers != null)? Collections.unmodifiableMap(qualifiers) : null; } @@ -229,20 +228,19 @@ public Map getQualifiers() { * @return the subpath * @since 1.0.0 */ - public String getSubpath() { + public @Nullable String getSubpath() { return subpath; } - private String validateScheme(final String value) throws MalformedPackageURLException { - if ("pkg".equals(value)) { - return "pkg"; + private void validateScheme(final String value) throws MalformedPackageURLException { + if (!SCHEME.equals(value)) { + throw new MalformedPackageURLException("The PackageURL scheme is invalid"); } - throw new MalformedPackageURLException("The PackageURL scheme is invalid"); } - private String validateType(final String value) throws MalformedPackageURLException { - if (value == null || value.isEmpty()) { - throw new MalformedPackageURLException("The PackageURL type cannot be null or empty"); + private static String validateType(final String value) throws MalformedPackageURLException { + if (value.isEmpty()) { + throw new MalformedPackageURLException("The PackageURL type cannot be empty"); } if (value.charAt(0) >= '0' && value.charAt(0) <= '9') { throw new MalformedPackageURLException("The PackageURL type cannot start with a number"); @@ -256,27 +254,29 @@ private String validateType(final String value) throws MalformedPackageURLExcept return retVal; } - private String validateNamespace(final String value) throws MalformedPackageURLException { - if (value == null || value.isEmpty()) { + private @Nullable String validateNamespace(final @Nullable String value) throws MalformedPackageURLException { + if (isEmpty(value)) { return null; } return validateNamespace(value.split("/")); } - private String validateNamespace(final String[] values) throws MalformedPackageURLException { - if (values == null || values.length == 0) { + private @Nullable String validateNamespace(final String[] values) throws MalformedPackageURLException { + if (values.length == 0) { return null; } final String tempNamespace = validatePath(values, false); - String retVal; + String retVal = null; switch (type) { case StandardTypes.BITBUCKET: case StandardTypes.DEBIAN: case StandardTypes.GITHUB: case StandardTypes.GOLANG: case StandardTypes.RPM: - retVal = tempNamespace.toLowerCase(); + if (tempNamespace != null) { + retVal = tempNamespace.toLowerCase(); + } break; default: retVal = tempNamespace; @@ -286,7 +286,7 @@ private String validateNamespace(final String[] values) throws MalformedPackageU } private String validateName(final String value) throws MalformedPackageURLException { - if (value == null || value.isEmpty()) { + if (value.isEmpty()) { throw new MalformedPackageURLException("The PackageURL name specified is invalid"); } String temp; @@ -307,61 +307,59 @@ private String validateName(final String value) throws MalformedPackageURLExcept return temp; } - private String validateVersion(final String value) { - if (value == null) { - return null; - } - return value; + private @Nullable String validateVersion(final @Nullable String value) { + return isEmpty(value) ? null : value; } - private Map validateQualifiers(final Map values) throws MalformedPackageURLException { - if (values == null) { + private @Nullable Map validateQualifiers(final @Nullable Map values) throws MalformedPackageURLException { + if (values == null || values.isEmpty()) { return null; } for (Map.Entry entry : values.entrySet()) { - validateKey(entry.getKey()); - final String value = entry.getValue(); - if (value == null || value.isEmpty()) { - throw new MalformedPackageURLException("The PackageURL specified contains a qualifier key with an empty or null value"); - } + String key = entry.getKey(); + validateKey(key); + validateValue(key, entry.getValue()); } return values; } - private String validateKey(final String value) throws MalformedPackageURLException { - if (value == null || value.isEmpty()) { - throw new MalformedPackageURLException("Qualifier key is invalid: " + value); + private static void validateKey(final @Nullable String value) throws MalformedPackageURLException { + if (isEmpty(value)) { + throw new MalformedPackageURLException("The specified PackageURL contains an empty or null qualifier key"); } - final String retValue = value.toLowerCase(); if ((value.charAt(0) >= '0' && value.charAt(0) <= '9') || !value.chars().allMatch(c -> (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-' || c == '_')) { - throw new MalformedPackageURLException("Qualifier key is invalid: " + value); + throw new MalformedPackageURLException("The specified PackageURL contains an invalid qualifier key: " + value); } - return retValue; } - private String validatePath(final String value, final boolean isSubpath) throws MalformedPackageURLException { - if (value == null || value.isEmpty()) { + private static void validateValue(final String key, final @Nullable String value) throws MalformedPackageURLException { + if (isEmpty(value)) { + throw new MalformedPackageURLException("The specified PackageURL contains an empty or null qualifier value for key " + key); + } + } + + private @Nullable String validateSubPath(final @Nullable String value) throws MalformedPackageURLException { + if (isEmpty(value)) { return null; } - return validatePath(value.split("/"), isSubpath); + return validatePath(value.split("/"), true); } - private String validatePath(final String[] segments, final boolean isSubpath) throws MalformedPackageURLException { - if (segments == null || segments.length == 0) { + private static @Nullable String validatePath(final String[] segments, final boolean isSubPath) throws MalformedPackageURLException { + if (segments.length == 0) { return null; } try { return Arrays.stream(segments) - .map(segment -> { - if (isSubpath && ("..".equals(segment) || ".".equals(segment))) { + .peek(segment -> { + if (isSubPath && ("..".equals(segment) || ".".equals(segment))) { throw new ValidationException("Segments in the subpath may not be a period ('.') or repeated period ('..')"); } else if (segment.contains("/")) { throw new ValidationException("Segments in the namespace and subpath may not contain a forward slash ('/')"); } else if (segment.isEmpty()) { throw new ValidationException("Segments in the namespace and subpath may not be empty"); } - return segment; }).collect(Collectors.joining("/")); } catch (ValidationException ex) { throw new MalformedPackageURLException(ex.getMessage()); @@ -397,28 +395,22 @@ public String canonicalize() { */ private String canonicalize(boolean coordinatesOnly) { final StringBuilder purl = new StringBuilder(); - purl.append(scheme).append(":"); - if (type != null) { - purl.append(type); - } - purl.append("/"); + purl.append(SCHEME).append(":").append(type).append("/"); if (namespace != null) { purl.append(encodePath(namespace)); purl.append("/"); } - if (name != null) { - purl.append(percentEncode(name)); - } + purl.append(percentEncode(name)); if (version != null) { purl.append("@").append(percentEncode(version)); } if (! coordinatesOnly) { - if (qualifiers != null && qualifiers.size() > 0) { + if (qualifiers != null) { purl.append("?"); - qualifiers.entrySet().stream().forEachOrdered((entry) -> { - purl.append(entry.getKey().toLowerCase()); + qualifiers.forEach((key, value) -> { + purl.append(key.toLowerCase()); purl.append("="); - purl.append(percentEncode(entry.getValue())); + purl.append(percentEncode(value)); purl.append("&"); }); purl.setLength(purl.length() - 1); @@ -437,16 +429,12 @@ private String canonicalize(boolean coordinatesOnly) { * @return an encoded String */ private String percentEncode(final String input) { - return uriEncode(input, StandardCharsets.UTF_8); - } - - private static String uriEncode(String source, Charset charset) { - if (source == null || source.length() == 0) { - return source; + if (input.isEmpty()) { + return input; } StringBuilder builder = new StringBuilder(); - for (byte b : source.getBytes(charset)) { + for (byte b : input.getBytes(StandardCharsets.UTF_8)) { if (isUnreserved(b)) { builder.append((char) b); } @@ -479,9 +467,6 @@ private static boolean isDigit(int c) { * @return a decoded String */ private String percentDecode(final String input) { - if (input == null) { - return null; - } final String decoded = uriDecode(input); if (!decoded.equals(input)) { return decoded; @@ -489,10 +474,14 @@ private String percentDecode(final String input) { return input; } + /** + * Decodes a percent-encoded string. + * + * @param source string to decode, not {@code null} + * @return A decoded string + * @throws NullPointerException if {@code source} is {@code null} + */ public static String uriDecode(String source) { - if (source == null) { - return source; - } int length = source.length(); StringBuilder builder = new StringBuilder(); for (int i = 0; i < length; i++) { @@ -518,7 +507,7 @@ public static String uriDecode(String source) { * @throws MalformedPackageURLException if an exception occurs when parsing */ private void parse(final String purl) throws MalformedPackageURLException { - if (purl == null || purl.trim().isEmpty()) { + if (purl.isEmpty()) { throw new MalformedPackageURLException("Invalid purl: Contains an empty or null value"); } @@ -529,7 +518,7 @@ private void parse(final String purl) throws MalformedPackageURLException { throw new MalformedPackageURLException("Invalid purl: Contains parts not supported by the purl spec"); } - this.scheme = validateScheme(uri.getScheme()); + validateScheme(uri.getScheme()); // subpath is optional - check for existence if (uri.getRawFragment() != null && !uri.getRawFragment().isEmpty()) { @@ -588,7 +577,7 @@ private void parse(final String purl) throws MalformedPackageURLException { remainder.setLength(index); this.namespace = validateNamespace(parsePath(remainder.substring(start), false)); } - verifyTypeConstraints(this.type, this.namespace, this.name); + verifyTypeConstraints(this.type, this.namespace); } catch (URISyntaxException e) { throw new MalformedPackageURLException("Invalid purl: " + e.getMessage()); } @@ -598,22 +587,21 @@ private void parse(final String purl) throws MalformedPackageURLException { * Some purl types may have specific constraints. This method attempts to verify them. * @param type the purl type * @param namespace the purl namespace - * @param name the purl name * @throws MalformedPackageURLException if constraints are not met */ - private void verifyTypeConstraints(String type, String namespace, String name) throws MalformedPackageURLException { + private void verifyTypeConstraints(String type, @Nullable String namespace) throws MalformedPackageURLException { if (StandardTypes.MAVEN.equals(type)) { - if (namespace == null || namespace.isEmpty() || name == null || name.isEmpty()) { + if (isEmpty(namespace)) { throw new MalformedPackageURLException("The PackageURL specified is invalid. Maven requires both a namespace and name."); } } } @SuppressWarnings("StringSplitter")//reason: surprising behavior is okay in this case - private Map parseQualifiers(final String encodedString) throws MalformedPackageURLException { + private @Nullable Map parseQualifiers(final String encodedString) throws MalformedPackageURLException { try { final TreeMap results = Arrays.stream(encodedString.split("&")) - .collect(TreeMap::new, + .collect(TreeMap::new, (map, value) -> { final String[] entry = value.split("=", 2); if (entry.length == 2 && !entry[1].isEmpty()) { @@ -622,7 +610,7 @@ private Map parseQualifiers(final String encodedString) throws M } } }, - TreeMap::putAll); + TreeMap::putAll); return validateQualifiers(results); } catch (ValidationException ex) { throw new MalformedPackageURLException(ex.getMessage()); @@ -630,18 +618,18 @@ private Map parseQualifiers(final String encodedString) throws M } @SuppressWarnings("StringSplitter")//reason: surprising behavior is okay in this case - private String[] parsePath(final String value, final boolean isSubpath) throws MalformedPackageURLException { - if (value == null || value.isEmpty()) { - return null; + private String[] parsePath(final @Nullable String value, final boolean isSubpath) { + if (isEmpty(value)) { + return new String[0]; } return PATH_SPLITTER.splitAsStream(value) .filter(segment -> !segment.isEmpty() && !(isSubpath && (".".equals(segment) || "..".equals(segment)))) - .map(segment -> percentDecode(segment)) + .map(this::percentDecode) .toArray(String[]::new); } private String encodePath(final String path) { - return Arrays.stream(path.split("/")).map(segment -> percentEncode(segment)).collect(Collectors.joining("/")); + return Arrays.stream(path.split("/")).map(this::percentEncode).collect(Collectors.joining("/")); } /** @@ -667,15 +655,14 @@ public boolean isBaseEquals(final PackageURL purl) { * the qualifier (querystring). This includes equivalence of: scheme, type, namespace, * name, and version, but excludes qualifier and subpath from evaluation. * - * @param purl the Package URL to evaluate + * @param purl the Package URL to evaluate, not {@code null} * @return true if equivalence passes, false if not * @since 1.4.0 */ public boolean isCoordinatesEquals(final PackageURL purl) { - return Objects.equals(scheme, purl.scheme) && - Objects.equals(type, purl.type) && + return type.equals(purl.type) && Objects.equals(namespace, purl.namespace) && - Objects.equals(name, purl.name) && + name.equals(purl.name) && Objects.equals(version, purl.version); } @@ -695,7 +682,7 @@ public String getCoordinates() { * canonical values. Canonical equivalence is especially useful for qualifiers, which * can be in any order, but have a predictable order in canonicalized form. * - * @param purl the Package URL to evaluate + * @param purl the Package URL to evaluate, not {@code null} * @return true if equivalence passes, false if not * @since 1.2.0 */ @@ -703,15 +690,18 @@ public boolean isCanonicalEquals(final PackageURL purl) { return (this.canonicalize().equals(purl.canonicalize())); } + private static boolean isEmpty(@Nullable String value) { + return value == null || value.isEmpty(); + } + @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; final PackageURL other = (PackageURL) o; - return Objects.equals(scheme, other.scheme) && - Objects.equals(type, other.type) && + return type.equals(other.type) && Objects.equals(namespace, other.namespace) && - Objects.equals(name, other.name) && + name.equals(other.name) && Objects.equals(version, other.version) && Objects.equals(qualifiers, other.qualifiers) && Objects.equals(subpath, other.subpath); @@ -719,7 +709,7 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(scheme, type, namespace, name, version, qualifiers, subpath); + return Objects.hash(type, namespace, name, version, qualifiers, subpath); } /** diff --git a/src/main/java/com/github/packageurl/PackageURLBuilder.java b/src/main/java/com/github/packageurl/PackageURLBuilder.java index 94fcd05..a59372b 100644 --- a/src/main/java/com/github/packageurl/PackageURLBuilder.java +++ b/src/main/java/com/github/packageurl/PackageURLBuilder.java @@ -21,18 +21,22 @@ */ package com.github.packageurl; +import static java.util.Objects.requireNonNull; + +import java.util.Objects; import java.util.TreeMap; +import org.jspecify.annotations.Nullable; /** * A builder construct for Package-URL objects. */ public final class PackageURLBuilder { - private String type = null; - private String namespace = null; - private String name = null; - private String version = null; - private String subpath = null; - private TreeMap qualifiers = null; + private @Nullable String type = null; + private @Nullable String namespace = null; + private @Nullable String name = null; + private @Nullable String version = null; + private @Nullable String subpath = null; + private @Nullable TreeMap qualifiers = null; private PackageURLBuilder() { //empty constructor for utility class @@ -50,24 +54,25 @@ public static PackageURLBuilder aPackageURL() { /** * Adds the package URL type. * - * @param type the package type + * @param type the package type, not {@code null} * @return a reference to the builder + * @throws NullPointerException if the argument is {@code null} * @see PackageURL#getName() * @see com.github.packageurl.PackageURL.StandardTypes */ public PackageURLBuilder withType(final String type) { - this.type = type; + this.type = requireNonNull(type, "type"); return this; } /** * Adds the package namespace. * - * @param namespace the package namespace + * @param namespace the package namespace or {@code null} * @return a reference to the builder * @see PackageURL#getNamespace() */ - public PackageURLBuilder withNamespace(final String namespace) { + public PackageURLBuilder withNamespace(final @Nullable String namespace) { this.namespace = namespace; return this; } @@ -75,23 +80,24 @@ public PackageURLBuilder withNamespace(final String namespace) { /** * Adds the package name. * - * @param name the package name + * @param name the package name, not {@code null} * @return a reference to the builder + * @throws NullPointerException if the argument is {@code null} * @see PackageURL#getName() */ public PackageURLBuilder withName(final String name) { - this.name = name; + this.name = requireNonNull(name, "name"); return this; } /** * Adds the package version. * - * @param version the package version + * @param version the package version or {@code null} * @return a reference to the builder * @see PackageURL#getVersion() */ - public PackageURLBuilder withVersion(final String version) { + public PackageURLBuilder withVersion(final @Nullable String version) { this.version = version; return this; } @@ -99,11 +105,11 @@ public PackageURLBuilder withVersion(final String version) { /** * Adds the package subpath. * - * @param subpath the package subpath + * @param subpath the package subpath or {@code null} * @return a reference to the builder * @see PackageURL#getSubpath() */ - public PackageURLBuilder withSubpath(final String subpath) { + public PackageURLBuilder withSubpath(final @Nullable String subpath) { this.subpath = subpath; return this; } @@ -111,16 +117,17 @@ public PackageURLBuilder withSubpath(final String subpath) { /** * Adds a package qualifier. * - * @param key the package qualifier key - * @param value the package qualifier value + * @param key the package qualifier key, not {@code null} + * @param value the package qualifier value, not {@code null} * @return a reference to the builder + * @throws NullPointerException if either {@code key} or {@code value} are {@code null} * @see PackageURL#getQualifiers() */ public PackageURLBuilder withQualifier(final String key, final String value) { if (qualifiers == null) { qualifiers = new TreeMap<>(); } - qualifiers.put(key, value); + qualifiers.put(requireNonNull(key, "qualifier key can not be null"), requireNonNull(value, "qualifier value can not be null")); return this; } @@ -128,10 +135,11 @@ public PackageURLBuilder withQualifier(final String key, final String value) { * Removes a package qualifier. This is a no-op if the qualifier is not present. * @param key the package qualifier key to remove * @return a reference to the builder + * @throws NullPointerException if {@code key} is {@code null} */ public PackageURLBuilder withoutQualifier(final String key) { if (qualifiers != null) { - qualifiers.remove(key); + qualifiers.remove(requireNonNull(key)); if (qualifiers.isEmpty()) { qualifiers = null; } } return this; @@ -150,7 +158,7 @@ public PackageURLBuilder withNoQualifiers() { * Returns current type value set in the builder. * @return type set in this builder */ - public String getType() { + public @Nullable String getType() { return type; } @@ -158,7 +166,7 @@ public String getType() { * Returns current namespace value set in the builder. * @return namespace set in this builder */ - public String getNamespace() { + public @Nullable String getNamespace() { return namespace; } @@ -166,7 +174,7 @@ public String getNamespace() { * Returns current name value set in the builder. * @return name set in this builder */ - public String getName() { + public @Nullable String getName() { return name; } @@ -174,7 +182,7 @@ public String getName() { * Returns current version value set in the builder. * @return version set in this builder */ - public String getVersion() { + public @Nullable String getVersion() { return version; } @@ -182,7 +190,7 @@ public String getVersion() { * Returns current subpath value set in the builder. * @return subpath set in this builder */ - public String getSubpath() { + public @Nullable String getSubpath() { return subpath; } @@ -201,9 +209,8 @@ public TreeMap getQualifiers() { * @param key qualifier key * @return qualifier value or {@code null} if one is not set. */ - public String getQualifier(String key) { - if (qualifiers == null) { return null; } - return qualifiers.get(key); + public @Nullable String getQualifier(String key) { + return qualifiers == null ? null : qualifiers.get(key); } /** @@ -213,6 +220,12 @@ public String getQualifier(String key) { * @throws MalformedPackageURLException thrown if the type or name has not been specified or if a field fails validation */ public PackageURL build() throws MalformedPackageURLException { + if (type == null) { + throw new MalformedPackageURLException("type is required"); + } + if (name == null) { + throw new MalformedPackageURLException("name is required"); + } return new PackageURL(type, namespace, name, version, qualifiers, subpath); } } diff --git a/src/main/java/com/github/packageurl/package-info.java b/src/main/java/com/github/packageurl/package-info.java index 6a7436c..55397be 100644 --- a/src/main/java/com/github/packageurl/package-info.java +++ b/src/main/java/com/github/packageurl/package-info.java @@ -1,6 +1,29 @@ +/* + * MIT License + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ /** *

Java implementation of the Package-URL Specification.

*

https://github.com/package-url/purl-spec

*/ +@NullMarked package com.github.packageurl; +import org.jspecify.annotations.NullMarked; \ No newline at end of file diff --git a/src/main/java/com/github/packageurl/validator/PackageURLConstraintValidator.java b/src/main/java/com/github/packageurl/validator/PackageURLConstraintValidator.java index 3c6a839..ecf70b3 100644 --- a/src/main/java/com/github/packageurl/validator/PackageURLConstraintValidator.java +++ b/src/main/java/com/github/packageurl/validator/PackageURLConstraintValidator.java @@ -24,6 +24,7 @@ import com.github.packageurl.MalformedPackageURLException; import javax.validation.ConstraintValidator; import javax.validation.ConstraintValidatorContext; +import org.jspecify.annotations.Nullable; /** * A JSR-303 compliant validator that validates String fields conform to the Package URL specification. @@ -32,7 +33,7 @@ public class PackageURLConstraintValidator implements ConstraintValidator { @Override - public boolean isValid(String value, ConstraintValidatorContext context) { + public boolean isValid(@Nullable String value, ConstraintValidatorContext context) { try { if (value != null) { new com.github.packageurl.PackageURL(value); diff --git a/src/main/java/com/github/packageurl/validator/package-info.java b/src/main/java/com/github/packageurl/validator/package-info.java new file mode 100644 index 0000000..03dc629 --- /dev/null +++ b/src/main/java/com/github/packageurl/validator/package-info.java @@ -0,0 +1,25 @@ +/* + * MIT License + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +@NullMarked +package com.github.packageurl.validator; + +import org.jspecify.annotations.NullMarked; \ No newline at end of file diff --git a/src/test/java/com/github/packageurl/PackageURLBuilderTest.java b/src/test/java/com/github/packageurl/PackageURLBuilderTest.java index b52d735..e689ce9 100644 --- a/src/test/java/com/github/packageurl/PackageURLBuilderTest.java +++ b/src/test/java/com/github/packageurl/PackageURLBuilderTest.java @@ -21,6 +21,8 @@ */ package com.github.packageurl; +import org.hamcrest.CoreMatchers; +import org.hamcrest.MatcherAssert; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; @@ -28,6 +30,8 @@ import java.util.Map; +import static org.hamcrest.CoreMatchers.allOf; +import static org.hamcrest.CoreMatchers.containsString; import static org.junit.Assert.*; public class PackageURLBuilderTest { @@ -96,8 +100,8 @@ public void testPackageURLBuilder() throws MalformedPackageURLException { @Test public void testPackageURLBuilderException1() throws MalformedPackageURLException { exception.expect(MalformedPackageURLException.class); - exception.expectMessage("contains a qualifier key with an empty or null"); - PackageURL purl = PackageURLBuilder.aPackageURL() + exception.expectMessage(allOf(containsString("qualifier value"), containsString("empty or null"))); + PackageURLBuilder.aPackageURL() .withType("type") .withName("name") .withQualifier("key","") @@ -107,8 +111,8 @@ public void testPackageURLBuilderException1() throws MalformedPackageURLExceptio @Test public void testPackageURLBuilderException1Null() throws MalformedPackageURLException { - exception.expect(MalformedPackageURLException.class); - exception.expectMessage("contains a qualifier key with an empty or null"); + exception.expect(NullPointerException.class); + exception.expectMessage("can not be null"); PackageURLBuilder.aPackageURL() .withType("type") .withName("name") @@ -120,7 +124,7 @@ public void testPackageURLBuilderException1Null() throws MalformedPackageURLExce @Test public void testPackageURLBuilderException2() throws MalformedPackageURLException { exception.expect(MalformedPackageURLException.class); - PackageURL purl = PackageURLBuilder.aPackageURL() + PackageURLBuilder.aPackageURL() .withType("type") .withNamespace("invalid//namespace") .withName("name") @@ -131,7 +135,7 @@ public void testPackageURLBuilderException2() throws MalformedPackageURLExceptio @Test public void testPackageURLBuilderException3() throws MalformedPackageURLException { exception.expect(MalformedPackageURLException.class); - PackageURL purl = PackageURLBuilder.aPackageURL() + PackageURLBuilder.aPackageURL() .withType("typ^e") .withSubpath("invalid/name%2Fspace") .withName("name") @@ -142,7 +146,7 @@ public void testPackageURLBuilderException3() throws MalformedPackageURLExceptio @Test public void testPackageURLBuilderException4() throws MalformedPackageURLException { exception.expect(MalformedPackageURLException.class); - PackageURL purl = PackageURLBuilder.aPackageURL() + PackageURLBuilder.aPackageURL() .withType("0_type") .withName("name") .build(); @@ -152,7 +156,7 @@ public void testPackageURLBuilderException4() throws MalformedPackageURLExceptio @Test public void testPackageURLBuilderException5() throws MalformedPackageURLException { exception.expect(MalformedPackageURLException.class); - PackageURL purl = PackageURLBuilder.aPackageURL() + PackageURLBuilder.aPackageURL() .withType("ype") .withName("name") .withQualifier("0_key","value") @@ -163,7 +167,7 @@ public void testPackageURLBuilderException5() throws MalformedPackageURLExceptio @Test public void testPackageURLBuilderException6() throws MalformedPackageURLException { exception.expect(MalformedPackageURLException.class); - PackageURL purl = PackageURLBuilder.aPackageURL() + PackageURLBuilder.aPackageURL() .withType("ype") .withName("name") .withQualifier("","value") diff --git a/src/test/java/com/github/packageurl/PackageURLTest.java b/src/test/java/com/github/packageurl/PackageURLTest.java index d7ddf18..b54ba4a 100644 --- a/src/test/java/com/github/packageurl/PackageURLTest.java +++ b/src/test/java/com/github/packageurl/PackageURLTest.java @@ -139,8 +139,8 @@ public void testConstructorParameters() throws MalformedPackageURLException { if (invalid) { try { PackageURL purl = new PackageURL(type, namespace, name, version, map, subpath); - Assert.fail("Invalid package url components should have caused an exception: " + purl.toString()); - } catch (MalformedPackageURLException e) { + Assert.fail("Invalid package url components should have caused an exception: " + purl); + } catch (NullPointerException | MalformedPackageURLException e) { Assert.assertNotNull(e.getMessage()); } continue; @@ -220,7 +220,7 @@ public void testConstructorWithInvalidSubpath() throws MalformedPackageURLExcept @Test public void testConstructorWithNullPurl() throws MalformedPackageURLException { - exception.expect(MalformedPackageURLException.class); + exception.expect(NullPointerException.class); PackageURL purl = new PackageURL(null); Assert.fail("constructor with null purl should have thrown an error and this line should not be reached"); @@ -289,10 +289,10 @@ public void testStandardTypes() { } @Test - public void testBaseEquals() throws Exception { + public void testCoordinatesEquals() throws Exception { PackageURL p1 = new PackageURL("pkg:generic/acme/example-component@1.0.0?key1=value1&key2=value2"); PackageURL p2 = new PackageURL("pkg:generic/acme/example-component@1.0.0"); - Assert.assertTrue(p1.isBaseEquals(p2)); + Assert.assertTrue(p1.isCoordinatesEquals(p2)); } @Test From d749fb2d46e39c6033fcc608d3ebda641c29ae2b Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Wed, 12 Mar 2025 14:16:08 +0100 Subject: [PATCH 2/2] Fix qualifier semantics --- .../com/github/packageurl/PackageURL.java | 20 ++++++-------- .../github/packageurl/PackageURLBuilder.java | 22 ++++++++++----- .../packageurl/PackageURLBuilderTest.java | 4 +-- .../com/github/packageurl/PackageURLTest.java | 27 +++++++++---------- 4 files changed, 38 insertions(+), 35 deletions(-) diff --git a/src/main/java/com/github/packageurl/PackageURL.java b/src/main/java/com/github/packageurl/PackageURL.java index 226cf68..b35b4a7 100644 --- a/src/main/java/com/github/packageurl/PackageURL.java +++ b/src/main/java/com/github/packageurl/PackageURL.java @@ -122,7 +122,7 @@ public PackageURL(final String type, final @Nullable String namespace, final Str * @since 1.6.0 */ public PackageURL(final String type, final @Nullable String namespace, final String name, final @Nullable String version, - final @Nullable Map qualifiers, final @Nullable String subpath) + final @Nullable Map qualifiers, final @Nullable String subpath) throws MalformedPackageURLException { this(type, namespace, name, version, (qualifiers != null) ? new TreeMap<>(qualifiers) : null, subpath); } @@ -180,18 +180,13 @@ public PackageURL(final String type, final @Nullable String namespace, final Str * @deprecated Use {@link PackageURLBuilder#aPackageURL(PackageURL)} or {@link PackageURLBuilder#aPackageURL(String)} */ public PackageURLBuilder toBuilder() { - PackageURLBuilder builder = PackageURLBuilder.aPackageURL() + return PackageURLBuilder.aPackageURL() .withType(getType()) .withNamespace(getNamespace()) .withName(getName()) .withVersion(getVersion()) + .withQualifiers(getQualifiers()) .withSubpath(getSubpath()); - - if (qualifiers != null) { - qualifiers.forEach(builder::withQualifier); - } - - return builder; } /** @@ -247,11 +242,12 @@ public String getName() { /** * Returns extra qualifying data for a package such as an OS, architecture, a distro, etc. * This method returns an UnmodifiableMap. - * @return qualifiers + * + * @return all the qualifiers, or an empty map if none are set * @since 1.0.0 */ - public @Nullable Map getQualifiers() { - return (qualifiers != null) ? Collections.unmodifiableMap(qualifiers) : null; + public Map getQualifiers() { + return qualifiers != null ? Collections.unmodifiableMap(qualifiers) : Collections.emptyMap(); } /** @@ -728,7 +724,7 @@ private void verifyTypeConstraints(String type, @Nullable String namespace, @Nul try { final TreeMap results = qualifiers.entrySet().stream() - .filter(entry -> isEmpty(entry.getValue())) + .filter(entry -> !isEmpty(entry.getValue())) .collect(TreeMap::new, (map, value) -> map.put(toLowerCase(value.getKey()), value.getValue()), TreeMap::putAll); diff --git a/src/main/java/com/github/packageurl/PackageURLBuilder.java b/src/main/java/com/github/packageurl/PackageURLBuilder.java index 747f062..4544341 100644 --- a/src/main/java/com/github/packageurl/PackageURLBuilder.java +++ b/src/main/java/com/github/packageurl/PackageURLBuilder.java @@ -139,18 +139,28 @@ public PackageURLBuilder withSubpath(final @Nullable String subpath) { /** * Adds a package qualifier. + *

+ * If {@code value} is empty or {@code null}, the given qualifier is removed instead. + *

* * @param key the package qualifier key, not {@code null} - * @param value the package qualifier value, not {@code null} + * @param value the package qualifier value or {@code null} * @return a reference to the builder - * @throws NullPointerException if either {@code key} or {@code value} are {@code null} + * @throws NullPointerException if {@code key} is {@code null} * @see PackageURL#getQualifiers() */ - public PackageURLBuilder withQualifier(final String key, final String value) { - if (qualifiers == null) { - qualifiers = new TreeMap<>(); + public PackageURLBuilder withQualifier(final String key, final @Nullable String value) { + requireNonNull(key, "qualifier key can not be null"); + if (value == null || value.isEmpty()) { + if (qualifiers != null) { + qualifiers.remove(key); + } + } else { + if (qualifiers == null) { + qualifiers = new TreeMap<>(); + } + qualifiers.put(requireNonNull(key, "qualifier key can not be null"), value); } - qualifiers.put(requireNonNull(key, "qualifier key can not be null"), requireNonNull(value, "qualifier value can not be null")); return this; } diff --git a/src/test/java/com/github/packageurl/PackageURLBuilderTest.java b/src/test/java/com/github/packageurl/PackageURLBuilderTest.java index 42b0b37..3b682d5 100644 --- a/src/test/java/com/github/packageurl/PackageURLBuilderTest.java +++ b/src/test/java/com/github/packageurl/PackageURLBuilderTest.java @@ -106,7 +106,7 @@ public void testPackageURLBuilderException1() throws MalformedPackageURLExceptio .withName("name") .withQualifier("key","") .build(); - assertNull(purl.getQualifiers()); + assertEquals("qualifier count", 0, purl.getQualifiers().size()); } @Test @@ -116,7 +116,7 @@ public void testPackageURLBuilderException1Null() throws MalformedPackageURLExce .withName("name") .withQualifier("key",null) .build(); - assertNull(purl.getQualifiers()); + assertEquals("qualifier count", 0, purl.getQualifiers().size()); } @Test diff --git a/src/test/java/com/github/packageurl/PackageURLTest.java b/src/test/java/com/github/packageurl/PackageURLTest.java index 7a13998..508373c 100644 --- a/src/test/java/com/github/packageurl/PackageURLTest.java +++ b/src/test/java/com/github/packageurl/PackageURLTest.java @@ -24,14 +24,13 @@ import java.io.IOException; import java.io.InputStream; import java.util.HashMap; -import java.util.Map; import java.util.Locale; +import java.util.Map; import java.util.TreeMap; - import org.json.JSONArray; import org.json.JSONObject; -import org.junit.AfterClass; import org.json.JSONTokener; +import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Rule; @@ -107,11 +106,9 @@ public void testConstructorParsing() throws Exception { Assert.assertEquals(name, purl.getName()); Assert.assertEquals(version, purl.getVersion()); Assert.assertEquals(subpath, purl.getSubpath()); - if (qualifiers == null) { - Assert.assertNull(purl.getQualifiers()); - } else { - Assert.assertNotNull(purl.getQualifiers()); - Assert.assertEquals(qualifiers.length(), purl.getQualifiers().size()); + Assert.assertNotNull(purl.getQualifiers()); + Assert.assertEquals("qualifier count", qualifiers != null ? qualifiers.length() : 0, purl.getQualifiers().size()); + if (qualifiers != null){ qualifiers.keySet().forEach(key -> { String value = qualifiers.getString(key); Assert.assertTrue(purl.getQualifiers().containsKey(key)); @@ -142,7 +139,7 @@ public void testConstructorParameters() throws MalformedPackageURLException { final JSONObject qualifiers = testDefinition.optJSONObject("qualifiers"); final String subpath = testDefinition.optString("subpath", null); - TreeMap map = null; + Map map = null; Map hashMap = null; if (qualifiers != null) { map = qualifiers.toMap().entrySet().stream().collect( @@ -174,9 +171,9 @@ public void testConstructorParameters() throws MalformedPackageURLException { Assert.assertEquals(name, purl.getName()); Assert.assertEquals(version, purl.getVersion()); Assert.assertEquals(subpath, purl.getSubpath()); + Assert.assertNotNull(purl.getQualifiers()); + Assert.assertEquals("qualifier count", qualifiers != null ? qualifiers.length() : 0, purl.getQualifiers().size()); if (qualifiers != null) { - Assert.assertNotNull(purl.getQualifiers()); - Assert.assertEquals(qualifiers.length(), purl.getQualifiers().size()); qualifiers.keySet().forEach(key -> { String value = qualifiers.getString(key); Assert.assertTrue(purl.getQualifiers().containsKey(key)); @@ -299,9 +296,9 @@ public void testConstructorDuplicateQualifiersMixedCase() throws MalformedPackag @Test public void testConstructorWithUppercaseKey() throws MalformedPackageURLException { PackageURL purl = new PackageURL("pkg://generic/name?KEY=one"); - Assert.assertNotNull(purl.getQualifiers()); + Assert.assertEquals("qualifier count", 1, purl.getQualifiers().size()); Assert.assertEquals("one", purl.getQualifiers().get("key")); - TreeMap qualifiers = new TreeMap<>(); + Map qualifiers = new TreeMap<>(); qualifiers.put("key", "one"); PackageURL purl2 = new PackageURL("generic", null, "name", null, qualifiers, null); Assert.assertEquals(purl, purl2); @@ -310,8 +307,8 @@ public void testConstructorWithUppercaseKey() throws MalformedPackageURLExceptio @Test public void testConstructorWithEmptyKey() throws MalformedPackageURLException { PackageURL purl = new PackageURL("pkg://generic/name?KEY"); - Assert.assertNull(purl.getQualifiers()); - TreeMap qualifiers = new TreeMap<>(); + Assert.assertEquals("qualifier count", 0, purl.getQualifiers().size()); + Map qualifiers = new TreeMap<>(); qualifiers.put("KEY", null); PackageURL purl2 = new PackageURL("generic", null, "name", null, qualifiers, null); Assert.assertEquals(purl, purl2);