Skip to content
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.

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

MediaType.sortBySpecificityAndQuality throws java.lang.IllegalArgumentException: Comparison method violates its general contract #27488

Closed
samabcde opened this issue Sep 28, 2021 · 3 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@samabcde
Copy link

samabcde commented Sep 28, 2021

Affects: spring-web:5.3.9
Original Problem from stackoverflow

Implementation of QUALITY_VALUE_COMPARATOR seems violate general contract of comparator.

The issue can be reproduced by below program.
OS: Window 10
JDK: openjdk version "16" 2021-03-16

import org.springframework.http.MediaType;

import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Map;

public class TestMediaTypeSort {
    public static void main(String[] args) {
        demonstrateViolation();
        reproducibleExample();
    }

    private static void reproducibleExample() {
        Map<String, String> mapSize1 = Map.of("a", "b");
        Map<String, String> mapSize2 = Map.of("a", "b", "c", "d");
        Map<String, String> mapSize3 = Map.of("a", "b", "c", "d", "e", "f");
        List<MediaType> mediaTypeList = new ArrayList<>();
        for (int i = 0; i < 20; i++) {
            mediaTypeList.add(new MediaType("c", "a", mapSize1));
            mediaTypeList.add(new MediaType("c", "a", mapSize2));
            mediaTypeList.add(new MediaType("c", "a", mapSize3));
            mediaTypeList.add(new MediaType("b", "a", mapSize1));
            mediaTypeList.add(new MediaType("b", "a", mapSize2));
            mediaTypeList.add(new MediaType("b", "a", mapSize3));
            mediaTypeList.add(new MediaType("b", "a", mapSize2));
        }
        MediaType.sortBySpecificityAndQuality(mediaTypeList);
    }

    private static void demonstrateViolation() {
        Map<String, String> mapSize1 = Map.of("a", "b");
        Map<String, String> mapSize2 = Map.of("a", "b", "c", "d");
        Map<String, String> mapSize3 = Map.of("a", "b", "c", "d", "e", "f");
        MediaType a = new MediaType("c", "a", mapSize1);
        MediaType b = new MediaType("b", "a", mapSize2);
        MediaType c = new MediaType("b", "a", mapSize3);
        System.out.println("Compare a with b:" + QUALITY_VALUE_COMPARATOR.compare(a, b));
        System.out.println("Compare a with c:" + QUALITY_VALUE_COMPARATOR.compare(a, c));
        System.out.println("Compare b with c:" + QUALITY_VALUE_COMPARATOR.compare(b, c));
    }

    // Copied from MediaType.java
    public static final Comparator<MediaType> QUALITY_VALUE_COMPARATOR = (mediaType1, mediaType2) -> {
        double quality1 = mediaType1.getQualityValue();
        double quality2 = mediaType2.getQualityValue();
        int qualityComparison = Double.compare(quality2, quality1);
        if (qualityComparison != 0) {
            return qualityComparison;  // audio/*;q=0.7 < audio/*;q=0.3
        } else if (mediaType1.isWildcardType() && !mediaType2.isWildcardType()) {  // */* < audio/*
            return 1;
        } else if (mediaType2.isWildcardType() && !mediaType1.isWildcardType()) {  // audio/* > */*
            return -1;
        } else if (!mediaType1.getType().equals(mediaType2.getType())) {  // audio/basic == text/html
            return 0;
        } else {  // mediaType1.getType().equals(mediaType2.getType())
            if (mediaType1.isWildcardSubtype() && !mediaType2.isWildcardSubtype()) {  // audio/* < audio/basic
                return 1;
            } else if (mediaType2.isWildcardSubtype() && !mediaType1.isWildcardSubtype()) {  // audio/basic > audio/*
                return -1;
            } else if (!mediaType1.getSubtype().equals(mediaType2.getSubtype())) {  // audio/basic == audio/wave
                return 0;
            } else {
                int paramsSize1 = mediaType1.getParameters().size();
                int paramsSize2 = mediaType2.getParameters().size();
                return Integer.compare(paramsSize2, paramsSize1);  // audio/basic;level=1 < audio/basic
            }
        }
    };
}


@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 28, 2021
@poutsma poutsma self-assigned this Sep 29, 2021
@poutsma poutsma added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 30, 2021
@poutsma poutsma added this to the 5.3.11 milestone Sep 30, 2021
poutsma added a commit that referenced this issue Sep 30, 2021
poutsma added a commit that referenced this issue Oct 19, 2021
The fix made for gh-27488 resulted in a change of the default order
of codecs. This commit reverts these changes, so that the previous
order is restored.

Closes gh-27573
@poutsma
Copy link
Contributor

