Skip to content

Commit

Permalink
#452 exclude Content-Type header by default for multipart AddString
Browse files Browse the repository at this point in the history
and related refactoring of CapturedStringContent constructors
  • Loading branch information
tmenier committed Sep 26, 2020
1 parent 77b29c4 commit 7defe0c
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 49 deletions.
28 changes: 16 additions & 12 deletions Test/Flurl.Test/Http/MultipartTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,33 @@ public class MultipartTests
public void can_build_multipart_content() {
var content = new CapturedMultipartContent()
.AddString("string", "foo")
.AddString("string2", "bar", "text/blah")
.AddStringParts(new { part1 = 1, part2 = 2, part3 = (string)null }) // part3 should be excluded
.AddFile("file1", Path.Combine("path", "to", "image1.jpg"), "image/jpeg")
.AddFile("file2", Path.Combine("path", "to", "image2.jpg"), "image/jpeg", fileName: "new-name.jpg")
.AddJson("json", new { foo = "bar" })
.AddUrlEncoded("urlEnc", new { fizz = "buzz" });

Assert.AreEqual(7, content.Parts.Length);

AssertStringPart<CapturedStringContent>(content.Parts[0], "string", "foo");
AssertStringPart<CapturedStringContent>(content.Parts[1], "part1", "1");
AssertStringPart<CapturedStringContent>(content.Parts[2], "part2", "2");
AssertFilePart(content.Parts[3], "file1", "image1.jpg", "image/jpeg");
AssertFilePart(content.Parts[4], "file2", "new-name.jpg", "image/jpeg");
AssertStringPart<CapturedJsonContent>(content.Parts[5], "json", "{\"foo\":\"bar\"}");
AssertStringPart<CapturedUrlEncodedContent>(content.Parts[6], "urlEnc", "fizz=buzz");
Assert.AreEqual(8, content.Parts.Length);
AssertStringPart<CapturedStringContent>(content.Parts[0], "string", "foo", null);
AssertStringPart<CapturedStringContent>(content.Parts[1], "string2", "bar", "text/blah");
AssertStringPart<CapturedStringContent>(content.Parts[2], "part1", "1", null);
AssertStringPart<CapturedStringContent>(content.Parts[3], "part2", "2", null);
AssertFilePart(content.Parts[4], "file1", "image1.jpg", "image/jpeg");
AssertFilePart(content.Parts[5], "file2", "new-name.jpg", "image/jpeg");
AssertStringPart<CapturedJsonContent>(content.Parts[6], "json", "{\"foo\":\"bar\"}", "application/json; charset=UTF-8");
AssertStringPart<CapturedUrlEncodedContent>(content.Parts[7], "urlEnc", "fizz=buzz", "application/x-www-form-urlencoded");
}

private void AssertStringPart<TContent>(HttpContent part, string name, string content) {
private void AssertStringPart<TContent>(HttpContent part, string name, string content, string contentType) {
Assert.IsInstanceOf<TContent>(part);
Assert.AreEqual(name, part.Headers.ContentDisposition.Name);
Assert.AreEqual(content, (part as CapturedStringContent)?.Content);
Assert.IsFalse(part.Headers.Contains("Content-Type")); // #392
}
if (contentType == null)
Assert.IsFalse(part.Headers.Contains("Content-Type")); // #392
else
Assert.AreEqual(contentType, part.Headers.GetValues("Content-Type").SingleOrDefault());
}

