Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Commit 0c3585f

Browse files
kichallakichalla
committed
[Fixes #4945] Simple string returned by controller action is not a valid JSON!
1 parent 9cc20ff commit 0c3585f

File tree

3 files changed

+84
-35
lines changed

3 files changed

+84
-35
lines changed

‎src/Microsoft.AspNetCore.Mvc.Core/Formatters/StringOutputFormatter.cs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,32 @@ public override bool CanWriteResult(OutputFormatterCanWriteContext context)
3333
// always return it as a text/plain format.
3434
if (context.ObjectType == typeof(string) || context.Object is string)
3535
{
36-
if (!context.ContentType.HasValue)
36+
if (!context.ContentType.HasValue || IsSupportedMediaType(context.ContentType.Value))
3737
{
3838
var mediaType = SupportedMediaTypes[0];
3939
var encoding = SupportedEncodings[0];
4040
context.ContentType = new StringSegment(MediaType.ReplaceEncoding(mediaType, encoding));
41+
return true;
4142
}
42-
43-
return true;
4443
}
4544

4645
return false;
4746
}
4847

48+
private bool IsSupportedMediaType(string mediaType)
49+
{
50+
var parsedContentType = new MediaType(mediaType);
51+
for (var i = 0; i < SupportedMediaTypes.Count; i++)
52+
{
53+
var supportedMediaType = new MediaType(SupportedMediaTypes[i]);
54+
if (parsedContentType.IsSubsetOf(supportedMediaType))
55+
{
56+
return true;
57+
}
58+
}
59+
return false;
60+
}
61+
4962
public override Task WriteResponseBodyAsync(OutputFormatterWriteContext context, Encoding encoding)
5063
{
5164
if (context == null)

‎test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/StringOutputFormatterTests.cs

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,31 @@
1212

1313
namespace Microsoft.AspNetCore.Mvc.Formatters
1414
{
15-
public class TextPlainFormatterTests
15+
public class StringOutputFormatterTests
1616
{
17-
public static IEnumerable<object[]> OutputFormatterContextValues
17+
public static IEnumerable<object[]> CanWriteResultForStringTypesData
1818
{
1919
get
2020
{
21-
// object value, bool useDeclaredTypeAsString, bool expectedCanWriteResult
22-
yield return new object[] { "valid value", true, true };
23-
yield return new object[] { null, true, true };
24-
yield return new object[] { null, false, false };
25-
yield return new object[] { new object(), false, false };
21+
// object value, bool useDeclaredTypeAsString
22+
yield return new object[] { "declared and runtime type are same", true };
23+
yield return new object[] { "declared and runtime type are different", false };
24+
yield return new object[] { null, true };
25+
}
26+
}
27+
28+
public static IEnumerable<object[]> CannotWriteResultForNonStringTypesData
29+
{
30+
get
31+
{
32+
// object value, bool useDeclaredTypeAsString
33+
yield return new object[] { null, false };
34+
yield return new object[] { new object(), false };
2635
}
2736
}
2837

2938
[Fact]
30-
public void CanWriteResult_SetsAcceptContentType()
39+
public void CannotWriteResult_ForNonTextPlainOrNonBrowserMediaTypes()
3140
{
3241
// Arrange
3342
var formatter = new StringOutputFormatter();
@@ -44,7 +53,7 @@ public void CanWriteResult_SetsAcceptContentType()
4453
var result = formatter.CanWriteResult(context);
4554

4655
// Assert
47-
Assert.True(result);
56+
Assert.False(result);
4857
Assert.Equal(expectedContentType, context.ContentType);
4958
}
5059

@@ -53,13 +62,17 @@ public void CanWriteResult_DefaultContentType()
5362
{
5463
// Arrange
5564
var formatter = new StringOutputFormatter();
56-
5765
var context = new OutputFormatterWriteContext(
5866
new DefaultHttpContext(),
5967
new TestHttpResponseStreamWriterFactory().CreateWriter,
6068
typeof(string),
6169
"Thisisastring");
6270

71+
// For example, this can happen when a request is received without any Accept header OR a request
72+
// is from a browser (in which case this ContentType is set to null by the infrastructure when
73+
// RespectBrowserAcceptHeader is set to false)
74+
context.ContentType = new StringSegment();
75+
6376
// Act
6477
var result = formatter.CanWriteResult(context);
6578

@@ -69,14 +82,13 @@ public void CanWriteResult_DefaultContentType()
6982
}
7083

7184
[Theory]
72-
[MemberData(nameof(OutputFormatterContextValues))]
73-
public void CanWriteResult_ReturnsTrueForStringTypes(
85+
[MemberData(nameof(CanWriteResultForStringTypesData))]
86+
public void CanWriteResult_ForStringTypes(
7487
object value,
75-
bool useDeclaredTypeAsString,
76-
bool expectedCanWriteResult)
88+
bool useDeclaredTypeAsString)
7789
{
7890
// Arrange
79-
var expectedContentType = new StringSegment("application/json");
91+
var expectedContentType = new StringSegment("text/plain; charset=utf-8");
8092

8193
var formatter = new StringOutputFormatter();
8294
var type = useDeclaredTypeAsString ? typeof(string) : typeof(object);
@@ -86,16 +98,42 @@ public void CanWriteResult_ReturnsTrueForStringTypes(
8698
new TestHttpResponseStreamWriterFactory().CreateWriter,
8799
type,
88100
value);
89-
context.ContentType = expectedContentType;
101+
context.ContentType = new StringSegment("text/plain");
90102

91103
// Act
92104
var result = formatter.CanWriteResult(context);
93105

94106
// Assert
95-
Assert.Equal(expectedCanWriteResult, result);
107+
Assert.True(result);
96108
Assert.Equal(expectedContentType, context.ContentType);
97109
}
98110

111+
[Theory]
112+
[MemberData(nameof(CannotWriteResultForNonStringTypesData))]
113+
public void CannotWriteResult_ForNonStringTypes(
114+
object value,
115+
bool useDeclaredTypeAsString)
116+
{
117+
// Arrange
118+
var expectedContentType = new StringSegment("text/plain; charset=utf-8");
119+
120+
var formatter = new StringOutputFormatter();
121+
var type = useDeclaredTypeAsString ? typeof(string) : typeof(object);
122+
123+
var context = new OutputFormatterWriteContext(
124+
new DefaultHttpContext(),
125+
new TestHttpResponseStreamWriterFactory().CreateWriter,
126+
type,
127+
value);
128+
context.ContentType = new StringSegment("text/plain");
129+
130+
// Act
131+
var result = formatter.CanWriteResult(context);
132+
133+
// Assert
134+
Assert.False(result);
135+
}
136+
99137
[Fact]
100138
public async Task WriteAsync_DoesNotWriteNullStrings()
101139
{

‎test/Microsoft.AspNetCore.Mvc.FunctionalTests/ContentNegotiationTest.cs

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -345,33 +345,31 @@ public async Task XmlFormatter_SupportedMediaType_DoesNotChangeAcrossRequests()
345345
}
346346

347347
[Theory]
348-
[InlineData(true)]
349-
[InlineData(false)]
350-
public async Task ObjectResult_WithStringReturnType_DefaultToTextPlain(bool matchFormatterOnObjectType)
348+
[InlineData(null)]
349+
[InlineData("text/plain")]
350+
[InlineData("text/plain; charset=utf-8")]
351+
[InlineData("text/html, application/xhtml+xml, image/jxr, */*")] // typical browser accept header
352+
public async Task ObjectResult_WithStringReturnType_DefaultToTextPlain(string acceptMediaType)
351353
{
352354
// Arrange
353-
var targetUri = "http://localhost/FallbackOnTypeBasedMatch/ReturnString?matchFormatterOnObjectType=true" +
354-
matchFormatterOnObjectType;
355-
var request = new HttpRequestMessage(HttpMethod.Get, targetUri);
355+
var request = new HttpRequestMessage(HttpMethod.Get, "FallbackOnTypeBasedMatch/ReturnString");
356+
request.Headers.Accept.ParseAdd(acceptMediaType);
356357

357358
// Act
358359
var response = await Client.SendAsync(request);
359360

360361
// Assert
361362
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
362-
Assert.Equal("text/plain", response.Content.Headers.ContentType.MediaType);
363+
Assert.Equal("text/plain; charset=utf-8", response.Content.Headers.ContentType.ToString());
363364
var actualBody = await response.Content.ReadAsStringAsync();
364365
Assert.Equal("Hello World!", actualBody);
365366
}
366367

367-
[Theory]
368-
[InlineData(true)]
369-
[InlineData(false)]
370-
public async Task ObjectResult_WithStringReturnType_SetsMediaTypeToAccept(bool matchFormatterOnObjectType)
368+
[Fact]
369+
public async Task ObjectResult_WithStringReturnType_AndNonTextPlainMediaType_DoesNotReturnTextPlain()
371370
{
372371
// Arrange
373-
var targetUri = "http://localhost/FallbackOnTypeBasedMatch/ReturnString?matchFormatterOnObjectType=" +
374-
matchFormatterOnObjectType;
372+
var targetUri = "http://localhost/FallbackOnTypeBasedMatch/ReturnString";
375373
var request = new HttpRequestMessage(HttpMethod.Get, targetUri);
376374
request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/json"));
377375

@@ -380,9 +378,9 @@ public async Task ObjectResult_WithStringReturnType_SetsMediaTypeToAccept(bool m
380378

381379
// Assert
382380
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
383-
Assert.Equal("application/json", response.Content.Headers.ContentType.MediaType);
381+
Assert.Equal("application/json; charset=utf-8", response.Content.Headers.ContentType.ToString());
384382
var actualBody = await response.Content.ReadAsStringAsync();
385-
Assert.Equal("Hello World!", actualBody);
383+
Assert.Equal("\"Hello World!\"", actualBody);
386384
}
387385

388386
[Fact]

0 commit comments

Comments
 (0)