This is an automated email from the ASF dual-hosted git repository. joerghoh pushed a commit to branch SLING-13416 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-api.git
commit 97e8fcd2718bacae75c53b8b7ef8ecf75a88a663 Author: Joerg Hoh <[email protected]> AuthorDate: Mon Nov 9 18:11:52 2026 +0100 SLING-13416 improve handling of the accept request header --- .../api/request/header/JakartaMediaRangeList.java | 99 ++++++++++++++++++---- .../sling/api/request/header/MediaRangeList.java | 99 ++++++++++++++++++---- .../sling/api/request/header/package-info.java | 2 +- .../request/header/JakartaMediaRangeListTest.java | 95 +++++++++++++++++++++ .../api/request/header/MediaRangeListTest.java | 92 ++++++++++++++++++++ 5 files changed, 354 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/apache/sling/api/request/header/JakartaMediaRangeList.java b/src/main/java/org/apache/sling/api/request/header/JakartaMediaRangeList.java index bc7bf5d..e457ed6 100644 --- a/src/main/java/org/apache/sling/api/request/header/JakartaMediaRangeList.java +++ b/src/main/java/org/apache/sling/api/request/header/JakartaMediaRangeList.java @@ -19,12 +19,17 @@ package org.apache.sling.api.request.header; import java.util.HashMap; -import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.TreeSet; import jakarta.servlet.http.HttpServletRequest; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.osgi.annotation.versioning.ConsumerType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -123,20 +128,36 @@ public class JakartaMediaRangeList extends TreeSet<JakartaMediaRangeList.MediaRa /** * Given a list of media types, returns the one is preferred by this <code>MediaRangeList</code>. + * Wildcards are matched against the offered types. The most specific matching range, + * including media parameters before {@code q}, determines each type's quality. + * Types with quality zero are not acceptable. Accept extensions after {@code q} do not + * constrain matching. Equal-quality types follow the ordering of this list; when the + * same range matches multiple offered types, their iteration order breaks the tie. * @param mediaRanges An array of possible {@link JakartaMediaRangeList.MediaRange}s * @return One of the <code>mediaRanges</code> that this <code>MediaRangeList</code> prefers; * or <code>null</code> if this <code>MediaRangeList</code> does not contain any of the <code>mediaRanges</code> * @throws NullPointerException if <code>mediaRanges</code> is <code>null</code> or contains a <code>null</code> value */ - public MediaRange prefer(Set<MediaRange> mediaRanges) { - for (MediaRange range : this) { - for (MediaRange mediaType : mediaRanges) { - if (range.equals(mediaType)) { - return mediaType; + public @Nullable MediaRange prefer(@NotNull Set<MediaRange> mediaRanges) { + MediaRange preferred = null; + MediaRange preferredRange = null; + for (MediaRange mediaType : mediaRanges) { + Objects.requireNonNull(mediaType); + MediaRange matchingRange = null; + for (MediaRange range : this) { + if (range.matches(mediaType) + && (matchingRange == null || range.compareSpecificity(matchingRange) > 0)) { + matchingRange = range; } } + if (matchingRange != null + && matchingRange.getQ() > 0 + && (preferredRange == null || matchingRange.compareTo(preferredRange) < 0)) { + preferred = mediaType; + preferredRange = matchingRange; + } } - return null; + return preferred; } /** @@ -144,11 +165,12 @@ public class JakartaMediaRangeList extends TreeSet<JakartaMediaRangeList.MediaRa * @param mediaRanges String representations of <code>MediaRange</code>s. The strings must be * on the form required by {@link MediaRange#MediaRange(String)} * @see #prefer(java.util.Set) + * @implNote Offered order breaks ties when multiple types match the same range. * @return the <code>toString()</code> representation of the preferred <code>MediaRange</code>, or <code>null</code> * if this <code>MediaRangeList</code> does not contain any of the <code>mediaRanges</code> */ - public String prefer(String... mediaRanges) { - Set<MediaRange> ranges = new HashSet<MediaRange>(); + public @Nullable String prefer(@NotNull String... mediaRanges) { + Set<MediaRange> ranges = new LinkedHashSet<MediaRange>(); for (String mediaRange : mediaRanges) { ranges.add(new MediaRange(mediaRange)); } @@ -197,18 +219,24 @@ public class JakartaMediaRangeList extends TreeSet<JakartaMediaRangeList.MediaRa String[] parts = exp.split(";"); this.setType(parts[0].trim()); if (parts.length > 1) { - this.parameters = new HashMap<String, String>(parts.length - 1); + this.parameters = new LinkedHashMap<String, String>(parts.length - 1); } for (int i = 1, partsLength = parts.length; i < partsLength; i++) { - String parameter = parts[i]; - String[] keyValue = parameter.split("="); - if (keyValue[0].equals("q")) { - this.q = Double.parseDouble(keyValue[1]); - if (this.q < 0 || this.q > 1) { + String[] keyValue = parts[i].split("=", 2); + if (keyValue.length != 2 + || keyValue[0].trim().isEmpty() + || keyValue[1].trim().isEmpty()) { + throw new IllegalArgumentException("Invalid media range parameter: " + exp); + } + String name = keyValue[0].trim().toLowerCase(Locale.ROOT); + String value = keyValue[1].trim(); + if (name.equals("q")) { + this.q = Double.parseDouble(value); + if (!Double.isFinite(this.q) || this.q < 0 || this.q > 1) { throw new IllegalArgumentException("Quality factor out of bounds: " + exp); } } - this.parameters.put(keyValue[0], keyValue[1]); + this.parameters.put(name, value); } } @@ -281,6 +309,45 @@ public class JakartaMediaRangeList extends TreeSet<JakartaMediaRangeList.MediaRa return parameters != null ? parameters : new HashMap<String, String>(0); } + private boolean matches(MediaRange mediaType) { + if ((!WILDCARD.equals(supertype) && !supertype.equalsIgnoreCase(mediaType.supertype)) + || (!WILDCARD.equals(subtype) && !subtype.equalsIgnoreCase(mediaType.subtype))) { + return false; + } + if (parameters != null) { + for (Map.Entry<String, String> parameter : parameters.entrySet()) { + if ("q".equals(parameter.getKey())) { + break; + } + if (!parameter.getValue().equals(mediaType.getParameter(parameter.getKey()))) { + return false; + } + } + } + return true; + } + + private int compareSpecificity(MediaRange other) { + int typeSpecificity = matchesAll() ? 0 : WILDCARD.equals(subtype) ? 1 : 2; + int otherTypeSpecificity = other.matchesAll() ? 0 : WILDCARD.equals(other.subtype) ? 1 : 2; + return typeSpecificity != otherTypeSpecificity + ? Integer.compare(typeSpecificity, otherTypeSpecificity) + : Integer.compare(mediaParameterCount(), other.mediaParameterCount()); + } + + private int mediaParameterCount() { + int count = 0; + if (parameters != null) { + for (String name : parameters.keySet()) { + if ("q".equals(name)) { + break; + } + count++; + } + } + return count; + } + /* -- Comparable implementation -- */ public int compareTo(MediaRange o) { double diff = this.q - o.getQ(); diff --git a/src/main/java/org/apache/sling/api/request/header/MediaRangeList.java b/src/main/java/org/apache/sling/api/request/header/MediaRangeList.java index ea90646..bed69e9 100644 --- a/src/main/java/org/apache/sling/api/request/header/MediaRangeList.java +++ b/src/main/java/org/apache/sling/api/request/header/MediaRangeList.java @@ -21,11 +21,16 @@ package org.apache.sling.api.request.header; import javax.servlet.http.HttpServletRequest; import java.util.HashMap; -import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.TreeSet; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.osgi.annotation.versioning.ConsumerType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -125,20 +130,36 @@ public class MediaRangeList extends TreeSet<MediaRangeList.MediaRange> { /** * Given a list of media types, returns the one is preferred by this <code>MediaRangeList</code>. + * Wildcards are matched against the offered types. The most specific matching range, + * including media parameters before {@code q}, determines each type's quality. + * Types with quality zero are not acceptable. Accept extensions after {@code q} do not + * constrain matching. Equal-quality types follow the ordering of this list; when the + * same range matches multiple offered types, their iteration order breaks the tie. * @param mediaRanges An array of possible {@link MediaRangeList.MediaRange}s * @return One of the <code>mediaRanges</code> that this <code>MediaRangeList</code> prefers; * or <code>null</code> if this <code>MediaRangeList</code> does not contain any of the <code>mediaRanges</code> * @throws NullPointerException if <code>mediaRanges</code> is <code>null</code> or contains a <code>null</code> value */ - public MediaRange prefer(Set<MediaRange> mediaRanges) { - for (MediaRange range : this) { - for (MediaRange mediaType : mediaRanges) { - if (range.equals(mediaType)) { - return mediaType; + public @Nullable MediaRange prefer(@NotNull Set<MediaRange> mediaRanges) { + MediaRange preferred = null; + MediaRange preferredRange = null; + for (MediaRange mediaType : mediaRanges) { + Objects.requireNonNull(mediaType); + MediaRange matchingRange = null; + for (MediaRange range : this) { + if (range.matches(mediaType) + && (matchingRange == null || range.compareSpecificity(matchingRange) > 0)) { + matchingRange = range; } } + if (matchingRange != null + && matchingRange.getQ() > 0 + && (preferredRange == null || matchingRange.compareTo(preferredRange) < 0)) { + preferred = mediaType; + preferredRange = matchingRange; + } } - return null; + return preferred; } /** @@ -146,11 +167,12 @@ public class MediaRangeList extends TreeSet<MediaRangeList.MediaRange> { * @param mediaRanges String representations of <code>MediaRange</code>s. The strings must be * on the form required by {@link MediaRange#MediaRange(String)} * @see #prefer(java.util.Set) + * @implNote Offered order breaks ties when multiple types match the same range. * @return the <code>toString()</code> representation of the preferred <code>MediaRange</code>, or <code>null</code> * if this <code>MediaRangeList</code> does not contain any of the <code>mediaRanges</code> */ - public String prefer(String... mediaRanges) { - Set<MediaRange> ranges = new HashSet<MediaRange>(); + public @Nullable String prefer(@NotNull String... mediaRanges) { + Set<MediaRange> ranges = new LinkedHashSet<MediaRange>(); for (String mediaRange : mediaRanges) { ranges.add(new MediaRange(mediaRange)); } @@ -199,18 +221,24 @@ public class MediaRangeList extends TreeSet<MediaRangeList.MediaRange> { String[] parts = exp.split(";"); this.setType(parts[0].trim()); if (parts.length > 1) { - this.parameters = new HashMap<String, String>(parts.length - 1); + this.parameters = new LinkedHashMap<String, String>(parts.length - 1); } for (int i = 1, partsLength = parts.length; i < partsLength; i++) { - String parameter = parts[i]; - String[] keyValue = parameter.split("="); - if (keyValue[0].equals("q")) { - this.q = Double.parseDouble(keyValue[1]); - if (this.q < 0 || this.q > 1) { + String[] keyValue = parts[i].split("=", 2); + if (keyValue.length != 2 + || keyValue[0].trim().isEmpty() + || keyValue[1].trim().isEmpty()) { + throw new IllegalArgumentException("Invalid media range parameter: " + exp); + } + String name = keyValue[0].trim().toLowerCase(Locale.ROOT); + String value = keyValue[1].trim(); + if (name.equals("q")) { + this.q = Double.parseDouble(value); + if (!Double.isFinite(this.q) || this.q < 0 || this.q > 1) { throw new IllegalArgumentException("Quality factor out of bounds: " + exp); } } - this.parameters.put(keyValue[0], keyValue[1]); + this.parameters.put(name, value); } } @@ -283,6 +311,45 @@ public class MediaRangeList extends TreeSet<MediaRangeList.MediaRange> { return parameters != null ? parameters : new HashMap<String, String>(0); } + private boolean matches(MediaRange mediaType) { + if ((!WILDCARD.equals(supertype) && !supertype.equalsIgnoreCase(mediaType.supertype)) + || (!WILDCARD.equals(subtype) && !subtype.equalsIgnoreCase(mediaType.subtype))) { + return false; + } + if (parameters != null) { + for (Map.Entry<String, String> parameter : parameters.entrySet()) { + if ("q".equals(parameter.getKey())) { + break; + } + if (!parameter.getValue().equals(mediaType.getParameter(parameter.getKey()))) { + return false; + } + } + } + return true; + } + + private int compareSpecificity(MediaRange other) { + int typeSpecificity = matchesAll() ? 0 : WILDCARD.equals(subtype) ? 1 : 2; + int otherTypeSpecificity = other.matchesAll() ? 0 : WILDCARD.equals(other.subtype) ? 1 : 2; + return typeSpecificity != otherTypeSpecificity + ? Integer.compare(typeSpecificity, otherTypeSpecificity) + : Integer.compare(mediaParameterCount(), other.mediaParameterCount()); + } + + private int mediaParameterCount() { + int count = 0; + if (parameters != null) { + for (String name : parameters.keySet()) { + if ("q".equals(name)) { + break; + } + count++; + } + } + return count; + } + /* -- Comparable implementation -- */ public int compareTo(MediaRange o) { double diff = this.q - o.getQ(); diff --git a/src/main/java/org/apache/sling/api/request/header/package-info.java b/src/main/java/org/apache/sling/api/request/header/package-info.java index a7085e2..24cb520 100644 --- a/src/main/java/org/apache/sling/api/request/header/package-info.java +++ b/src/main/java/org/apache/sling/api/request/header/package-info.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -@Version("1.1.0") +@Version("1.1.1") package org.apache.sling.api.request.header; import org.osgi.annotation.versioning.Version; diff --git a/src/test/java/org/apache/sling/api/request/header/JakartaMediaRangeListTest.java b/src/test/java/org/apache/sling/api/request/header/JakartaMediaRangeListTest.java index c30034f..b94229f 100644 --- a/src/test/java/org/apache/sling/api/request/header/JakartaMediaRangeListTest.java +++ b/src/test/java/org/apache/sling/api/request/header/JakartaMediaRangeListTest.java @@ -18,6 +18,9 @@ */ package org.apache.sling.api.request.header; +import java.util.LinkedHashSet; +import java.util.Set; + import junit.framework.TestCase; import org.apache.sling.api.SlingJakartaHttpServletRequest; import org.mockito.Mockito; @@ -46,6 +49,98 @@ public class JakartaMediaRangeListTest extends TestCase { assertEquals("application/json", rangeList.prefer("text/html", "application/json")); } + /** + * Matches wildcard ranges and treats missing or empty Accept headers as accepting all types. + */ + public void testPreferWildcardsAndMissingHeader() { + for (String accept : new String[] {null, "", "*/*", "text/*"}) { + assertEquals( + accept, "text/html", new JakartaMediaRangeList(accept).prefer("text/html", "application/json")); + } + assertEquals( + "application/json", new JakartaMediaRangeList("application/*").prefer("text/html", "application/json")); + } + + /** + * Rejects unsupported types and q=0 ranges, including exclusions that override broader wildcards. + */ + public void testPreferExcludesUnacceptableTypes() { + for (String accept : new String[] { + "text/plain", + "*/*;q=0", + "text/html;q=0", + "text/html;q=0,application/json;q=0,*/*", + "text/*;q=0,application/*;q=0,*/*" + }) { + assertNull(accept, new JakartaMediaRangeList(accept).prefer("text/html", "application/json")); + } + assertEquals( + "application/json", + new JakartaMediaRangeList("text/html;q=0,*/*").prefer("text/html", "application/json")); + } + + /** + * Uses the most specific matching range's quality rather than the quality of a broader wildcard. + */ + public void testPreferUsesMostSpecificQuality() { + assertEquals( + "application/json", + new JakartaMediaRangeList("text/html;q=0.1,*/*;q=0.9").prefer("text/html", "application/json")); + assertEquals( + "text/html", + new JakartaMediaRangeList("text/html;q=0.2,text/*;q=0.1,application/json;q=0.15") + .prefer("text/html", "application/json")); + } + + /** + * Requires media parameters to match and gives parameter-specific ranges precedence. + */ + public void testPreferMatchesMediaParameters() { + JakartaMediaRangeList ranges = new JakartaMediaRangeList("text/html;level=1;q=0,text/html;q=0.5,*/*"); + assertNull(ranges.prefer("text/html;level=1")); + assertEquals("text/html;level=2", ranges.prefer("text/html;level=2")); + assertEquals("text/html", ranges.prefer("text/html")); + assertNull(new JakartaMediaRangeList("text/html;level=1").prefer("text/html;level=2")); + } + + /** + * Ignores accept extensions after q when matching ranges and determining specificity. + */ + public void testPreferIgnoresAcceptExtensions() { + assertEquals("text/html", new JakartaMediaRangeList("text/html;q=1;extension=value").prefer("text/html")); + assertNull(new JakartaMediaRangeList("text/html;q=0.5;extension=value,text/html;level=1;q=0") + .prefer("text/html;level=1", "application/json")); + } + + /** + * Accepts whitespace around quality parameters and rejects invalid or non-finite quality values. + */ + public void testPreferHandlesQualityWhitespaceAndInvalidValues() { + for (String accept : + new String[] {"text/html; q=0", "text/html;q=NaN", "text/html;q=Infinity", "text/html;q=invalid"}) { + assertNull(accept, new JakartaMediaRangeList(accept).prefer("text/html")); + } + assertEquals( + "application/json", + new JakartaMediaRangeList("Text/HTML; q=0.2, application/json; q=0.8") + .prefer("text/html", "application/json")); + } + + /** + * Preserves explicit-type tie-breaking and returns the first offered instance for a shared wildcard match. + */ + public void testPreferPreservesExplicitTieAndOfferedInstance() { + assertEquals( + "application/json", + new JakartaMediaRangeList("text/html,application/json").prefer("text/html", "application/json")); + JakartaMediaRangeList ranges = new JakartaMediaRangeList("text/*"); + JakartaMediaRangeList.MediaRange html = ranges.new MediaRange("text/html"); + Set<JakartaMediaRangeList.MediaRange> offered = new LinkedHashSet<>(); + offered.add(html); + offered.add(ranges.new MediaRange("text/plain")); + assertSame(html, ranges.prefer(offered)); + } + public void testHttpEquivParam() { SlingJakartaHttpServletRequest req = Mockito.mock(SlingJakartaHttpServletRequest.class); Mockito.when(req.getHeader(JakartaMediaRangeList.HEADER_ACCEPT)).thenReturn("text/plain"); diff --git a/src/test/java/org/apache/sling/api/request/header/MediaRangeListTest.java b/src/test/java/org/apache/sling/api/request/header/MediaRangeListTest.java index 43b67ec..17ca8a9 100644 --- a/src/test/java/org/apache/sling/api/request/header/MediaRangeListTest.java +++ b/src/test/java/org/apache/sling/api/request/header/MediaRangeListTest.java @@ -18,6 +18,9 @@ */ package org.apache.sling.api.request.header; +import java.util.LinkedHashSet; +import java.util.Set; + import junit.framework.TestCase; import org.apache.sling.api.SlingHttpServletRequest; import org.mockito.Mockito; @@ -47,6 +50,95 @@ public class MediaRangeListTest extends TestCase { assertEquals("application/json", rangeList.prefer("text/html", "application/json")); } + /** + * Matches wildcard ranges and treats missing or empty Accept headers as accepting all types. + */ + public void testPreferWildcardsAndMissingHeader() { + for (String accept : new String[] {null, "", "*/*", "text/*"}) { + assertEquals(accept, "text/html", new MediaRangeList(accept).prefer("text/html", "application/json")); + } + assertEquals("application/json", new MediaRangeList("application/*").prefer("text/html", "application/json")); + } + + /** + * Rejects unsupported types and q=0 ranges, including exclusions that override broader wildcards. + */ + public void testPreferExcludesUnacceptableTypes() { + for (String accept : new String[] { + "text/plain", + "*/*;q=0", + "text/html;q=0", + "text/html;q=0,application/json;q=0,*/*", + "text/*;q=0,application/*;q=0,*/*" + }) { + assertNull(accept, new MediaRangeList(accept).prefer("text/html", "application/json")); + } + assertEquals( + "application/json", new MediaRangeList("text/html;q=0,*/*").prefer("text/html", "application/json")); + } + + /** + * Uses the most specific matching range's quality rather than the quality of a broader wildcard. + */ + public void testPreferUsesMostSpecificQuality() { + assertEquals( + "application/json", + new MediaRangeList("text/html;q=0.1,*/*;q=0.9").prefer("text/html", "application/json")); + assertEquals( + "text/html", + new MediaRangeList("text/html;q=0.2,text/*;q=0.1,application/json;q=0.15") + .prefer("text/html", "application/json")); + } + + /** + * Requires media parameters to match and gives parameter-specific ranges precedence. + */ + public void testPreferMatchesMediaParameters() { + MediaRangeList ranges = new MediaRangeList("text/html;level=1;q=0,text/html;q=0.5,*/*"); + assertNull(ranges.prefer("text/html;level=1")); + assertEquals("text/html;level=2", ranges.prefer("text/html;level=2")); + assertEquals("text/html", ranges.prefer("text/html")); + assertNull(new MediaRangeList("text/html;level=1").prefer("text/html;level=2")); + } + + /** + * Ignores accept extensions after q when matching ranges and determining specificity. + */ + public void testPreferIgnoresAcceptExtensions() { + assertEquals("text/html", new MediaRangeList("text/html;q=1;extension=value").prefer("text/html")); + assertNull(new MediaRangeList("text/html;q=0.5;extension=value,text/html;level=1;q=0") + .prefer("text/html;level=1", "application/json")); + } + + /** + * Accepts whitespace around quality parameters and rejects invalid or non-finite quality values. + */ + public void testPreferHandlesQualityWhitespaceAndInvalidValues() { + for (String accept : + new String[] {"text/html; q=0", "text/html;q=NaN", "text/html;q=Infinity", "text/html;q=invalid"}) { + assertNull(accept, new MediaRangeList(accept).prefer("text/html")); + } + assertEquals( + "application/json", + new MediaRangeList("Text/HTML; q=0.2, application/json; q=0.8") + .prefer("text/html", "application/json")); + } + + /** + * Preserves explicit-type tie-breaking and returns the first offered instance for a shared wildcard match. + */ + public void testPreferPreservesExplicitTieAndOfferedInstance() { + assertEquals( + "application/json", + new MediaRangeList("text/html,application/json").prefer("text/html", "application/json")); + MediaRangeList ranges = new MediaRangeList("text/*"); + MediaRangeList.MediaRange html = ranges.new MediaRange("text/html"); + Set<MediaRangeList.MediaRange> offered = new LinkedHashSet<>(); + offered.add(html); + offered.add(ranges.new MediaRange("text/plain")); + assertSame(html, ranges.prefer(offered)); + } + public void testHttpEquivParam() { SlingHttpServletRequest req = Mockito.mock(SlingHttpServletRequest.class); Mockito.when(req.getHeader(MediaRangeList.HEADER_ACCEPT)).thenReturn("text/plain");
