Fix chunkedCoding. (#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.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c12d2cb..2c50995 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 3.1.1
+
+* Fix a logic bug in the `chunkedCoding` codec. It had been producing invalid
+ output and rejecting valid input.
+
## 3.1.0
* Add `chunkedCoding`, a `Codec` that supports encoding and decoding the
diff --git a/lib/src/chunked_coding/decoder.dart b/lib/src/chunked_coding/decoder.dart
index e2a27fc..18530bc 100644
--- a/lib/src/chunked_coding/decoder.dart
+++ b/lib/src/chunked_coding/decoder.dart
@@ -70,7 +70,7 @@
/// describe the character in the exception text.
assertCurrentChar(int char, String name) {
if (bytes[start] != char) {
- throw new FormatException("Expected LF.", bytes, start);
+ throw new FormatException("Expected $name.", bytes, start);
}
}
@@ -85,7 +85,7 @@
case _State.size:
if (bytes[start] == $cr) {
- _state = _State.beforeLF;
+ _state = _State.sizeBeforeLF;
} else {
// Shift four bits left since a single hex digit contains four bits
// of information.
@@ -94,7 +94,7 @@
start++;
break;
- case _State.beforeLF:
+ case _State.sizeBeforeLF:
assertCurrentChar($lf, "LF");
_state = _size == 0 ? _State.endBeforeCR : _State.body;
start++;
@@ -105,7 +105,19 @@
buffer.addAll(bytes, start, chunkEnd);
_size -= chunkEnd - start;
start = chunkEnd;
- if (_size == 0) _state = _State.boundary;
+ if (_size == 0) _state = _State.bodyBeforeCR;
+ break;
+
+ case _State.bodyBeforeCR:
+ assertCurrentChar($cr, "CR");
+ _state = _State.bodyBeforeLF;
+ start++;
+ break;
+
+ case _State.bodyBeforeLF:
+ assertCurrentChar($lf, "LF");
+ _state = _State.boundary;
+ start++;
break;
case _State.endBeforeCR:
@@ -115,7 +127,7 @@
break;
case _State.endBeforeLF:
- assertCurrentChar($lf, "CR");
+ assertCurrentChar($lf, "LF");
_state = _State.end;
start++;
break;
@@ -161,8 +173,6 @@
/// An enumeration of states that [_Sink] can exist in when decoded a chunked
/// message.
-///
-/// [_SizeState], [_CRState], and [_ChunkState] have additional data attached.
class _State {
/// The parser has fully parsed one chunk and is expecting the header for the
/// next chunk.
@@ -173,20 +183,32 @@
/// The parser has parsed at least one digit of the chunk size header, but has
/// not yet parsed the `CR LF` sequence that indicates the end of that header.
///
- /// Transitions to [beforeLF].
+ /// Transitions to [sizeBeforeLF].
static const size = const _State._("size");
/// The parser has parsed the chunk size header and the CR character after it,
/// but not the LF.
///
- /// Transitions to [body] or [endBeforeCR].
- static const beforeLF = const _State._("before LF");
+ /// Transitions to [body] or [bodyBeforeCR].
+ static const sizeBeforeLF = const _State._("size before LF");
/// The parser has parsed a chunk header and possibly some of the body, but
/// still needs to consume more bytes.
///
- /// Transitions to [boundary].
- static const body = const _State._("CR");
+ /// Transitions to [bodyBeforeCR].
+ static const body = const _State._("body");
+
+ // The parser has parsed all the bytes in a chunk body but not the CR LF
+ // sequence that follows it.
+ //
+ // Transitions to [bodyBeforeLF].
+ static const bodyBeforeCR = const _State._("body before CR");
+
+ // The parser has parsed all the bytes in a chunk body and the CR that follows
+ // it, but not the LF after that.
+ //
+ // Transitions to [bounday].
+ static const bodyBeforeLF = const _State._("body before LF");
/// The parser has parsed the final empty chunk but not the CR LF sequence
/// that follows it.
diff --git a/lib/src/chunked_coding/encoder.dart b/lib/src/chunked_coding/encoder.dart
index c724700..183ecfc 100644
--- a/lib/src/chunked_coding/encoder.dart
+++ b/lib/src/chunked_coding/encoder.dart
@@ -59,12 +59,18 @@
var sizeInHex = size.toRadixString(16);
var footerSize = isLast ? _doneChunk.length : 0;
- // Add 2 for the CRLF sequence that follows the size header.
- var list = new Uint8List(sizeInHex.length + 2 + size + footerSize);
+ // Add 4 for the CRLF sequences that follow the size header and the bytes.
+ var list = new Uint8List(sizeInHex.length + 4 + size + footerSize);
list.setRange(0, sizeInHex.length, sizeInHex.codeUnits);
- list[sizeInHex.length] = $cr;
- list[sizeInHex.length + 1] = $lf;
- list.setRange(sizeInHex.length + 2, list.length - footerSize, bytes, start);
+
+ var cursor = sizeInHex.length;
+ list[cursor++] = $cr;
+ list[cursor++] = $lf;
+ list.setRange(cursor, cursor + end - start, bytes, start);
+ cursor += end - start;
+ list[cursor++] = $cr;
+ list[cursor++] = $lf;
+
if (isLast) {
list.setRange(list.length - footerSize, list.length, _doneChunk);
}
diff --git a/test/chunked_coding_test.dart b/test/chunked_coding_test.dart
index 29fce25..241cfc9 100644
--- a/test/chunked_coding_test.dart
+++ b/test/chunked_coding_test.dart
@@ -14,7 +14,7 @@
group("encoder", () {
test("adds a header to the chunk of bytes", () {
expect(chunkedCoding.encode([1, 2, 3]),
- equals([$3, $cr, $lf, 1, 2, 3, $0, $cr, $lf, $cr, $lf]));
+ equals([$3, $cr, $lf, 1, 2, 3, $cr, $lf, $0, $cr, $lf, $cr, $lf]));
});
test("uses hex for chunk size", () {
@@ -22,7 +22,7 @@
expect(chunkedCoding.encode(data),
equals([$a, $7, $cr, $lf]
..addAll(data)
- ..addAll([$0, $cr, $lf, $cr, $lf])));
+ ..addAll([$cr, $lf, $0, $cr, $lf, $cr, $lf])));
});
test("just generates a footer for an empty input", () {
@@ -41,18 +41,18 @@
test("adds headers to each chunk of bytes", () {
sink.add([1, 2, 3, 4]);
- expect(results, equals([[$4, $cr, $lf, 1, 2, 3, 4]]));
+ expect(results, equals([[$4, $cr, $lf, 1, 2, 3, 4, $cr, $lf]]));
sink.add([5, 6, 7]);
expect(results, equals([
- [$4, $cr, $lf, 1, 2, 3, 4],
- [$3, $cr, $lf, 5, 6, 7],
+ [$4, $cr, $lf, 1, 2, 3, 4, $cr, $lf],
+ [$3, $cr, $lf, 5, 6, 7, $cr, $lf],
]));
sink.close();
expect(results, equals([
- [$4, $cr, $lf, 1, 2, 3, 4],
- [$3, $cr, $lf, 5, 6, 7],
+ [$4, $cr, $lf, 1, 2, 3, 4, $cr, $lf],
+ [$3, $cr, $lf, 5, 6, 7, $cr, $lf],
[$0, $cr, $lf, $cr, $lf],
]));
});
@@ -62,15 +62,15 @@
expect(results, equals([[]]));
sink.add([1, 2, 3]);
- expect(results, equals([[], [$3, $cr, $lf, 1, 2, 3]]));
+ expect(results, equals([[], [$3, $cr, $lf, 1, 2, 3, $cr, $lf]]));
sink.add([]);
- expect(results, equals([[], [$3, $cr, $lf, 1, 2, 3], []]));
+ expect(results, equals([[], [$3, $cr, $lf, 1, 2, 3, $cr, $lf], []]));
sink.close();
expect(results, equals([
[],
- [$3, $cr, $lf, 1, 2, 3],
+ [$3, $cr, $lf, 1, 2, 3, $cr, $lf],
[],
[$0, $cr, $lf, $cr, $lf],
]));
@@ -79,7 +79,7 @@
group("addSlice()", () {
test("adds bytes from the specified slice", () {
sink.addSlice([1, 2, 3, 4, 5], 1, 4, false);
- expect(results, equals([[$3, $cr, $lf, 2, 3, 4]]));
+ expect(results, equals([[$3, $cr, $lf, 2, 3, 4, $cr, $lf]]));
});
test("doesn't add a header if the slice is empty", () {
@@ -89,8 +89,8 @@
test("adds a footer if isLast is true", () {
sink.addSlice([1, 2, 3, 4, 5], 1, 4, true);
- expect(results,
- equals([[$3, $cr, $lf, 2, 3, 4, $0, $cr, $lf, $cr, $lf]]));
+ expect(results, equals(
+ [[$3, $cr, $lf, 2, 3, 4, $cr, $lf, $0, $cr, $lf, $cr, $lf]]));
// Setting isLast shuld close the sink.
expect(() => sink.add([]), throwsStateError);
@@ -119,8 +119,8 @@
group("decoder", () {
test("parses chunked data", () {
expect(chunkedCoding.decode([
- $3, $cr, $lf, 1, 2, 3,
- $4, $cr, $lf, 4, 5, 6, 7,
+ $3, $cr, $lf, 1, 2, 3, $cr, $lf,
+ $4, $cr, $lf, 4, 5, 6, 7, $cr, $lf,
$0, $cr, $lf, $cr, $lf,
]), equals([1, 2, 3, 4, 5, 6, 7]));
});
@@ -130,7 +130,7 @@
expect(
chunkedCoding.decode([$a, $7, $cr, $lf]
..addAll(data)
- ..addAll([$0, $cr, $lf, $cr, $lf])),
+ ..addAll([$cr, $lf, $0, $cr, $lf, $cr, $lf])),
equals(data));
});
@@ -139,7 +139,7 @@
expect(
chunkedCoding.decode([$A, $7, $cr, $lf]
..addAll(data)
- ..addAll([$0, $cr, $lf, $cr, $lf])),
+ ..addAll([$cr, $lf, $0, $cr, $lf, $cr, $lf])),
equals(data));
});
@@ -170,11 +170,21 @@
throwsFormatException);
});
- test("that ends at a chunk boundary", () {
+ test("that ends after a chunk's bytes", () {
expect(() => chunkedCoding.decode([$1, $cr, $lf, 1]),
throwsFormatException);
});
+ test("that ends after a chunk's CR", () {
+ expect(() => chunkedCoding.decode([$1, $cr, $lf, 1, $cr]),
+ throwsFormatException);
+ });
+
+ test("that ends atfter a chunk's LF", () {
+ expect(() => chunkedCoding.decode([$1, $cr, $lf, 1, $cr, $lf]),
+ throwsFormatException);
+ });
+
test("that ends after the empty chunk", () {
expect(() => chunkedCoding.decode([$0, $cr, $lf]),
throwsFormatException);
@@ -208,10 +218,10 @@
});
test("decodes each chunk of bytes", () {
- sink.add([$4, $cr, $lf, 1, 2, 3, 4]);
+ sink.add([$4, $cr, $lf, 1, 2, 3, 4, $cr, $lf]);
expect(results, equals([[1, 2, 3, 4]]));
- sink.add([$3, $cr, $lf, 5, 6, 7]);
+ sink.add([$3, $cr, $lf, 5, 6, 7, $cr, $lf]);
expect(results, equals([[1, 2, 3, 4], [5, 6, 7]]));
sink.add([$0, $cr, $lf, $cr, $lf]);
@@ -223,7 +233,7 @@
sink.add([]);
expect(results, isEmpty);
- sink.add([$3, $cr, $lf, 1, 2, 3]);
+ sink.add([$3, $cr, $lf, 1, 2, 3, $cr, $lf]);
expect(results, equals([[1, 2, 3]]));
sink.add([]);
@@ -281,6 +291,30 @@
expect(results, equals([[1, 2], [3]]));
});
+ test("after all bytes", () {
+ sink.add([$3, $cr, $lf, 1, 2, 3]);
+ expect(results, equals([[1, 2, 3]]));
+
+ sink.add([$cr, $lf, $3, $cr, $lf, 2, 3, 4, $cr, $lf]);
+ expect(results, equals([[1, 2, 3], [2, 3, 4]]));
+ });
+
+ test("after a post-chunk CR", () {
+ sink.add([$3, $cr, $lf, 1, 2, 3, $cr]);
+ expect(results, equals([[1, 2, 3]]));
+
+ sink.add([$lf, $3, $cr, $lf, 2, 3, 4, $cr, $lf]);
+ expect(results, equals([[1, 2, 3], [2, 3, 4]]));
+ });
+
+ test("after a post-chunk LF", () {
+ sink.add([$3, $cr, $lf, 1, 2, 3, $cr, $lf]);
+ expect(results, equals([[1, 2, 3]]));
+
+ sink.add([$3, $cr, $lf, 2, 3, 4, $cr, $lf]);
+ expect(results, equals([[1, 2, 3], [2, 3, 4]]));
+ });
+
test("after empty chunk size", () {
sink.add([$0]);
expect(results, isEmpty);
