Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Avoid string.Split allocations in System.Net.Http #7468

Merged
stephentoub merged 3 commits into from
Apr 22, 2016
Merged

Avoid string.Split allocations in System.Net.Http #7468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
94898c4
Avoid string.Split allocations in ContentDispositionHeaderValue
jamesqo Apr 17, 2016
da18ff0
Add tests for changes in ContentDispositionHeaderValue
jamesqo Apr 19, 2016
be706be
Remove problematic test cases with double quotes
jamesqo Apr 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions src/System.Net.Http/src/System/Net/Http/Headers/ContentDispositionHeaderValue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public string DispositionType
get { return _dispositionType; }
set
{
CheckDispositionTypeFormat(value, "value");
CheckDispositionTypeFormat(value, nameof(value));
_dispositionType = value;
}
}
Expand Down Expand Up @@ -157,7 +157,7 @@ protected ContentDispositionHeaderValue(ContentDispositionHeaderValue source)

public ContentDispositionHeaderValue(string dispositionType)
{
CheckDispositionTypeFormat(dispositionType, "dispositionType");
CheckDispositionTypeFormat(dispositionType, nameof(dispositionType));
_dispositionType = dispositionType;
}

Expand Down Expand Up @@ -501,6 +501,7 @@ private bool TryDecodeMime(string input, out string output)
{
return false;
}

string[] parts = processedInput.Split('?');
// "=, encodingName, encodingType, encodedData, ="
if (parts.Length != 5 || parts[0] != "\"=" || parts[4] != "=\"" || parts[2].ToLowerInvariant() != "b")
Expand Down Expand Up @@ -564,18 +565,27 @@ private string Encode5987(string input)
private bool TryDecode5987(string input, out string output)
{
output = null;
string[] parts = input.Split('\'');
if (parts.Length != 3)

int quoteIndex = input.IndexOf('\'');
if (quoteIndex == -1)
{
return false;
}

int lastQuoteIndex = input.LastIndexOf('\'');
if (quoteIndex == lastQuoteIndex || input.IndexOf('\'', quoteIndex + 1) != lastQuoteIndex)
{
return false;
Copy link
Member

@stephentoub stephentoub Apr 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any tests that hit this? I don't see any in either the functional or unit tests.

Copy link
Contributor Author

@jamesqo jamesqo Apr 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephentoub Added tests in a new commit. Let me know if/when this is ready to be merged, and I'll squash it.

}

string encodingString = input.Substring(0, quoteIndex);
string dataString = input.Substring(lastQuoteIndex + 1, input.Length - (lastQuoteIndex + 1));

StringBuilder decoded = new StringBuilder();
try
{
Encoding encoding = Encoding.GetEncoding(parts[0]);
Encoding encoding = Encoding.GetEncoding(encodingString);

string dataString = parts[2];
byte[] unescapedBytes = new byte[dataString.Length];
int unescapedBytesCount = 0;
for (int index = 0; index < dataString.Length; index++)
Expand Down
19 changes: 17 additions & 2 deletions src/System.Net.Http/tests/UnitTests/Headers/ContentDispositionHeaderValueTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,22 @@ public void FileNameStar_AddNameParameterThenUseProperty_ParametersEntryIsOverwr
contentDisposition.Parameters.Remove(fileNameStar);
Assert.Null(contentDisposition.FileNameStar);
}

[Theory]
[InlineData("no_quotes")]
[InlineData("one'quote")]
[InlineData("'triple'quotes'")]
public void FileNameStar_NotTwoQuotes_IsNull(string value)
{
ContentDispositionHeaderValue contentDisposition = new ContentDispositionHeaderValue("inline");

// Note that uppercase letters are used. Comparison should happen case-insensitive.
NameValueHeaderValue fileNameStar = new NameValueHeaderValue("FILENAME*", value);
contentDisposition.Parameters.Add(fileNameStar);
Assert.Equal(1, contentDisposition.Parameters.Count);
Assert.Same(fileNameStar, contentDisposition.Parameters.First());
Assert.Null(contentDisposition.FileNameStar); // Decode failure
}

[Fact]
public void FileNameStar_NeedsEncoding_EncodedAndDecodedCorrectly()
Expand All @@ -213,8 +229,7 @@ public void FileNameStar_UnknownOrBadEncoding_PropertyFails()
NameValueHeaderValue fileNameStar = new NameValueHeaderValue("FILENAME*", "utf-99'lang'File%CZName.bat");
contentDisposition.Parameters.Add(fileNameStar);
Assert.Equal(1, contentDisposition.Parameters.Count);
Assert.Equal("FILENAME*", contentDisposition.Parameters.First().Name);
Assert.Equal("utf-99'lang'File%CZName.bat", contentDisposition.Parameters.First().Value);
Assert.Same(fileNameStar, contentDisposition.Parameters.First());
Assert.Null(contentDisposition.FileNameStar); // Decode failure

contentDisposition.FileNameStar = "new_name";
Expand Down