Skip to content

Commit 317b58a

Browse files
nex3nex3
authored
Fix chunkedCoding. (flutter#9)
I misread the spec; it turns out there's a CR LF sequence after the body bytes as well as after the size header.
1 parent 50e55f6 commit 317b58a

File tree

4 files changed

+105
-38
lines changed

4 files changed

+105
-38
lines changed

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
## 3.1.1
2+
3+
* Fix a logic bug in the `chunkedCoding` codec. It had been producing invalid
4+
output and rejecting valid input.
5+
16
## 3.1.0
27

38
* Add `chunkedCoding`, a `Codec` that supports encoding and decoding the

‎lib/src/chunked_coding/decoder.dart‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class _Sink extends ByteConversionSinkBase {
7070
/// describe the character in the exception text.
7171
assertCurrentChar(int char, String name) {
7272
if (bytes[start] != char) {
73-
throw new FormatException("Expected LF.", bytes, start);
73+
throw new FormatException("Expected $name.", bytes, start);
7474
}
7575
}
7676

@@ -85,7 +85,7 @@ class _Sink extends ByteConversionSinkBase {
8585

8686
case _State.size:
8787
if (bytes[start] == $cr) {
88-
_state = _State.beforeLF;
88+
_state = _State.sizeBeforeLF;
8989
} else {
9090
// Shift four bits left since a single hex digit contains four bits
9191
// of information.
@@ -94,7 +94,7 @@ class _Sink extends ByteConversionSinkBase {
9494
start++;
9595
break;
9696

97-
case _State.beforeLF:
97+
case _State.sizeBeforeLF:
9898
assertCurrentChar($lf, "LF");
9999
_state = _size == 0 ? _State.endBeforeCR : _State.body;
100100
start++;
@@ -105,7 +105,19 @@ class _Sink extends ByteConversionSinkBase {
105105
buffer.addAll(bytes, start, chunkEnd);
106106
_size -= chunkEnd - start;
107107
start = chunkEnd;
108-
if (_size == 0) _state = _State.boundary;
108+
if (_size == 0) _state = _State.bodyBeforeCR;
109+
break;
110+
111+
case _State.bodyBeforeCR:
112+
assertCurrentChar($cr, "CR");
113+
_state = _State.bodyBeforeLF;
114+
start++;
115+
break;
116+
117+
case _State.bodyBeforeLF:
118+
assertCurrentChar($lf, "LF");
119+
_state = _State.boundary;
120+
start++;
109121
break;
110122

111123
case _State.endBeforeCR:
@@ -115,7 +127,7 @@ class _Sink extends ByteConversionSinkBase {
115127
break;
116128

117129
case _State.endBeforeLF:
118-
assertCurrentChar($lf, "CR");
130+
assertCurrentChar($lf, "LF");
119131
_state = _State.end;
120132
start++;
121133
break;
@@ -161,8 +173,6 @@ class _Sink extends ByteConversionSinkBase {
161173

162174
/// An enumeration of states that [_Sink] can exist in when decoded a chunked
163175
/// message.
164-
///
165-
/// [_SizeState], [_CRState], and [_ChunkState] have additional data attached.
166176
class _State {
167177
/// The parser has fully parsed one chunk and is expecting the header for the
168178
/// next chunk.
@@ -173,20 +183,32 @@ class _State {
173183
/// The parser has parsed at least one digit of the chunk size header, but has
174184
/// not yet parsed the `CR LF` sequence that indicates the end of that header.
175185
///
176-
/// Transitions to [beforeLF].
186+
/// Transitions to [sizeBeforeLF].
177187
static const size = const _State._("size");
178188

179189
/// The parser has parsed the chunk size header and the CR character after it,
180190
/// but not the LF.
181191
///
182-
/// Transitions to [body] or [endBeforeCR].
183-
static const beforeLF = const _State._("before LF");
192+
/// Transitions to [body] or [bodyBeforeCR].
193+
static const sizeBeforeLF = const _State._("size before LF");
184194

185195
/// The parser has parsed a chunk header and possibly some of the body, but
186196
/// still needs to consume more bytes.
187197
///
188-
/// Transitions to [boundary].
189-
static const body = const _State._("CR");
198+
/// Transitions to [bodyBeforeCR].
199+
static const body = const _State._("body");
200+
201+
// The parser has parsed all the bytes in a chunk body but not the CR LF
202+
// sequence that follows it.
203+
//
204+
// Transitions to [bodyBeforeLF].
205+
static const bodyBeforeCR = const _State._("body before CR");
206+
207+
// The parser has parsed all the bytes in a chunk body and the CR that follows
208+
// it, but not the LF after that.
209+
//
210+
// Transitions to [bounday].
211+
static const bodyBeforeLF = const _State._("body before LF");
190212

191213
/// The parser has parsed the final empty chunk but not the CR LF sequence
192214
/// that follows it.

‎lib/src/chunked_coding/encoder.dart‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,18 @@ List<int> _convert(List<int> bytes, int start, int end, {bool isLast: false}) {
5959
var sizeInHex = size.toRadixString(16);
6060
var footerSize = isLast ? _doneChunk.length : 0;
6161

62-
// Add 2 for the CRLF sequence that follows the size header.
63-
var list = new Uint8List(sizeInHex.length + 2 + size + footerSize);
62+
// Add 4 for the CRLF sequences that follow the size header and the bytes.
63+
var list = new Uint8List(sizeInHex.length + 4 + size + footerSize);
6464
list.setRange(0, sizeInHex.length, sizeInHex.codeUnits);
65-
list[sizeInHex.length] = $cr;
66-
list[sizeInHex.length + 1] = $lf;
67-
list.setRange(sizeInHex.length + 2, list.length - footerSize, bytes, start);
65+
66+
var cursor = sizeInHex.length;
67+
list[cursor++] = $cr;
68+
list[cursor++] = $lf;
69+
list.setRange(cursor, cursor + end - start, bytes, start);
70+
cursor += end - start;
71+
list[cursor++] = $cr;
72+
list[cursor++] = $lf;
73+
6874
if (isLast) {
6975
list.setRange(list.length - footerSize, list.length, _doneChunk);
7076
}

‎test/chunked_coding_test.dart‎

Lines changed: 55 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@ void main() {
1414
group("encoder", () {
1515
test("adds a header to the chunk of bytes", () {
1616
expect(chunkedCoding.encode([1, 2, 3]),
17-
equals([$3, $cr, $lf, 1, 2, 3, $0, $cr, $lf, $cr, $lf]));
17+
equals([$3, $cr, $lf, 1, 2, 3, $cr, $lf, $0, $cr, $lf, $cr, $lf]));
1818
});
1919

2020
test("uses hex for chunk size", () {
2121
var data = new Iterable.generate(0xA7).toList();
2222
expect(chunkedCoding.encode(data),
2323
equals([$a, $7, $cr, $lf]
2424
..addAll(data)
25-
..addAll([$0, $cr, $lf, $cr, $lf])));
25+
..addAll([$cr, $lf, $0, $cr, $lf, $cr, $lf])));
2626
});
2727

2828
test("just generates a footer for an empty input", () {
@@ -41,18 +41,18 @@ void main() {
4141

4242
test("adds headers to each chunk of bytes", () {
4343
sink.add([1, 2, 3, 4]);
44-
expect(results, equals([[$4, $cr, $lf, 1, 2, 3, 4]]));
44+
expect(results, equals([[$4, $cr, $lf, 1, 2, 3, 4, $cr, $lf]]));
4545

4646
sink.add([5, 6, 7]);
4747
expect(results, equals([
48-
[$4, $cr, $lf, 1, 2, 3, 4],
49-
[$3, $cr, $lf, 5, 6, 7],
48+
[$4, $cr, $lf, 1, 2, 3, 4, $cr, $lf],
49+
[$3, $cr, $lf, 5, 6, 7, $cr, $lf],
5050
]));
5151

5252
sink.close();
5353
expect(results, equals([
54-
[$4, $cr, $lf, 1, 2, 3, 4],
55-
[$3, $cr, $lf, 5, 6, 7],
54+
[$4, $cr, $lf, 1, 2, 3, 4, $cr, $lf],
55+
[$3, $cr, $lf, 5, 6, 7, $cr, $lf],
5656
[$0, $cr, $lf, $cr, $lf],
5757
]));
5858
});
@@ -62,15 +62,15 @@ void main() {
6262
expect(results, equals([[]]));
6363

6464
sink.add([1, 2, 3]);
65-
expect(results, equals([[], [$3, $cr, $lf, 1, 2, 3]]));
65+
expect(results, equals([[], [$3, $cr, $lf, 1, 2, 3, $cr, $lf]]));
6666

6767
sink.add([]);
68-
expect(results, equals([[], [$3, $cr, $lf, 1, 2, 3], []]));
68+
expect(results, equals([[], [$3, $cr, $lf, 1, 2, 3, $cr, $lf], []]));
6969

7070
sink.close();
7171
expect(results, equals([
7272
[],
73-
[$3, $cr, $lf, 1, 2, 3],
73+
[$3, $cr, $lf, 1, 2, 3, $cr, $lf],
7474
[],
7575
[$0, $cr, $lf, $cr, $lf],
7676
]));
@@ -79,7 +79,7 @@ void main() {
7979
group("addSlice()", () {
8080
test("adds bytes from the specified slice", () {
8181
sink.addSlice([1, 2, 3, 4, 5], 1, 4, false);
82-
expect(results, equals([[$3, $cr, $lf, 2, 3, 4]]));
82+
expect(results, equals([[$3, $cr, $lf, 2, 3, 4, $cr, $lf]]));
8383
});
8484

8585
test("doesn't add a header if the slice is empty", () {
@@ -89,8 +89,8 @@ void main() {
8989

9090
test("adds a footer if isLast is true", () {
9191
sink.addSlice([1, 2, 3, 4, 5], 1, 4, true);
92-
expect(results,
93-
equals([[$3, $cr, $lf, 2, 3, 4, $0, $cr, $lf, $cr, $lf]]));
92+
expect(results, equals(
93+
[[$3, $cr, $lf, 2, 3, 4, $cr, $lf, $0, $cr, $lf, $cr, $lf]]));
9494

9595
// Setting isLast shuld close the sink.
9696
expect(() => sink.add([]), throwsStateError);
@@ -119,8 +119,8 @@ void main() {
119119
group("decoder", () {
120120
test("parses chunked data", () {
121121
expect(chunkedCoding.decode([
122-
$3, $cr, $lf, 1, 2, 3,
123-
$4, $cr, $lf, 4, 5, 6, 7,
122+
$3, $cr, $lf, 1, 2, 3, $cr, $lf,
123+
$4, $cr, $lf, 4, 5, 6, 7, $cr, $lf,
124124
$0, $cr, $lf, $cr, $lf,
125125
]), equals([1, 2, 3, 4, 5, 6, 7]));
126126
});
@@ -130,7 +130,7 @@ void main() {
130130
expect(
131131
chunkedCoding.decode([$a, $7, $cr, $lf]
132132
..addAll(data)
133-
..addAll([$0, $cr, $lf, $cr, $lf])),
133+
..addAll([$cr, $lf, $0, $cr, $lf, $cr, $lf])),
134134
equals(data));
135135
});
136136

@@ -139,7 +139,7 @@ void main() {
139139
expect(
140140
chunkedCoding.decode([$A, $7, $cr, $lf]
141141
..addAll(data)
142-
..addAll([$0, $cr, $lf, $cr, $lf])),
142+
..addAll([$cr, $lf, $0, $cr, $lf, $cr, $lf])),
143143
equals(data));
144144
});
145145

@@ -170,11 +170,21 @@ void main() {
170170
throwsFormatException);
171171
});
172172

173-
test("that ends at a chunk boundary", () {
173+
test("that ends after a chunk's bytes", () {
174174
expect(() => chunkedCoding.decode([$1, $cr, $lf, 1]),
175175
throwsFormatException);
176176
});
177177

178+
test("that ends after a chunk's CR", () {
179+
expect(() => chunkedCoding.decode([$1, $cr, $lf, 1, $cr]),
180+
throwsFormatException);
181+
});
182+
183+
test("that ends atfter a chunk's LF", () {
184+
expect(() => chunkedCoding.decode([$1, $cr, $lf, 1, $cr, $lf]),
185+
throwsFormatException);
186+
});
187+
178188
test("that ends after the empty chunk", () {
179189
expect(() => chunkedCoding.decode([$0, $cr, $lf]),
180190
throwsFormatException);
@@ -208,10 +218,10 @@ void main() {
208218
});
209219

210220
test("decodes each chunk of bytes", () {
211-
sink.add([$4, $cr, $lf, 1, 2, 3, 4]);
221+
sink.add([$4, $cr, $lf, 1, 2, 3, 4, $cr, $lf]);
212222
expect(results, equals([[1, 2, 3, 4]]));
213223

214-
sink.add([$3, $cr, $lf, 5, 6, 7]);
224+
sink.add([$3, $cr, $lf, 5, 6, 7, $cr, $lf]);
215225
expect(results, equals([[1, 2, 3, 4], [5, 6, 7]]));
216226

217227
sink.add([$0, $cr, $lf, $cr, $lf]);
@@ -223,7 +233,7 @@ void main() {
223233
sink.add([]);
224234
expect(results, isEmpty);
225235

226-
sink.add([$3, $cr, $lf, 1, 2, 3]);
236+
sink.add([$3, $cr, $lf, 1, 2, 3, $cr, $lf]);
227237
expect(results, equals([[1, 2, 3]]));
228238

229239
sink.add([]);
@@ -281,6 +291,30 @@ void main() {
281291
expect(results, equals([[1, 2], [3]]));
282292
});
283293

294+
test("after all bytes", () {
295+
sink.add([$3, $cr, $lf, 1, 2, 3]);
296+
expect(results, equals([[1, 2, 3]]));
297+
298+
sink.add([$cr, $lf, $3, $cr, $lf, 2, 3, 4, $cr, $lf]);
299+
expect(results, equals([[1, 2, 3], [2, 3, 4]]));
300+
});
301+
302+
test("after a post-chunk CR", () {
303+
sink.add([$3, $cr, $lf, 1, 2, 3, $cr]);
304+
expect(results, equals([[1, 2, 3]]));
305+
306+
sink.add([$lf, $3, $cr, $lf, 2, 3, 4, $cr, $lf]);
307+
expect(results, equals([[1, 2, 3], [2, 3, 4]]));
308+
});
309+
310+
test("after a post-chunk LF", () {
311+
sink.add([$3, $cr, $lf, 1, 2, 3, $cr, $lf]);
312+
expect(results, equals([[1, 2, 3]]));
313+
314+
sink.add([$3, $cr, $lf, 2, 3, 4, $cr, $lf]);
315+
expect(results, equals([[1, 2, 3], [2, 3, 4]]));
316+
});
317+
284318
test("after empty chunk size", () {
285319
sink.add([$0]);
286320
expect(results, isEmpty);

0 commit comments

Comments
 (0)