Infinite loop while passing File(FS.h) resolved

[mayankgour13]

Stream.available() never reaches to -1 which makes it an infinite loop. When File (FS.h) goes empty it reaches to 0 not -1.
Please do check with other Stream implementation and if all is fine, Change requested to be applied.

[devyte]

@igrr please check my thinking here:
I don't think that when available() returns 0 it means "I'm done, stop reading". It means literally "are there any bytes available".
For example, I believe that in the case of a WiFiClient, who inherits from Client who inherits from Stream, and is therefore also a stream, when available() returns 0 it just means "nothing yet, come back later".
I suspect that changing this check here could introduce a subtle timing bug depending on what stream is passed in.
I suspect that the correct fix here should be in File::available(), which should return -1 when EOF is reached.
As a side note, WiFiClient::available should probably return -1 instead of false when _client is closed.

[igrr]

Arduino SD library seems to return 0 when no bytes are available: https://github.com/adafruit/SD/blob/master/File.cpp#L102, so i think that changing the behavior of File::available may introduce some compatibility issues as well.

StreamString::available() also returns 0 when no bytes are available.

In practice, i think using HardwareSerial as a Stream argument for HTTPClient will not work very well, because it's impossible to say whether the stream has reached EOF. Using WiFiClient as Stream might make more sense. To support this cleanly, i think that HTTPClient::sendRequest needs to be modified to rely on Stream::readBytes for timing out, i.e. honor whatever timeout is set on the stream. In essence, the loop should be:

while (connected() && (len > 0 || len == -1)) {
  int readBytes = ... ;
  int bytesRead = stream->readBytes(buff, readBytes);
  if (bytesRead == 0) {
    // stream read has timed out, bail out
    break;
  }
  // send bytes read from the stream
  if (len > 0) len -= bytesRead;
}

[d-a-v]

edited Comment deleted I was thinking the other way around, with a tcp stream as source, not a file stream.
Sorry :-]

@igrr my previous comment (deleted, readable in comment history) applies when using a WiFiClient as stream like you evoked (Using WiFiClient as Stream might make more sense).

Still, we need to implement a generic Print::write(Stream&) with timeout (as previously stated in gitter with @igrr some month ago) that would find its use in many places (where virtuals are still in use :-) ) in the core and particularly here. This make more sense with

To support this cleanly, i think that HTTPClient::sendRequest needs to be modified to rely on Stream::readBytes for timing out, i.e. honor whatever timeout is set on the stream.