http: add xml support to http::mime_types::mappings

[WillemKauf]

It seems strange that xml isn't supported as a mime type here.

In particular, we expect all S3 REST error responses to be returned with a Content-Type header of application/xml, so being able to map the extension makes life a lot easier for testing with imposter http S3 servers/services.

[avikivity]

Perhaps we should load /etc/mime-types (and merge with the built-in set of mappings)

[xemul]

There's set_mime_type() that can be used to set whatever "raw" type you need. What's the exact issue you're facing? @WillemKauf

[nyh]

There's set_mime_type() that can be used to set whatever "raw" type you need. What's the exact issue you're facing? @WillemKauf

Note that although set_mime_type() exists, there are other methods which take only that fixed set of mime types, which is silly. So for example when in ScyllaDB Alternator I had the same problem - I wanted to use application/x-amz-json-1.0 but this wasn't one of the mime types on the list - I had to use a random other type, "json", in write_body()

                    rep->write_body("json", std::move(json_return_value._body_writer));

And then override it later with

                    rep->set_mime_type("application/x-amz-json-1.0");
                    rep->done();

This is ugly, and need to be rethought. I don't think adding one or 17 more mime types to the list really solves this problem.

[WillemKauf]

What's the exact issue you're facing?

Similar to @nyh's problem above, using function_handler to mock HTTP responses doesn't allow for an application/xml Content-Type header, since rep->done(_type) is called.

seastar/include/seastar/http/function_handlers.hh

Line 111 in f05eb1e

rep->done(_type);

I'm getting around this currently via a new impl for handler_base, but I think the mapping is worth adding to avoid having to do this.

[nyh]

I have an idea: What if when the mime type contains a slash, we use it without looking it up in any tables?
I don't see why Seastar needs to contain a long list of popular (and unpopular) mime types if the application can supply the full mime type.

[xemul]

I have an idea: What if when the mime type contains a slash, we use it without looking it up in any tables?

You mean the write_body() methods? Because if you use set_mime_type directly it doesn't look up any tables already. @nyh

[xemul]

I have an idea: What if when the mime type contains a slash, we use it without looking it up in any tables?

You mean the write_body() methods? Because if you use set_mime_type directly it doesn't look up any tables already. @nyh

Ah, it's the function_handler::done(). OK

[nyh]

I have an idea: What if when the mime type contains a slash, we use it without looking it up in any tables?

You mean the write_body() methods?

Yes, write_body() calls set_content_type() instead of set_mime_type() - and set_content_type() is the one that does the lookup and calls set_content_type().
Also done() calls set_content_type() instead of set_mime_type().

I claim that this distinction, between "content type" and "mime type" is simply unnecessary and even misleading (as the set_content_type() today specifies the extension not a MIME or HTTP Content-Type). As you can see in the MIME RFC 2045 section 5.1, https://datatracker.ietf.org/doc/html/rfc2045#section-5.1, a full MIME type always needs to look like type/subtype. E.g., application/json. A string without a slash, like "json", cannot be a full MIME type so it makes sense to look it up in some sort of shortcut table (a la Unix's /etc/mime.types).

I propose the following fix instead of this patch:

1. Remove one of the two functions set_content_type() or set_mime_type() (both are reasonable names), call only the second one.
2. Make the one function call the array lookup function only if there is no slash in the given type.

I think this fix will be very easy to do and will avoid a deluge of patches like this one each adding their favorite MIME type. I'll also use the new patch in ScyllaDB instead of an ugly workaround (as I explained above).

I'll open an issue asking for this.

[nyh]

Don't need to open an issue - I already opened one four years ago: #964
Amazing :-)
So if you follow my suggestion, your patch can also "Fixes #964".

[nyh]

I see that despite my recommendation, Avi merged this match.
I still claim that the list of extensions, whether or not it contains xml, is the wrong way to go, and we should do
#964

I'll send a patch to do it.