Laravel-json-api: multipart/form-data is not supported?

[upstream-nsk]

How to enable multipart support?

Request:
Content-Type: multipart/form-data; boundary=----WebKitFormBoundary0xxx
Response error №415 Invalid Content-Type Header

[lindyhopchris]

@upstream-nsk there's nothing in the spec about multipart support, however you might be able to get it working using the content negotiation config. That's described here:
https://laravel-json-api.readthedocs.io/en/latest/basics/api/#content-negotiation

Let me know how you get on!

[upstream-nsk]

Perfect! Look carefully ! This is a pakeage for laravel with JSON API. Please check the documentation of JSON API

I know, I have the task to make upload attachments through the API and I do not want to read the content on the client side, i need send attachments with multipart/form-data.

[upstream-nsk]

@upstream-nsk there's nothing in the spec about multipart support, however you might be able to get it working using the content negotiation config. That's described here:
https://laravel-json-api.readthedocs.io/en/latest/basics/api/#content-negotiation

Let me know how you get on!

It did not help, I tried. I think a problem in the "boundary=----WebKitFormBoundary..." and the headers in the library are incorrectly check it.

[lindyhopchris]

@upstream-nsk I'll be honest, I'm not entirely sure how multipart requests look on the Laravel side as I haven't used them before. Plus also the content negotiation stuff is being refactored as the current implementation is very limited in what you can do with it.

Getting multipart support working is definitely something that would be good to work on though but I'll need you to point me in the right direction as to what isn't working.

When you say:

the headers in the library are incorrectly check it

What do you mean? Do you know where the library is rejecting it? Plus it would really help if you could post the list of headers from the Laravel request object.

[lindyhopchris]

I've now added support for different media types, and this is documented here:
https://github.com/cloudcreativity/laravel-json-api/blob/develop/docs/features/media-types.md

Will be released in 1.0.0-rc.1

[tsterker]

@lindyhopchris

There still is a problem with mediatypes if there are additional (dynamic) parameters specified, such as boundary=... for multipart/form-data. I currently have no good idea on how to solve this properly, but wanted to at least share my insights:

TL;DR

neomerx/json-api's MediaType's equalsTo considers parameters, which in case of e.g. multipart/form-data can contain dynamic data.

Details

I tracked down the code that is responsible, but am not sure how one could handle that case. Maybe support wildcards?

The code that checks for support of a given mediatype does the following:

• In ContentNegotiator::checkContentType the passed in DecodingList is checked...

// CloudCreativity\LaravelJsonApi\Http\ContentNegotiator

protected function checkContentType(HeaderInterface $header, DecodingList $supported): Decoding
{
    if (!$decoder = $supported->forHeader($header)) {  // <== does decoder exist for given headers?
        throw $this->unsupportedMediaType();
    }
    return $decoder;
}

• ... whether it has decoders supporting provided headers (DecodingList::forHeader)

// CloudCreativity\LaravelJsonApi\Codec\DecodingList
public function forHeader(HeaderInterface $header): ?Decoding
{
    foreach ($header->getMediaTypes() as $mediaType) {
        if ($decoding = $this->equalsTo($mediaType)) {  // <== does any mediatype match decoder's mediatype?
            return $decoding;
        }
    }
    return null;
}

public function equalsTo(MediaTypeInterface $mediaType): ?Decoding
{
    return collect($this->stack)->first(function (Decoding $decoding) use ($mediaType) {
        return $decoding->equalsTo($mediaType);
    });
}

• ... via Decoding::equalsTo

// CloudCreativity\LaravelJsonApi\Codec\Decoding

public function equalsTo(MediaTypeInterface $mediaType): bool
{
    return $this->mediaType->equalsTo($mediaType);
}

• ... leveraging neomerx/json-api's MediaType, ⚠️ which compares the parameters of the MediaTypes in question and does not allow wildcards

