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

[API Proposal]: Add multipart/related to MediaTypeNames.Multipart #95446

Closed
CollinAlpert opened this issue Nov 30, 2023 · 10 comments · Fixed by #103575
Closed

[API Proposal]: Add multipart/related to MediaTypeNames.Multipart #95446

CollinAlpert opened this issue Nov 30, 2023 · 10 comments · Fixed by #103575
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Net in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@CollinAlpert
Copy link
Contributor

CollinAlpert commented Nov 30, 2023

Background and motivation

The System.Net.Mime.MediaTypeNames.Multipart type already contains a constant for multipart/form-data which can be used by developers instead of introducing own constants at the risk of typos or duplication.

The same should exist for multipart/related.

API Proposal

namespace System.Net.Mime;

public static class MediaTypeNames
{
    public static class Multipart
    {
         // see https://www.ietf.org/rfc/rfc2387.txt
+        public const string Related = "multipart/related";
         // see https://www.w3.org/Protocols/rfc1341/7_2_Multipart.html
+        public const string Mixed = "multipart/mixed"
    }
    public static class Application
    {
         // https://datatracker.ietf.org/doc/html/rfc6713
+        public const string Gzip = "application/gzip";
         // see https://github.com/ndjson/ndjson-spec
+        public const string NDJson = "application/x-ndjson";
    }
    public static class Text
    {
        // see https://github.com/dotnet/runtime/issues/102194
+       public const string EventStream = "text/event-stream";
    }
}

API Usage

using Microsoft.AspNetCore.Http;
using System.Net.Mime;

void M(HttpRequest request)
{
    Console.WriteLine(request.ContentType == MediaTypeNames.Multipart.Related)
}

Alternative Designs

No response

Risks

No response

@CollinAlpert CollinAlpert added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Nov 30, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 30, 2023
@ghost
Copy link

ghost commented Nov 30, 2023

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

The System.Net.Mime.MediaTypeNames.Multipart type already contains a constant for multipart/form-data which can be used by developers instead of introducing own constants at the risk of typos or duplication.

The same should exist for multipart/related.

API Proposal

namespace System.Net.Mime;

public static class MediaTypeNames
{
    public static class Multipart
    {
+        public const string Related = "multipart/related";
    }
}

API Usage

using Microsoft.AspNetCore.Http;
using System.Net.Mime;

void M(HttpRequest request)
{
    Console.WriteLine(request.ContentType == MediaTypeNames.Multipart.Related)
}

Alternative Designs

No response

Risks

No response

Author: CollinAlpert
Assignees: -
Labels:

api-suggestion, area-System.Net

Milestone: -

@ManickaP
Copy link
Member

Triage: seems reasonable, but we might consider collecting more together and batch them as we did in #85807.

@ManickaP ManickaP added this to the Future milestone Nov 30, 2023
@ManickaP ManickaP removed the untriaged New issue has not been triaged by the area owner label Nov 30, 2023
@CollinAlpert
Copy link
Contributor Author

Given this would only be introduced in .NET 9 anyway, I have no problem waiting for some more media types.

@CollinAlpert
Copy link
Contributor Author

Just discovered another one:

namespace System.Net.Mime;

public static class MediaTypeNames
{
    public static class Application
    {
+        public const string Gzip = "application/gzip";
    }
}

@RenderMichael
Copy link
Contributor

What about application/x-ndjson?

@ManickaP
Copy link
Member

ManickaP commented Dec 1, 2023

Keep them incoming. But please also upvote the issue so that we can prioritize accordingly.

@CollinAlpert
Copy link
Contributor Author

@ManickaP Since the first preview of .NET 9 has been released and there has been no further activity, can we hand this over to API review?

@Bellgrad
Copy link

Bellgrad commented Apr 8, 2024

In relation to an issue I submitted today, there is also multipart/mixed missing in system.net.mime.mediatypenames.multipart. Not dramatic but still usefull.

@ManickaP
Copy link
Member

@dotnet/ncl Let's talk about this issue and #102194 in our next sync so we can push it through API review.

@ManickaP ManickaP added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels May 30, 2024
@ManickaP ManickaP self-assigned this May 30, 2024
@ManickaP ManickaP added the blocking Marks issues that we want to fast track in order to unblock other important work label Jun 5, 2024
@bartonjs
Copy link
Member

bartonjs commented Jun 13, 2024

Video

  • We changed Gzip to GZip to match the casing of GZipStream
  • We removed NDJson as it's not recognized by IANA.
namespace System.Net.Mime;

public static partial class MediaTypeNames
{
    public static partial class Multipart
    {
         // see https://www.ietf.org/rfc/rfc2387.txt
         public const string Related = "multipart/related";
         // see https://www.w3.org/Protocols/rfc1341/7_2_Multipart.html
         public const string Mixed = "multipart/mixed";
    }
    public static partial class Application
    {
         // https://datatracker.ietf.org/doc/html/rfc6713
         public const string GZip = "application/gzip";
    }
    public static partial class Text
    {
        // see https://github.com/dotnet/runtime/issues/102194
        public const string EventStream = "text/event-stream";
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jun 13, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-System.Net in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants