Wrong test for content_types_accepted

[zoggy]

The same function Wm_util.MediaType.match_header is used so that the following checks are performed:

• content types provided by server are subtypes of content types required by client
• content types accepted by server are subtypes of content types provided by client.

In the second case this is wrong, as the control should be the other way: content types provided by client must be subtypes of content types accepted by server.

I will provide a patch.

[seliopou]

I've had a chance to look into this, and I agree that there is something fishy about the way that this is implemented. However, I don't think it's quite as you describe. In a POST or PUT request, the Content-Type header specifies the media type of the body. Note that this is not a media range. Similarly, the result of resource#content_types_accepted is an association list whose first elements are also media types. These also are not media ranged.

For the resource to be able to service the request, the Content-Type of the request must match a media type reported by resource#content_types_accepted. But since both are media types and not media ranges, this should amount to a simple equality check. between to two (ignoring suffixes). When provided only media types Wm_util.MediaType.match_header will essentially perform this check.

Having said that, there are still problems with the code as it currently exists in master. First, the implementation of this check is sloppy and obviously misleading, given the existence of this issue. Second, the intended behavior of resource#content_types_provided isn't documented, which further leads to confusion. Third and finally, there is no check that resource#content_types_accepted and resource#content_types_provided are actually returning media types and not media ranges. These areas could certainly be improved.

Let me know if you think I've misunderstood the situation.

[zoggy]

In a POST or PUT request, the Content-Type header specifies the media type of the body. Note that this is not a media range. Similarly, the result of resource#content_types_accepted is an association list whose first elements are also media types. These also are not media ranged.

Ah. I thought this first component ought to be a media range, so that I could say that the server accepts any image (with "image/*") for example. This is why I I fixed that way in this PR and I think it would be better if the first component of the list returned by resource#content_type_accepted was a media range, not only a media type. This seems to me a simple way to indicate the method to call according to the media type provided by client. Then the selected method can still access the media type of the POST/PUT request to make some specific treatment.

I bumped into this problem with resource#content_type_accepted because I wanted to accept any content in a PUT request, so I used "*/*" as first component of the returned list. I could live with the current implementation and use the content type of the request. But if I wanted to handle only images, the provided patch (i.e. having a media range as first component) would be more convenient.

Regarding resource#content_type_provided, no problem, I think it has the correct behaviour.

[seliopou]

The issue with accepting a media range rather than a media type is that without the second component, you don't know how to interpret the content of the PUT. image/* may tell you it's an image, but what kind of image? a PNG? a GIF? If the type is not relevant, and you just wanna write the bytes out somewhere and serve them up later, then the media type you probably want is application/octet-stream.

It looks like there are only a limited number of image types that are considered "web safe" listed here. You could just enumerate these in the list you return and just map them all to the same handler:

let image_types =

method content_types_accepted rd =
  Wm.continue [
    ("image/gif", self#handle_image);
    ("image/jpg", self#handle_image);
    ("image/png", self#handle_image);
    ("image/svg+xml", self#handle_image);
  ] rd

I think for now the code should still only recognize media types rather than ranges. But the code needs to be cleaned up a bit for sure.