streamFile does only send first 64 bytes

[TD-er]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git).
• I have searched the issue tracker for a similar issue.
• If there is a stack dump, I have decoded it.
• I have filled out all fields below.

Platform

• Hardware: [ESP-12|ESP-01|ESP-07|ESP8285 device|other]
• Core Version: [latest git hash or date]
• Development Env: [Arduino IDE|Platformio|Make|other]
• Operating System: [Windows|Ubuntu|MacOS]

Problem Description

Starting from this commit, the streamFile in ESP8266WebServer does only send the first 64 bytes.

See also the findings reported here: letscontrolit/ESPEasy#3561 (comment)

It is sent in my code in this part:
https://github.com/letscontrolit/ESPEasy/blob/e4bd63aff4be231f957b9c910fcc17343e8e91f8/src/src/WebServer/LoadFromFS.cpp#L63-L85

Which does resemble quite a bit how it is described in the examples like this one:

Arduino/libraries/ESP8266WebServer/examples/Graph/Graph.ino

Line 125 in da6ec83

if (server.streamFile(file, contentType) != file.size()) {

Am I missing something here?
Should I include extra headers, or loop over it to serve what is left?

[d-a-v]

That is a bug from me when converting WiFiClient::write(stream) to use new Stream::send API.
The line

Arduino/libraries/ESP8266WiFi/src/WiFiClient.cpp

Line 237 in 209e467

return stream.sendAvailable(this);

should be changed to

return stream.sendAll(this);

I successfully tested this fix with a random local snippet I have with which I could reproduce the bug you found.
Can you confirm ?

[TD-er]

@d-a-v I asked @clumsy-stefan to test it, who deserves all the credits for finding the commit causing this.

[clumsy-stefan]

I can confirm a ESPEasy build with latest GIT commit from esp8266 core and above mentioned line changed makes it work again!
Thanks a lot for the quick fix!!

[TD-er]

So the biggest delay in fixing/testing it was sleeping time? ;)
@d-a-v Thanks for the extremely fast fix.

[TD-er]

By the way, even though it is a fix for this problem, I am not 100% sure it is the correct fix.
Just looking at the context:

Arduino/libraries/ESP8266WiFi/src/WiFiClient.cpp

Lines 226 to 238 in 209e467

	size_t WiFiClient::write(Stream& stream)
	{
	if (!_client || !stream.available())
	{
	return 0;
	}
	if (stream.hasPeekBufferAPI())
	{
	_client->setTimeout(_timeout);
	return _client->write(stream);
	}
	return stream.sendAvailable(this);
	}

This is a stream write, so it does not make sense to write all in there.
Wouldn't it make more sense to have a separate function in that class to writeAll (in whatever captilization) and call that function for the specific use case of serving a file?
Or make a loop in the webserver class to loop until the file is served?

[d-a-v]

By the way, even though it is a fix for this problem, I am not 100% sure it is the correct fix.

This is what core v2.7.4 does.

This is a stream write, so it does not make sense to write all in there.

This is what WiFiClient API does for a long time (although this is not official Arduino API).

Wouldn't it make more sense to have a separate function in that class to writeAll (in whatever captilization) and call that function for the specific use case of serving a file?

That's possible.

Or make a loop in the webserver class to loop until the file is served?

This kind of loop have been replaced in the core by the API Stream::send*() and is used here (proposal is to update to the right call).

There are several possibilities

• 1. the proposal above (use the right call) (= not a breaking change)
• 2. move this transfer to the webserver's ::streamFile() method, and
  • 1. ... remove all WiFiClient::write(Stream[,size]) because of the new-and-unofficial-but-generic Stream::send*() API does the job (= breaking change)
  • 2. ... remove deprecated WiFiClient::write(Stream, size), and leave WiFiClient::write(Stream) using Stream::sendAvailable() (= breaking change)
  • 3. ... remove deprecated WiFiClient::write(Stream, size), and leave WiFiClient::write(Stream) using Stream::sendAll() (the proposal above) (= not a breaking change)

I think ii. a. is the best bet.

edit: ii. a. + deprecation of WiFiClient::write(Stream) is not a breaking change

[clumsy-stefan]

Just now the problem occurred again. If downloading a larger file (more than about 900 Bytes) it stops at 880Bytes.

[d-a-v]

@clumsy-stefan I must thank you for testing and reporting. I could reproduce and fix in #7987 .
(edit: I could see the issue with a 1.5MB file and downloading it is now working reliably)

Can you retry with #7987 which has just been updated ?

[clumsy-stefan]

Did a new version and some quick checks with #7987 included. So far it seems to work (with ESPEasy). I can download all files again correctly (tested up to 65kB)

Thanks again for the quick fix, really appreciated!!