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

Commit f8a1bf1

Browse files
TratcherTratcher
committed
#515 Make forgiving vs strict cookie parsers.
1 parent 765a520 commit f8a1bf1

File tree

5 files changed

+146
-43
lines changed

5 files changed

+146
-43
lines changed

‎src/Microsoft.Net.Http.Headers/CookieHeaderParser.cs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,6 @@ namespace Microsoft.Net.Http.Headers
77
{
88
internal class CookieHeaderParser : HttpHeaderParser<CookieHeaderValue>
99
{
10-
// The Cache-Control header is special: It is a header supporting a list of values, but we represent the list
11-
// as _one_ instance of CacheControlHeaderValue. I.e we set 'SupportsMultipleValues' to 'true' since it is
12-
// OK to have multiple Cache-Control headers in a request/response message. However, after parsing all
13-
// Cache-Control headers, only one instance of CacheControlHeaderValue is created (if all headers contain valid
14-
// values, otherwise we may have multiple strings containing the invalid values).
1510
internal CookieHeaderParser(bool supportsMultipleValues)
1611
: base(supportsMultipleValues)
1712
{
@@ -49,14 +44,11 @@ public sealed override bool TryParseValue(string value, ref int index, out Cooki
4944
}
5045

5146
CookieHeaderValue result = null;
52-
int length = CookieHeaderValue.GetCookieLength(value, current, out result);
53-
54-
if (length == 0)
47+
if (!CookieHeaderValue.TryGetCookieLength(value, ref current, out result))
5548
{
5649
return false;
5750
}
5851

59-
current = current + length;
6052
current = GetNextNonEmptyOrWhitespaceIndex(value, current, SupportsMultipleValues, out separatorFound);
6153

6254
// If we support multiple values and we've not reached the end of the string, then we must have a separator.
@@ -91,6 +83,7 @@ private static int GetNextNonEmptyOrWhitespaceIndex(string input, int startIndex
9183

9284
if (skipEmptyValues)
9385
{
86+
// Most headers only split on ',', but cookies primarily split on ';'
9487
while ((current < input.Length) && ((input[current] == ',') || (input[current] == ';')))
9588
{
9689
current++; // skip delimiter.

‎src/Microsoft.Net.Http.Headers/CookieHeaderValue.cs

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -107,22 +107,31 @@ public static IList<CookieHeaderValue> ParseList(IList<string> inputs)
107107
return MultipleValueParser.ParseValues(inputs);
108108
}
109109

110+
public static IList<CookieHeaderValue> ParseStrictList(IList<string> inputs)
111+
{
112+
return MultipleValueParser.ParseStrictValues(inputs);
113+
}
114+
110115
public static bool TryParseList(IList<string> inputs, out IList<CookieHeaderValue> parsedValues)
111116
{
112117
return MultipleValueParser.TryParseValues(inputs, out parsedValues);
113118
}
114119

120+
public static bool TryParseStrictList(IList<string> inputs, out IList<CookieHeaderValue> parsedValues)
121+
{
122+
return MultipleValueParser.TryParseStrictValues(inputs, out parsedValues);
123+
}
124+
115125
// name=value; name="value"
116-
internal static int GetCookieLength(string input, int startIndex, out CookieHeaderValue parsedValue)
126+
internal static bool TryGetCookieLength(string input, ref int offset, out CookieHeaderValue parsedValue)
117127
{
118-
Contract.Requires(startIndex >= 0);
119-
var offset = startIndex;
128+
Contract.Requires(offset >= 0);
120129

121130
parsedValue = null;
122131

123132
if (string.IsNullOrEmpty(input) || (offset >= input.Length))
124133
{
125-
return 0;
134+
return false;
126135
}
127136

128137
var result = new CookieHeaderValue();
@@ -135,44 +144,41 @@ internal static int GetCookieLength(string input, int startIndex, out CookieHead
135144
var itemLength = HttpRuleParser.GetTokenLength(input, offset);
136145
if (itemLength == 0)
137146
{
138-
return 0;
147+
return false;
139148
}
140149
result._name = input.Substring(offset, itemLength);
141150
offset += itemLength;
142151

143152
// = (no spaces)
144153
if (!ReadEqualsSign(input, ref offset))
145154
{
146-
return 0;
155+
return false;
147156
}
148157

149-
string value;
150158
// value or "quoted value"
151-
itemLength = GetCookieValueLength(input, offset, out value);
152159
// The value may be empty
153-
result._value = input.Substring(offset, itemLength);
154-
offset += itemLength;
160+
result._value = GetCookieValue(input, ref offset);
155161

156162
parsedValue = result;
157-
return offset - startIndex;
163+
return true;
158164
}
159165

160166
// cookie-value = *cookie-octet / ( DQUOTE* cookie-octet DQUOTE )
161167
// cookie-octet = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
162168
// ; US-ASCII characters excluding CTLs, whitespace DQUOTE, comma, semicolon, and backslash
163-
internal static int GetCookieValueLength(string input, int startIndex, out string value)
169+
internal static string GetCookieValue(string input, ref int offset)
164170
{
165171
Contract.Requires(input != null);
166-
Contract.Requires(startIndex >= 0);
167-
Contract.Ensures((Contract.Result<int>() >= 0) && (Contract.Result<int>() <= (input.Length - startIndex)));
172+
Contract.Requires(offset >= 0);
173+
Contract.Ensures((Contract.Result<int>() >= 0) && (Contract.Result<int>() <= (input.Length - offset)));
174+
175+
var startIndex = offset;
168176

169-
value = null;
170-
if (startIndex >= input.Length)
177+
if (offset >= input.Length)
171178
{
172-
return 0;
179+
return string.Empty;
173180
}
174181
var inQuotes = false;
175-
var offset = startIndex;
176182

177183
if (input[offset] == '"')
178184
{
@@ -195,19 +201,19 @@ internal static int GetCookieValueLength(string input, int startIndex, out strin
195201
{
196202
if (offset == input.Length || input[offset] != '"')
197203
{
198-
return 0; // Missing final quote
204+
// Missing final quote
205+
return string.Empty;
199206
}
200207
offset++;
201208
}
202209

203210
int length = offset - startIndex;
204-
if (length == 0)
211+
if (offset > startIndex)
205212
{
206-
return 0;
213+
return input.Substring(startIndex, length);
207214
}
208215

209-
value = input.Substring(startIndex, length);
210-
return length;
216+
return string.Empty;
211217
}
212218

213219
private static bool ReadEqualsSign(string input, ref int offset)
@@ -252,8 +258,9 @@ internal static void CheckValueFormat(string value, string parameterName)
252258
throw new ArgumentNullException(nameof(value));
253259
}
254260

255-
string temp;
256-
if (GetCookieValueLength(value, 0, out temp) != value.Length)
261+
var offset = 0;
262+
var result = GetCookieValue(value, ref offset);
263+
if (result.Length != value.Length)
257264
{
258265
throw new ArgumentException("Invalid cookie value: " + value, parameterName);
259266
}

‎src/Microsoft.Net.Http.Headers/HttpHeaderParser.cs

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,16 @@ public T ParseValue(string value, ref int index)
4646
}
4747

4848
public virtual bool TryParseValues(IList<string> values, out IList<T> parsedValues)
49+
{
50+
return TryParseValues(values, strict: false, parsedValues: out parsedValues);
51+
}
52+
53+
public virtual bool TryParseStrictValues(IList<string> values, out IList<T> parsedValues)
54+
{
55+
return TryParseValues(values, strict: true, parsedValues: out parsedValues);
56+
}
57+
58+
protected virtual bool TryParseValues(IList<string> values, bool strict, out IList<T> parsedValues)
4959
{
5060
Contract.Assert(_supportsMultipleValues);
5161
// If a parser returns an empty list, it means there was no value, but that's valid (e.g. "Accept: "). The caller
@@ -71,10 +81,15 @@ public virtual bool TryParseValues(IList<string> values, out IList<T> parsedValu
7181
results.Add(output);
7282
}
7383
}
74-
else
84+
else if (strict)
7585
{
7686
return false;
7787
}
88+
else
89+
{
90+
// Skip the invalid values and keep trying.
91+
index++;
92+
}
7893
}
7994
}
8095
if (results.Count > 0)
@@ -85,7 +100,17 @@ public virtual bool TryParseValues(IList<string> values, out IList<T> parsedValu
85100
return false;
86101
}
87102

88-
public IList<T> ParseValues(IList<string> values)
103+
public virtual IList<T> ParseValues(IList<string> values)
104+
{
105+
return ParseValues(values, strict: false);
106+
}
107+
108+
public virtual IList<T> ParseStrictValues(IList<string> values)
109+
{
110+
return ParseValues(values, strict: true);
111+
}
112+
113+
protected virtual IList<T> ParseValues(IList<string> values, bool strict)
89114
{
90115
Contract.Assert(_supportsMultipleValues);
91116
// If a parser returns an empty list, it means there was no value, but that's valid (e.g. "Accept: "). The caller
@@ -110,10 +135,14 @@ public IList<T> ParseValues(IList<string> values)
110135
parsedValues.Add(output);
111136
}
112137
}
138+
else if (strict)
139+
{
140+
throw new FormatException(string.Format(CultureInfo.InvariantCulture,
141+
"The header contains invalid values at index {0}: '{1}'", index, value));
142+
}
113143
else
114144
{
115-
throw new FormatException(string.Format(CultureInfo.InvariantCulture, "Invalid values '{0}'.",
116-
value.Substring(index)));
145+
index++;
117146
}
118147
}
119148
}

‎src/Microsoft.Net.Http.Headers/SetCookieHeaderValue.cs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,12 +195,9 @@ private static int GetSetCookieLength(string input, int startIndex, out SetCooki
195195
return 0;
196196
}
197197

198-
string value;
199198
// value or "quoted value"
200-
itemLength = CookieHeaderValue.GetCookieValueLength(input, offset, out value);
201199
// The value may be empty
202-
result._value = input.Substring(offset, itemLength);
203-
offset += itemLength;
200+
result._value = CookieHeaderValue.GetCookieValue(input, ref offset);
204201

205202
// *(';' SP cookie-av)
206203
while (offset < input.Length)

‎test/Microsoft.Net.Http.Headers.Tests/CookieHeaderValueTest.cs

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ public static TheoryData<string> InvalidCookieValues
7979
};
8080
}
8181
}
82+
8283
public static TheoryData<IList<CookieHeaderValue>, string[]> ListOfCookieHeaderDataSet
8384
{
8485
get
@@ -110,8 +111,48 @@ public static TheoryData<IList<CookieHeaderValue>, string[]> ListOfCookieHeaderD
110111
return dataset;
111112
}
112113
}
114+
public static TheoryData<IList<CookieHeaderValue>, string[]> ListOfInvalidCookieHeaderDataSet
115+
{
116+
get
117+
{
118+
var dataset = new TheoryData<IList<CookieHeaderValue>, string[]>();
119+
var header1 = new CookieHeaderValue("name1", "n1=v1&n2=v2&n3=v3");
120+
var validString1 = "name1=n1=v1&n2=v2&n3=v3";
121+
122+
var header2 = new CookieHeaderValue("name2", "value2");
123+
var validString2 = "name2=value2";
124+
125+
var header3 = new CookieHeaderValue("name3", "value3");
126+
var validString3 = "name3=value3";
127+
128+
var invalidString1 = "ipt={\"v\":{\"L\":3},\"pt\":{\"d\":3},\"ct\":{},\"_t\":44,\"_v\":\"2\"}";
129+
130+
dataset.Add(null, new[] { invalidString1 });
131+
dataset.Add(new[] { header1 }.ToList(), new[] { validString1, invalidString1 });
132+
dataset.Add(new[] { header1 }.ToList(), new[] { validString1, null, "", " ", ";", " , ", invalidString1 });
133+
dataset.Add(new[] { header1 }.ToList(), new[] { invalidString1, null, "", " ", ";", " , ", validString1 });
134+
dataset.Add(new[] { header1 }.ToList(), new[] { validString1 + ", " + invalidString1 });
135+
dataset.Add(new[] { header2 }.ToList(), new[] { invalidString1 + ", " + validString2 });
136+
dataset.Add(new[] { header1 }.ToList(), new[] { invalidString1 + "; " + validString1 });
137+
dataset.Add(new[] { header2 }.ToList(), new[] { validString2 + "; " + invalidString1 });
138+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { invalidString1, validString1, validString2, validString3 });
139+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { validString1, invalidString1, validString2, validString3 });
140+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { validString1, validString2, invalidString1, validString3 });
141+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { validString1, validString2, validString3, invalidString1 });
142+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(",", invalidString1, validString1, validString2, validString3) });
143+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(",", validString1, invalidString1, validString2, validString3) });
144+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(",", validString1, validString2, invalidString1, validString3) });
145+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(",", validString1, validString2, validString3, invalidString1) });
146+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(";", invalidString1, validString1, validString2, validString3) });
147+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(";", validString1, invalidString1, validString2, validString3) });
148+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(";", validString1, validString2, invalidString1, validString3) });
149+
dataset.Add(new[] { header1, header2, header3 }.ToList(), new[] { string.Join(";", validString1, validString2, validString3, invalidString1) });
113150