private void AssertFilePart(HttpContent part, string name, string fileName, string contentType) {
Assert.IsInstanceOf<FileContent>(part);
Expand Down
2 changes: 1 addition & 1 deletion src/Flurl.Http/Content/CapturedJsonContent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ public class CapturedJsonContent : CapturedStringContent
/// Initializes a new instance of the <see cref="CapturedJsonContent"/> class.
/// </summary>
/// <param name="json">The json.</param>
public CapturedJsonContent(string json) : base(json, Encoding.UTF8, "application/json") { }
public CapturedJsonContent(string json) : base(json, "application/json; charset=UTF-8") { }
}
}
51 changes: 22 additions & 29 deletions src/Flurl.Http/Content/CapturedMultipartContent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,31 +35,29 @@ public CapturedMultipartContent(FlurlHttpSettings settings = null) : base("form-
/// <param name="name">The control name of the part.</param>
/// <param name="content">The HttpContent of the part.</param>
/// <returns>This CapturedMultipartContent instance (supports method chaining).</returns>
public CapturedMultipartContent Add(string name, HttpContent content) => AddInternal(name, content, true, null);
public CapturedMultipartContent Add(string name, HttpContent content) => AddInternal(name, content, null);

/// <summary>
/// Add a simple string part to the multipart request.
/// </summary>
/// <param name="name">The control name of the part.</param>
/// <param name="content">The string content of the part.</param>
/// <param name="encoding">The encoding of the part.</param>
/// <param name="mediaType">The media type of the part.</param>
/// <param name="name">The name of the part.</param>
/// <param name="value">The string value of the part.</param>
/// <param name="contentType">The value of the Content-Type header for this part. If null (the default), header will be excluded, which complies with the HTML 5 standard.</param>
/// <returns>This CapturedMultipartContent instance (supports method chaining).</returns>
public CapturedMultipartContent AddString(string name, string content, Encoding encoding = null, string mediaType = null) =>
AddInternal(name, new CapturedStringContent(content, encoding, mediaType), false, null);
public CapturedMultipartContent AddString(string name, string value, string contentType = null) =>
AddInternal(name, new CapturedStringContent(value, contentType), null);

/// <summary>
/// Add multiple string parts to the multipart request by parsing an object's properties into control name/content pairs.
/// </summary>
/// <param name="data">The object (typically anonymous) whose properties are parsed into control name/content pairs.</param>
/// <param name="encoding">The encoding of the parts.</param>
/// <param name="mediaType">The media type of the parts.</param>
/// <param name="contentType">The value of the Content-Type header for this part. If null, header will be excluded, which complies with the HTML 5 standard.</param>
/// <returns>This CapturedMultipartContent instance (supports method chaining).</returns>
public CapturedMultipartContent AddStringParts(object data, Encoding encoding = null, string mediaType = null) {
public CapturedMultipartContent AddStringParts(object data, string contentType = null) {
foreach (var kv in data.ToKeyValuePairs()) {
if (kv.Value == null)
continue;
AddString(kv.Key, kv.Value.ToInvariantString(), encoding, mediaType);
AddString(kv.Key, kv.Value.ToInvariantString(), contentType);
}
return this;
}
Expand All @@ -71,7 +69,7 @@ public CapturedMultipartContent AddStringParts(object data, Encoding encoding =
/// <param name="data">The content of the part, which will be serialized to JSON.</param>
/// <returns>This CapturedMultipartContent instance (supports method chaining).</returns>
public CapturedMultipartContent AddJson(string name, object data) =>
AddInternal(name, new CapturedJsonContent(_settings.JsonSerializer.Serialize(data)), false, null);
AddInternal(name, new CapturedJsonContent(_settings.JsonSerializer.Serialize(data)), null);

/// <summary>
/// Add a URL-encoded part to the multipart request.
Expand All @@ -80,50 +78,45 @@ public CapturedMultipartContent AddJson(string name, object data) =>
/// <param name="data">The content of the part, whose properties will be parsed and serialized to URL-encoded format.</param>
/// <returns>This CapturedMultipartContent instance (supports method chaining).</returns>
public CapturedMultipartContent AddUrlEncoded(string name, object data) =>
AddInternal(name, new CapturedUrlEncodedContent(_settings.UrlEncodedSerializer.Serialize(data)), false, null);
AddInternal(name, new CapturedUrlEncodedContent(_settings.UrlEncodedSerializer.Serialize(data)), null);

/// <summary>
/// Adds a file to the multipart request from a stream.
/// </summary>
/// <param name="name">The control name of the part.</param>
/// <param name="stream">The file stream to send.</param>
/// <param name="fileName">The filename, added to the Content-Disposition header of the part.</param>
/// <param name="mediaType">The media type of the file.</param>
/// <param name="contentType">The content type of the file.</param>
/// <param name="bufferSize">The buffer size of the stream upload in bytes. Defaults to 4096.</param>
/// <returns>This CapturedMultipartContent instance (supports method chaining).</returns>
public CapturedMultipartContent AddFile(string name, Stream stream, string fileName, string mediaType = null, int bufferSize = 4096) {
public CapturedMultipartContent AddFile(string name, Stream stream, string fileName, string contentType = null, int bufferSize = 4096) {
var content = new StreamContent(stream, bufferSize);
if (mediaType != null)
content.Headers.ContentType = new MediaTypeHeaderValue(mediaType);
return AddInternal(name, content, true, fileName);
if (contentType != null)
content.Headers.TryAddWithoutValidation("Content-Type", contentType);
return AddInternal(name, content, fileName);
}

/// <summary>
/// Adds a file to the multipart request from a local path.
/// </summary>
/// <param name="name">The control name of the part.</param>
/// <param name="path">The local path to the file.</param>
/// <param name="mediaType">The media type of the file.</param>
/// <param name="contentType">The content type of the file.</param>
/// <param name="bufferSize">The buffer size of the stream upload in bytes. Defaults to 4096.</param>
/// <param name="fileName">The filename, added to the Content-Disposition header of the part. Defaults to local file name.</param>
/// <returns>This CapturedMultipartContent instance (supports method chaining).</returns>
public CapturedMultipartContent AddFile(string name, string path, string mediaType = null, int bufferSize = 4096, string fileName = null) {
public CapturedMultipartContent AddFile(string name, string path, string contentType = null, int bufferSize = 4096, string fileName = null) {
fileName = fileName ?? FileUtil.GetFileName(path);
var content = new FileContent(path, bufferSize);
if (mediaType != null)
content.Headers.ContentType = new MediaTypeHeaderValue(mediaType);
return AddInternal(name, content, true, fileName);
if (contentType != null)
content.Headers.TryAddWithoutValidation("Content-Type", contentType);
return AddInternal(name, content, fileName);
}

private CapturedMultipartContent AddInternal(string name, HttpContent content, bool allowContentType, string fileName) {
private CapturedMultipartContent AddInternal(string name, HttpContent content, string fileName) {
if (string.IsNullOrWhiteSpace(name))
throw new ArgumentException("name must not be empty", nameof(name));

// StringContent is the simplest way to add a string part, but it always
// includes Content-Type, and per HTML spec that's not allowed (#392)
if (!allowContentType && content.Headers.Contains("Content-Type"))
content.Headers.Remove("Content-Type");

content.Headers.ContentDisposition = new ContentDispositionHeaderValue("form-data") {
Name = name,
FileName = fileName,
Expand Down
19 changes: 14 additions & 5 deletions src/Flurl.Http/Content/CapturedStringContent.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Net.Http;
using System.Net.Http.Headers;
using System.Text;

namespace Flurl.Http.Content
Expand All @@ -14,16 +15,24 @@ public class CapturedStringContent : StringContent
/// </summary>
public string Content { get; }

/// <summary>
/// Initializes a new instance of <see cref="CapturedStringContent"/> with a Content-Type header of text/plain; charset=UTF-8
/// </summary>
/// <param name="content">The content.</param>
public CapturedStringContent(string content) : base(content) {
Content = content;
}

/// <summary>
/// Initializes a new instance of the <see cref="CapturedStringContent"/> class.
/// </summary>
/// <param name="content">The content.</param>
/// <param name="encoding">The encoding.</param>
/// <param name="mediaType">Type of the media.</param>
public CapturedStringContent(string content, Encoding encoding = null, string mediaType = null) :
base(content, encoding, mediaType)
{
/// <param name="contentType">Value of the Content-Type header. To exclude the header, set to null explicitly.</param>
public CapturedStringContent(string content, string contentType) : base(content) {
Content = content;
Headers.Remove("Content-Type");
if (contentType != null)
Headers.TryAddWithoutValidation("Content-Type", contentType);
}
}
}
2 changes: 1 addition & 1 deletion src/Flurl.Http/Content/CapturedUrlEncodedContent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ public class CapturedUrlEncodedContent : CapturedStringContent
/// Initializes a new instance of the <see cref="CapturedUrlEncodedContent"/> class.
/// </summary>
/// <param name="data">Content represented as a (typically anonymous) object, which will be parsed into name/value pairs.</param>
public CapturedUrlEncodedContent(string data) : base(data, null, "application/x-www-form-urlencoded") { }
public CapturedUrlEncodedContent(string data) : base(data, "application/x-www-form-urlencoded") { }
}
}
1 change: 0 additions & 1 deletion src/Flurl.Http/HttpMessageExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ private static void SetHeader(this HttpMessage msg, string name, object value, b
}
}


/// <summary>
/// Wrapper class for treating HttpRequestMessage and HttpResponseMessage uniformly. (Unfortunately they don't have a common interface.)
/// </summary>
Expand Down

0 comments on commit 7defe0c

Please sign in to comment.