Problem/Motivation

This is a split from #1505080: [META] Content Negotiation for Accept Headers where a better content negotiation for Symfony is under discussion. It is not sure if/when Symfony will this improved content negotiation logic, and in the meantime it was suggested for Drupal to implement its own a stop gap measure.

Proposed resolution

Use the mimeparse library.

Remaining tasks

finish the WIP patch.

User interface changes

none

API changes

additional class and method Mimeparse::getWeightedAcceptableContentTypes()

CommentFileSizeAuthor
#11 1857138_11_dedicated_conneg.patch34.94 KBwamilton
#6 1857138_6_dedicated_conneg.patch36.51 KBscor
#6 interdiff.txt660 bytesscor
#1 1857138_1_dedicated_conneg.patch29.03 KBscor
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

scor’s picture

scor CreditAttribution: scor commented
Status: Active » Needs review
Issue tags: +WSCCI
FileSize
1857138_1_dedicated_conneg.patch29.03 KB

This patch include the mimeparse library as well as a new Mimeparse subsystem in core/lib/Drupal/Core/Mimeparse.php where I've added some logic to adapt mimeparse to our needs (in particular preserving the quality factors along with the MIME types). The default conneg and the MIME type partial matcher have been adapted to use this new library.

setting to needs review to see what the testbot says.

Status: Needs review » Needs work

The last submitted patch, 1857138_1_dedicated_conneg.patch, failed testing.

Crell’s picture

Crell CreditAttribution: Crell commented
Status: Needs work » Needs review 
+++ b/core/lib/Drupal/Core/Mimeparse.php
@@ -0,0 +1,43 @@
+      $normalized = self::parseAndNormalizeMediaRange($r);

This should be static::, not self::. self:: probably should never be used anymore.

Crell’s picture

Crell CreditAttribution: Crell commented
Status: Needs review » Needs work
scor’s picture

scor CreditAttribution: scor commented
Title: Improve Content Negotiation with dedicated library » Improve Content Negotiation with dedicated library (mimeparse)
scor’s picture

scor CreditAttribution: scor commented
Status: Needs work » Needs review
FileSize
interdiff.txt660 bytes
1857138_6_dedicated_conneg.patch36.51 KB

addressed crell's comment in #3.

The last submitted patch, 1857138_6_dedicated_conneg.patch, failed testing.

wamilton’s picture

wamilton CreditAttribution: wamilton commented
Issue summary: View changes
Status: Needs work » Needs review

Poking testbot for a new list of broken stuff.

wamilton’s picture

wamilton CreditAttribution: wamilton commented

6: 1857138_6_dedicated_conneg.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 6: 1857138_6_dedicated_conneg.patch, failed testing.

wamilton’s picture

wamilton CreditAttribution: wamilton commented
Status: Needs work » Needs review
FileSize
1857138_11_dedicated_conneg.patch34.94 KB

A reroll with composer cruft... fingers crossed.

Crell’s picture

Crell CreditAttribution: Crell commented
Related issues: +#2026431: Make ContentNegotiation a "internal" service, used only by the router, so that core or contrib can implement real negotiation

Let's get #2026431: Make ContentNegotiation a "internal" service, used only by the router, so that core or contrib can implement real negotiation in first, as that gives us more leeway here.

Status: Needs review » Needs work

The last submitted patch, 11: 1857138_11_dedicated_conneg.patch, failed testing.

Crell’s picture

Crell CreditAttribution: Crell commented
Status: Needs work » Closed (duplicate)

We're going with a stack approach: #2331919: Implement StackNegotiation