Skip to content

Commit

Permalink
Correctly report error when HTTP connection was closed before Content…
Browse files Browse the repository at this point in the history
…-Length was fulfilled.

Instead of reporting checksum mismatch, the downloader will now report

```
Connection was closed before Content-Length was fulfilled. Bytes read %s but wanted %s
```

when HTTP connection is closed before Content-Length is fulfilled.

Closes bazelbuild#13918.

PiperOrigin-RevId: 395411418
  • Loading branch information
coeuvre authored and copybara-github committed Sep 8, 2021
1 parent d1da696 commit 0e90eab
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
// Copyright 2021 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.devtools.build.lib.bazel.repository.downloader;

import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import java.io.IOException;
import java.io.InputStream;

/**
* Input stream that guarantees its contents match a size.
*
* <p>This class is not thread safe, but it is safe to message pass this object between threads.
*/
@ThreadCompatible
public class CheckContentLengthInputStream extends InputStream {
private final InputStream delegate;
private final long expectedSize;
private long actualSize;

public CheckContentLengthInputStream(InputStream delegate, long expectedSize) {
this.delegate = delegate;
this.expectedSize = expectedSize;
}

@Override
public int read() throws IOException {
int result = delegate.read();
if (result == -1) {
checkContentLength();
} else {
actualSize += result;
}
return result;
}

@Override
public int read(byte[] buffer, int offset, int length) throws IOException {
int amount = delegate.read(buffer, offset, length);
if (amount == -1) {
checkContentLength();
} else {
actualSize += amount;
}
return amount;
}

@Override
public int available() throws IOException {
return delegate.available();
}

@Override
public void close() throws IOException {
delegate.close();
checkContentLength();
}

private void checkContentLength() throws IOException {
if (actualSize < expectedSize) {
throw new UnrecoverableHttpException(
String.format(
"Connection was closed before Content-Length was fulfilled. Bytes read %s but wanted"
+ " %s",
actualSize, expectedSize));
} else if (actualSize > expectedSize) {
throw new UnrecoverableHttpException(
String.format(
"Received more bytes than Content-Length. Bytes read %s but wanted %s",
actualSize, expectedSize));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,16 @@ HttpStream create(
stream = retrier;
}

try {
String contentLength = connection.getHeaderField("Content-Length");
if (contentLength != null) {
long expectedSize = Long.parseLong(contentLength);
stream = new CheckContentLengthInputStream(stream, expectedSize);
}
} catch (NumberFormatException ignored) {
// ignored
}

stream = progressInputStreamFactory.create(stream, connection.getURL(), originalUrl);

// Determine if we need to transparently gunzip. See RFC2616 § 3.5 and § 14.11. Please note
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.bazel.repository.downloader.DownloaderTestUtils.makeUrl;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
Expand Down Expand Up @@ -205,6 +206,56 @@ public void bigDataWithInvalidChecksum_throwsIOExceptionAfterCreateOnEof() throw
}
}

@Test
public void bigDataTruncated_throwsExpectedError() throws Exception {
byte[] bigData = new byte[HttpStream.PRECHECK_BYTES + 70001];
randoCalrissian.nextBytes(bigData);
when(connection.getHeaderField("Content-Length"))
.thenReturn(String.valueOf(bigData.length + 1));
when(connection.getInputStream()).thenReturn(new ByteArrayInputStream(bigData));

IOException thrown =
assertThrows(
IOException.class,
() -> {
try (HttpStream stream =
streamFactory.create(
connection,
AURL,
makeChecksum(Hashing.sha256().hashBytes(bigData).toString()),
reconnector)) {
ByteStreams.exhaust(stream);
}
});

assertThat(thrown).hasMessageThat().contains("Content-Length");
}

@Test
public void bigDataOverflowed_throwsExpectedError() throws Exception {
byte[] bigData = new byte[HttpStream.PRECHECK_BYTES + 70001];
randoCalrissian.nextBytes(bigData);
when(connection.getHeaderField("Content-Length"))
.thenReturn(String.valueOf(bigData.length - 1));
when(connection.getInputStream()).thenReturn(new ByteArrayInputStream(bigData));

IOException thrown =
assertThrows(
IOException.class,
() -> {
try (HttpStream stream =
streamFactory.create(
connection,
AURL,
makeChecksum(Hashing.sha256().hashBytes(bigData).toString()),
reconnector)) {
ByteStreams.exhaust(stream);
}
});

assertThat(thrown).hasMessageThat().contains("Content-Length");
}

@Test
public void httpServerSaidGzippedButNotGzipped_throwsZipExceptionInCreate() throws Exception {
when(connection.getURL()).thenReturn(AURL);
Expand Down

0 comments on commit 0e90eab

Please sign in to comment.