Kong/unirest-java

downloadMonitor return wrong bytesWritten value

TheOnlyAndLastOne opened this issue · 11 comments

when I print bytesWritten and totalBytes in my project, I found that bytesWritten is great than totalBytes in the end. by the way, the image was downloaded correctly.

image

image

ryber commented

The TotalBytes comes from the Content-Length header returned by the server, so if its wrong, thats kind of on them, I suppose maybe we could start to push it up once the actual bytes is greater than what they said it was going to be? I've also seen the header be greater than the bytes eventually written

image

The first one is download via code, the second via chrome, it seems that totalBytes is right, totalBytes comes from the Content-Length header returned by the server, so where bytesWritten comes from.

ryber commented

The running bytes are summed from a MonitoringInputStream which simply counts the bytes as it proxies the original InputStream.

There are several unit tests in the project that run a real Jetty server and download content with the monitor. In these tests, at the end the Content-Length equals the total consumed.

I don't have access to the file or server you referenced, so I cannot verify with your data. Can you produce an example where this happens that we can duplicate?

This is my local code:

public static void main(String[] args) {
	String catDownloadPath = "/Users/peterzhao/Downloads/test.jpg";
	String cat = "https://cdn.pixabay.com/photo/2021/10/01/18/53/corgi-6673343_960_720.jpg";

	Unirest.get(cat).downloadMonitor((field, fileName, bytesWritten, totalBytes) -> {
		System.out.println(String.format("cat totalBytes: %s, bytesWritten: %s", totalBytes, bytesWritten));
	}).asFile(catDownloadPath);
}

image

I run the code in the main method, there some debug message, the Content-Length equals totalBytes.

public static void main(String[] args) {
	String cityDownloadPath = "/Users/peterzhao/Downloads/city.png";
	String city = "https://cdn.pixabay.com/photo/2023/02/09/16/36/bridge-7779222_1280.jpg";
	Unirest.get(city).downloadMonitor((field, fileName, bytesWritten, totalBytes) -> {
		System.out.println(String.format("city totalBytes: %s, bytesWritten: %s", totalBytes, bytesWritten));
	}).asFile(cityDownloadPath);
}

image

And I tried to download a new one, the same problem, but I found that bytesWritten seems like equal to double totalBytes, is it possible that bytesWritten was counted twice?

ryber commented

weird, I tried both and in both cases I get totalBytes equal to bytes written.

It seems that my previous guess is right, the read bytes were counted twice.

The MonitoringInputStream class override read(byte[] b) and read(byte[] b, int off, int len) method, but read(byte[] b) in super InputStream class called read(byte[] b, int off, int len).

So method read(byte[] b) and read(byte[] b, int off, int len) both called in once file read, and byteCount was added twice.

Please confirm findings above are correct or not thanks.

ryber commented

huh, I think it must depend on exactly which kind of InputStream is being monitored and which methods are being called the override for read(byte[] b) can be removed entirely and it doesn't fail the tests at all. I would like to find a way to create a failing test for this

ryber commented

checkout this f01154a

I'm honestly not entirely sure how the old one worked at all. It should never be calling its own methods. It should be delegating

BytesWritten is correct now except for the last one, I think it may be because the last read() method returns -1?

image

ryber commented

aaaaaaaaah, yea thats why, I was wondering where that missing bit was

The code works as expected now, appreciate