// Neomerx\JsonApi\Http\Headers\MediaType
public function equalsTo(MediaTypeInterface $mediaType): bool
{
    return
        $this->isTypeEquals($mediaType) &&
        $this->isSubTypeEquals($mediaType) &&
        $this->isMediaParametersEqual($mediaType);  // <== CULPRIT!
}

Thus, such a request will never match a configured decoder for multipart/form-data:

POST /api/v1/users HTTP/1.1
Host: localhost:8000
Accept: application/vnd.api+json
Content-Type: multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW

------WebKitFormBoundary7MA4YWxkTrZu0gW
Content-Disposition: form-data; name="user"; filename="user.xml"
Content-Type: text/xml
------WebKitFormBoundary7MA4YWxkTrZu0gW--

[lindyhopchris]

Ok yeah I see the problem. Thanks for the great explanation. I think we'll have to open an issue in the neomerx package and see if he wants to fix it there. If not, then maybe we do something to remove the boundary parameter from a multipart/form-data header before matching.

[lindyhopchris]

See https://github.com/neomerx/json-api/issues/221

[lindyhopchris]

Good news, neomerx/json-api has support for wildcards to v3 of their package.

Bad news, we're currently tied to v1 of neomerx/json-api. That's because it's going to be breaking to upgrade that package, and I haven't wanted to do that prior to v1 of this package.

The plan was to upgrade the neomerx package and add PHP 7 type-hinting throughout for v2 of this package. So my options are either:

• do v2 immediately after v1.
• add a custom solution to v1 that strips out boundaries to solve this specific use case, before we can use the wildcard solution in v2.

My initial feeling is to do the latter, because I'm not sure I want the churn of v2 immediately after releasing v1. But would be interested in hearing your thoughts @tsterker

[tsterker]

@lindyhopchris Do I understand correctly, that the second option would add an opt-in feature to allow stripping out the boundary parameter (or some generalized version of it)?

I think that sounds like a great idea! Esp. as it would add rudimentary content negotiation support for multipart/form-data in v1. :)

---

Some additional thoughts:

Would you consider this a "throw-away" feature that would essentially be deprecated in v2, or would/could it continue serving specific use cases that a wildcard can not solve? E.g. I guess a wildcard would not be able to support "strip out only the boundary parameter".

Maybe that might be overkill or not even possible in the current architecture, but I am thinking about some sort of "parameter exclusion list for MediaType matching"; e.g. in a ContentNegotiator:

protected $ignoreMediaTypeParametersForDecodermatching = [
    'multipart/form-data' => ['boundary'],
];

That said, I don't know if this is such a common use-case in real-world content negotiation that it would justify such a feature.

[lindyhopchris]

@tsterker I've pushed support for the wildcard parameters, only in the decoders and without upgrading the neomerx dependency.

Could you give it a go? It's on the issue265 branch. That's branched off 1.0.0-rc.1 so if you haven't upgraded to that yet, you'll need to check the upgrade guide.

It works by allowing you to set one of your decoding media types to:

multipart/form-data; boundary=*

I.e. just like the change in the neomerx package. But I've cheated and basically strip out any wildcard parameters before comparing the media types - i.e. ignore them.

You'd probably actually want to define two allowed media types: multipart/form-data and multipart/form-data; boundary=*, as per the updated docs in this commit: https://github.com/cloudcreativity/laravel-json-api/commit/34b6d9e9db841df43854f8a86f012034e4b3209d

Let me know if it works for your use-case. If it does I'll merge it and then the plan will be to remove the stripping once I've upgraded the neomerx package to the version that allows wildcards.

[tsterker]

@lindyhopchris Works like a charm, from what my initial tests showed. Thanks a lot!

[lindyhopchris]

@tsterker great, sounds good. I'll get it merged and tag it as 1.0.0-rc.2. I'm planning to tag 1.0.0 at the same time as Laravel 5.8 is released, as I think that's relatively soon.