114-
// TODO: [Fact]
151+
return dataset;
152+
}
153+
}
154+
155+
[Fact]
115156
public void CookieHeaderValue_CtorThrowsOnNullName()
116157
{
117158
Assert.Throws<ArgumentNullException>(() => new CookieHeaderValue(null, "value"));
@@ -225,5 +266,41 @@ public void CookieHeaderValue_TryParseList_AcceptsValidValues(IList<CookieHeader
225266

226267
Assert.Equal(cookies, results);
227268
}
269+
270+
[Theory]
271+
[MemberData(nameof(ListOfInvalidCookieHeaderDataSet))]
272+
public void CookieHeaderValue_ParseStrictList_ThrowsForAnyInvalidValues(IList<CookieHeaderValue> cookies, string[] input)
273+
{
274+
Assert.Throws<FormatException>(() => CookieHeaderValue.ParseStrictList(input));
275+
}
276+
277+
[Theory]
278+
[MemberData(nameof(ListOfInvalidCookieHeaderDataSet))]
279+
public void CookieHeaderValue_TryParseStrictList_FailsForAnyInvalidValues(IList<CookieHeaderValue> cookies, string[] input)
280+
{
281+
IList<CookieHeaderValue> results;
282+
bool result = CookieHeaderValue.TryParseStrictList(input, out results);
283+
Assert.False(result);
284+
Assert.Null(results);
285+
}
286+
287+
[Theory]
288+
[MemberData(nameof(ListOfInvalidCookieHeaderDataSet))]
289+
public void CookieHeaderValue_ParseList_ExcludesInvalidValues(IList<CookieHeaderValue> cookies, string[] input)
290+
{
291+
var results = CookieHeaderValue.ParseList(input);
292+
// ParseList aways returns a list, even if empty. TryParseList may return null (via out).
293+
Assert.Equal(cookies ?? new List<CookieHeaderValue>(), results);
294+
}
295+
296+
[Theory]
297+
[MemberData(nameof(ListOfInvalidCookieHeaderDataSet))]
298+
public void CookieHeaderValue_TryParseList_ExcludesInvalidValues(IList<CookieHeaderValue> cookies, string[] input)
299+
{
300+
IList<CookieHeaderValue> results;
301+
bool result = CookieHeaderValue.TryParseList(input, out results);
302+
Assert.Equal(cookies, results);
303+
Assert.Equal(cookies?.Count > 0, result);
304+
}
228305
}
229306
}

0 commit comments

Comments
 (0)