Skip to content

Commit

Permalink
Revert "Make HTTP headers use a growing buffer, not a fixed-size 8K o…
Browse files Browse the repository at this point in the history 
…ne."

This reverts commit 5119795.

BUG=
R=dgrove@google.com

Review-Url: https://codereview.chromium.org/2650583005 .
  • Loading branch information
@keertip
keertip committed Jan 23, 2017
1 parent b3c4f58 commit 1d6bd52
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 100 deletions.
65 changes: 20 additions & 45 deletions sdk/lib/io/bytes_builder.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,22 +83,24 @@ class _CopyingBytesBuilder implements BytesBuilder {
// Start with 1024 bytes.
static const int _INIT_SIZE = 1024;

static final _emptyList = new Uint8List(0);

int _length = 0;
Uint8List _buffer;

_CopyingBytesBuilder([int initialCapacity = 0])
: _buffer = (initialCapacity <= 0)
? _emptyList
: new Uint8List(_pow2roundup(initialCapacity));

void add(List<int> bytes) {
int bytesLength = bytes.length;
if (bytesLength == 0) return;
int required = _length + bytesLength;
if (_buffer.length < required) {
_grow(required);
if (_buffer == null) {
int size = _pow2roundup(required);
size = max(size, _INIT_SIZE);
_buffer = new Uint8List(size);
} else if (_buffer.length < required) {
// We will create a list in the range of 2-4 times larger than
// required.
int size = _pow2roundup(required) * 2;
var newBuffer = new Uint8List(size);
newBuffer.setRange(0, _buffer.length, _buffer);
_buffer = newBuffer;
}
assert(_buffer.length >= required);
if (bytes is Uint8List) {
Expand All @@ -111,40 +113,17 @@ class _CopyingBytesBuilder implements BytesBuilder {
_length = required;
}

void addByte(int byte) {
if (_buffer.length == _length) {
// The grow algorithm always at least doubles.
// If we added one to _length it would quadruple unnecessarily.
_grow(_length);
}
assert(_buffer.length > _length);
_buffer[_length] = byte;
_length++;
}

void _grow(int required) {
// We will create a list in the range of 2-4 times larger than
// required.
int newSize = required * 2;
if (newSize < _INIT_SIZE) {
newSize = _INIT_SIZE;
} else {
newSize = _pow2roundup(newSize);
}
var newBuffer = new Uint8List(newSize);
newBuffer.setRange(0, _buffer.length, _buffer);
_buffer = newBuffer;
}
void addByte(int byte) { add([byte]); }

List<int> takeBytes() {
if (_length == 0) return _emptyList;
if (_buffer == null) return new Uint8List(0);
var buffer = new Uint8List.view(_buffer.buffer, 0, _length);
clear();
return buffer;
}

List<int> toBytes() {
if (_length == 0) return _emptyList;
if (_buffer == null) return new Uint8List(0);
return new Uint8List.fromList(
new Uint8List.view(_buffer.buffer, 0, _length));
}
Expand All @@ -157,11 +136,10 @@ class _CopyingBytesBuilder implements BytesBuilder {

void clear() {
_length = 0;
_buffer = _emptyList;
_buffer = null;
}

static int _pow2roundup(int x) {
assert(x > 0);
int _pow2roundup(int x) {
--x;
x |= x >> 1;
x |= x >> 2;
Expand All @@ -188,15 +166,12 @@ class _BytesBuilder implements BytesBuilder {
_length += typedBytes.length;
}

void addByte(int byte) {
_chunks.add(new Uint8List(1)..[0] = byte);
_length++;
}
void addByte(int byte) { add([byte]); }

List<int> takeBytes() {
if (_length == 0) return _CopyingBytesBuilder._emptyList;
if (_chunks.length == 0) return new Uint8List(0);
if (_chunks.length == 1) {
var buffer = _chunks[0];
var buffer = _chunks.single;
clear();
return buffer;
}
Expand All @@ -211,7 +186,7 @@ class _BytesBuilder implements BytesBuilder {
}

List<int> toBytes() {
if (_length == 0) return _CopyingBytesBuilder._emptyList;
if (_chunks.length == 0) return new Uint8List(0);
var buffer = new Uint8List(_length);
int offset = 0;
for (var chunk in _chunks) {
Expand Down
38 changes: 24 additions & 14 deletions sdk/lib/io/http_headers.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,32 +465,42 @@ class _HttpHeaders implements HttpHeaders {
_mutable = false;
}

void _build(BytesBuilder builder) {
int _write(Uint8List buffer, int offset) {
void write(List<int> bytes) {
int len = bytes.length;
for (int i = 0; i < len; i++) {
buffer[offset + i] = bytes[i];
}
offset += len;
}

// Format headers.
for (String name in _headers.keys) {
List<String> values = _headers[name];
bool fold = _foldHeader(name);
var nameData = name.codeUnits;
builder.add(nameData);
builder.addByte(_CharCode.COLON);
builder.addByte(_CharCode.SP);
write(nameData);
buffer[offset++] = _CharCode.COLON;
buffer[offset++] = _CharCode.SP;
for (int i = 0; i < values.length; i++) {
if (i > 0) {
if (fold) {
builder.addByte(_CharCode.COMMA);
builder.addByte(_CharCode.SP);
buffer[offset++] = _CharCode.COMMA;
buffer[offset++] = _CharCode.SP;
} else {
builder.addByte(_CharCode.CR);
builder.addByte(_CharCode.LF);
builder.add(nameData);
builder.addByte(_CharCode.COLON);
builder.addByte(_CharCode.SP);
buffer[offset++] = _CharCode.CR;
buffer[offset++] = _CharCode.LF;
write(nameData);
buffer[offset++] = _CharCode.COLON;
buffer[offset++] = _CharCode.SP;
}
}
builder.add(values[i].codeUnits);
write(values[i].codeUnits);
}
builder.addByte(_CharCode.CR);
builder.addByte(_CharCode.LF);
buffer[offset++] = _CharCode.CR;
buffer[offset++] = _CharCode.LF;
}
return offset;
}

String toString() {
Expand Down
111 changes: 70 additions & 41 deletions sdk/lib/io/http_impl.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,20 +574,29 @@ class _HttpResponse extends _HttpOutboundMessage<HttpResponse>
}

void _writeHeader() {
BytesBuilder buffer = new _CopyingBytesBuilder(_OUTGOING_BUFFER_SIZE);
Uint8List buffer = new Uint8List(_OUTGOING_BUFFER_SIZE);
int offset = 0;

void write(List<int> bytes) {
int len = bytes.length;
for (int i = 0; i < len; i++) {
buffer[offset + i] = bytes[i];
}
offset += len;
}

// Write status line.
if (headers.protocolVersion == "1.1") {
buffer.add(_Const.HTTP11);
write(_Const.HTTP11);
} else {
buffer.add(_Const.HTTP10);
write(_Const.HTTP10);
}
buffer.addByte(_CharCode.SP);
buffer.add(statusCode.toString().codeUnits);
buffer.addByte(_CharCode.SP);
buffer.add(reasonPhrase.codeUnits);
buffer.addByte(_CharCode.CR);
buffer.addByte(_CharCode.LF);
buffer[offset++] = _CharCode.SP;
write(statusCode.toString().codeUnits);
buffer[offset++] = _CharCode.SP;
write(reasonPhrase.codeUnits);
buffer[offset++] = _CharCode.CR;
buffer[offset++] = _CharCode.LF;

var session = _httpRequest._session;
if (session != null && !session._destroyed) {
Expand Down Expand Up @@ -621,11 +630,10 @@ class _HttpResponse extends _HttpOutboundMessage<HttpResponse>
headers._finalize();

// Write headers.
headers._build(buffer);
buffer.addByte(_CharCode.CR);
buffer.addByte(_CharCode.LF);
Uint8List headerBytes = buffer.takeBytes();
_outgoing.setHeader(headerBytes, headerBytes.length);
offset = headers._write(buffer, offset);
buffer[offset++] = _CharCode.CR;
buffer[offset++] = _CharCode.LF;
_outgoing.setHeader(buffer, offset);
}

String _findReasonPhrase(int statusCode) {
Expand Down Expand Up @@ -813,18 +821,27 @@ class _HttpClientRequest extends _HttpOutboundMessage<HttpClientResponse>
}

void _writeHeader() {
BytesBuilder buffer = new _CopyingBytesBuilder(_OUTGOING_BUFFER_SIZE);
Uint8List buffer = new Uint8List(_OUTGOING_BUFFER_SIZE);
int offset = 0;

void write(List<int> bytes) {
int len = bytes.length;
for (int i = 0; i < len; i++) {
buffer[offset + i] = bytes[i];
}
offset += len;
}

// Write the request method.
buffer.add(method.codeUnits);
buffer.addByte(_CharCode.SP);
write(method.codeUnits);
buffer[offset++] = _CharCode.SP;
// Write the request URI.
buffer.add(_requestUri().codeUnits);
buffer.addByte(_CharCode.SP);
write(_requestUri().codeUnits);
buffer[offset++] = _CharCode.SP;
// Write HTTP/1.1.
buffer.add(_Const.HTTP11);
buffer.addByte(_CharCode.CR);
buffer.addByte(_CharCode.LF);
write(_Const.HTTP11);
buffer[offset++] = _CharCode.CR;
buffer[offset++] = _CharCode.LF;

// Add the cookies to the headers.
if (!cookies.isEmpty) {
Expand All @@ -839,11 +856,10 @@ class _HttpClientRequest extends _HttpOutboundMessage<HttpClientResponse>
headers._finalize();

// Write headers.
headers._build(buffer);
buffer.addByte(_CharCode.CR);
buffer.addByte(_CharCode.LF);
Uint8List headerBytes = buffer.takeBytes();
_outgoing.setHeader(headerBytes, headerBytes.length);
offset = headers._write(buffer, offset);
buffer[offset++] = _CharCode.CR;
buffer[offset++] = _CharCode.LF;
_outgoing.setHeader(buffer, offset);
}
}

Expand Down Expand Up @@ -919,6 +935,18 @@ class _HttpOutgoing implements StreamConsumer<List<int>> {
// Returns either a future or 'null', if it was able to write headers
// immediately.
Future writeHeaders({bool drainRequest: true, bool setOutgoing: true}) {
Future write() {
try {
outbound._writeHeader();
} catch (_) {
// Headers too large.
return new Future.error(new HttpException(
"Headers size exceeded the of '$_OUTGOING_BUFFER_SIZE'"
" bytes"));
}
return null;
}

if (headersWritten) return null;
headersWritten = true;
Future drainFuture;
Expand Down Expand Up @@ -947,22 +975,22 @@ class _HttpOutgoing implements StreamConsumer<List<int>> {
} else {
drainRequest = false;
}
if (!ignoreBody) {
if (setOutgoing) {
int contentLength = outbound.headers.contentLength;
if (outbound.headers.chunkedTransferEncoding) {
chunked = true;
if (gzip) this.gzip = true;
} else if (contentLength >= 0) {
this.contentLength = contentLength;
}
}
if (drainFuture != null) {
return drainFuture.then((_) => outbound._writeHeader());
if (ignoreBody) {
return write();
}
if (setOutgoing) {
int contentLength = outbound.headers.contentLength;
if (outbound.headers.chunkedTransferEncoding) {
chunked = true;
if (gzip) this.gzip = true;
} else if (contentLength >= 0) {
this.contentLength = contentLength;
}
}
outbound._writeHeader();
return null;
if (drainFuture != null) {
return drainFuture.then((_) => write());
}
return write();
}


Expand Down Expand Up @@ -1132,6 +1160,7 @@ class _HttpOutgoing implements StreamConsumer<List<int>> {

void setHeader(List<int> data, int length) {
assert(_length == 0);
assert(data.length == _OUTGOING_BUFFER_SIZE);
_buffer = data;
_length = length;
}
Expand Down
17 changes: 17 additions & 0 deletions tests/standalone/io/http_client_request_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,25 @@ void testBadResponseClose() {
}


void testBadHeaders() {
asyncStart();
testClientRequest((request) {
var value = "a";
for (int i = 0; i < 8 * 1024; i++) {
value += 'a';
}
request.headers.set('name', value);
request.done.catchError((error) {
asyncEnd();
}, test: (e) => e is HttpException);
return request.close();
});
}


void main() {
testResponseDone();
testBadResponseAdd();
testBadResponseClose();
testBadHeaders();
}

0 comments on commit 1d6bd52

Please sign in to comment.