poutsma commented Oct 19, 2021

Unfortunately, the changes made to fix this issue had undesirable consequences (#27573). Therefore, we have reverted these change for the 5.3.x branch, and will revisiting Media type ordering in 6.0 (#27580).

poutsma added a commit to poutsma/spring-framework that referenced this issue Oct 20, 2021
The fix made for spring-projectsgh-27488 resulted in a change of the default order
of codecs. This commit reverts these changes, so that the previous
order is restored.

Closes spring-projectsgh-27573
poutsma added a commit to poutsma/spring-framework that referenced this issue Oct 20, 2021
This commit makes several changes to MimeType and MediaType
related to the topic of specificity.

This commit deprecates the MimeType and MediaType Comparators.
Comparators require a transitive relationship, and the desired order for
these types is not transitive (see spring-projects#27488).

Instead, this commit introduces two new MimeType methods: isMoreSpecific
and isLessSpecific, both of which return booleans. MediaType overrides
these methods to include the quality factor (q) in the comparison.

All MediaType sorting methods have been deprecated in favor of
MimeTypeUtils::sortBySpecificity.  This sorting method now uses
MimeType::isLessSpecific in combination a bubble sort algorithm (which
does not require a transitive compare function).

Closes spring-projectsgh-27580
poutsma added a commit to poutsma/spring-framework that referenced this issue Oct 20, 2021
This commit makes several changes to MimeType and MediaType
related to the topic of specificity.

This commit deprecates the MimeType and MediaType Comparators.
Comparators require a transitive relationship, and the desired order for
these types is not transitive (see spring-projects#27488).

Instead, this commit introduces two new MimeType methods: isMoreSpecific
and isLessSpecific, both of which return booleans. MediaType overrides
these methods to include the quality factor (q) in the comparison.

All MediaType sorting methods have been deprecated in favor of
MimeTypeUtils::sortBySpecificity.  This sorting method now uses
MimeType::isLessSpecific in combination a bubble sort algorithm (which
does not require a transitive compare function).

Closes spring-projectsgh-27580
poutsma added a commit to poutsma/spring-framework that referenced this issue Oct 20, 2021
This commit makes several changes to MimeType and MediaType
related to the topic of specificity.

This commit deprecates the MimeType and MediaType Comparators.
Comparators require a transitive relationship, and the desired order for
these types is not transitive (see spring-projects#27488).

Instead, this commit introduces two new MimeType methods: isMoreSpecific
and isLessSpecific, both of which return booleans. MediaType overrides
these methods to include the quality factor (q) in the comparison.

All MediaType sorting methods have been deprecated in favor of
MimeTypeUtils::sortBySpecificity.  This sorting method now uses
MimeType::isLessSpecific in combination a bubble sort algorithm (which
does not require a transitive compare function).

Closes spring-projectsgh-27580
poutsma added a commit to poutsma/spring-framework that referenced this issue Oct 20, 2021
This commit makes several changes to MimeType and MediaType
related to the topic of specificity.

This commit deprecates the MimeType and MediaType Comparators.
Comparators require a transitive relationship, and the desired order for
these types is not transitive (see spring-projects#27488).

Instead, this commit introduces two new MimeType methods: isMoreSpecific
and isLessSpecific, both of which return booleans. MediaType overrides
these methods to include the quality factor (q) in the comparison.

All MediaType sorting methods have been deprecated in favor of
MimeTypeUtils::sortBySpecificity.  This sorting method now uses
MimeType::isLessSpecific in combination a bubble sort algorithm (which
does not require a transitive compare function).

Closes spring-projectsgh-27580
poutsma added a commit that referenced this issue Nov 23, 2021
This commit makes several changes to MimeType and MediaType
related to the topic of specificity.

This commit deprecates the MimeType and MediaType Comparators.
Comparators require a transitive relationship, and the desired order for
these types is not transitive (see #27488).

Instead, this commit introduces two new MimeType methods: isMoreSpecific
and isLessSpecific, both of which return booleans. MediaType overrides
these methods to include the quality factor (q) in the comparison.

All MediaType sorting methods have been deprecated in favor of
MimeTypeUtils::sortBySpecificity.  This sorting method now uses
MimeType::isLessSpecific in combination a bubble sort algorithm (which
does not require a transitive compare function).

Closes gh-27580
@Ryokki
Copy link

Ryokki commented Jun 16, 2023

I met the same question in spring 5.3.13, what can I do?

@poutsma
Copy link
Contributor

poutsma commented Jun 19, 2023

I met the same question in spring 5.3.13, what can I do?

Upgrade to 6.0. As you can read above, we could not fix this without breaking backward compatibility, so a 5.3.x fix is not forthcoming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants