V2.4.2 fails regularly when sending "Connection: close" on client.print (GET...

[mars000]

Hi

I'm using ESP8266 V2.4.2

Noticed that when I upgraded from V2.4.1 that regularly if I use the "close" connection vs "open" it fails to read HTTP headers correctly. Worked all fine on V2.4.0 and V2.4.1

Not sure what the issue it. As said - works perfectly if I use Connection: open on V2.4.2 but not "Connection: close"

The call I'm using is:

client.print(String("GET ") + url_local + " HTTP/1.1\r\n" +
							 "Host: " + host_local + "\r\n" +
							 "User-Agent: BuildFailureDetectorESP8266\r\n" +
							 "Connection: close\r\n\r\n"); // close connection

----------------------------- Delete below -----------------------------

If your issue is a general question, starts similar to "How do I..", is related to 3rd party libs, or is related to hardware, please discuss at a community forum like esp8266.com.

INSTRUCTIONS

If you do not follow these instructions, your issue may be dismissed.

1. Follow the checklist under Basic Infos and fill in the [ ] spaces with an X.
2. Fill in all the fields under Platform and Settings in IDE marked with [ ] (pick the correct option for you in each case, delete the others).
3. If you haven't already done so, test your issue against current master branch (aka latest git), because it may have been already fixed.
4. Describe your problem.
5. If you have a STACK DUMP decode it:

https://arduino-esp8266.readthedocs.io/en/latest/Troubleshooting/stack_dump.html

6. Include a Minimal Complete Reproducible Example sketch that shows your issue. Do not include your entire project, or a huge piece of code.
7. Include debug messages:

https://arduino-esp8266.readthedocs.io/en/latest/Troubleshooting/debugging.html

8. Use markup (buttons above) and the Preview tab to check what the issue will look like.
9. Delete these instructions from the above to the below marker lines before submitting this issue.

----------------------------- Delete above -----------------------------

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]

Settings in IDE

• Module: [Generic ESP8266 Module|Wemos D1 mini r2|Nodemcu|other]
• Flash Mode: [qio|dio|other]
• Flash Size: [4MB/1MB]
• lwip Variant: [v1.4|v2 Lower Memory|Higher Bandwidth]
• Reset Method: [ck|nodemcu]
• Flash Frequency: [40Mhz]
• CPU Frequency: [80Mhz|160MHz]
• Upload Using: [OTA|SERIAL]
• Upload Speed: [115200|other] (serial upload only)

Problem Description

Detailed problem description goes here.

MCVE Sketch

#include <Arduino.h>

void setup() {

}

void loop() {

}

Debug Messages

Debug messages go here

[d-a-v]

Please provide an MCVE and give more details about its behavior.

[DonKracho]

I can confirm this. I updated to 2.4.2 because I had issues to get the WIFI_AP mode to run stable. My web pages exceed the max size of 2920 bytes, therefore I send them in 3 chunks.

The header starts with: "HTTP/1.1 200 OK\n\Content-Type: text/html\n\Connection: close\n\n" followed by the page content itself.

    client.print(Html::getHead());   //1144 bytes
    client.print(Html::getScript());  //2039 bytes
    client.print(Html::getBody());   // 619 bytes
    client.stop();

With 2.4.2 the web page is broken (contains only parts of the last client.print() call for the body). I reverted to 2.4.0rc2 and everything is fine again without changing the sketch.

To me it looks like in 2.4.2 client.stop(); works asynchrounus and does not wait for the last client.print() to be completed.

Btw: My page uses websockets. These start very delayed with 2.4.2 in 2.4.0rc2 it is like expected.

[d-a-v]

Maybe related to #5021. full MCVE is welcome.

[igrr]

Could you please confirm whether calling client.flush() before client.stop() works around the problem?

[TheNitek]

Same problem here. client.flush() right before client.stop() did NOT change anything. Problem still exists.

[d-a-v]

This issues looks like the other above mentioned #5021 which is a user error with a sketch that worked with previous versions (unknown reason so far) but badly handling http protocol. Browser's header was not fully read (It can be fixed by at least honoring browser by reading its full http header before closing the connection).

Without MCVE, we can't help. We need more details.

[Scippi]

I've posted this short MCVE Sketch also in #5021.
As @TheNitek mentioned before client.flush() did NOT change anything.
I've used version 2.4.2.
Remove the last client.stop() to show the page in browser.

#include <ESP8266WiFi.h>

const char* ssid = "********";
const char* password = "********";


WiFiServer server(80);


void setup()
{
  Serial.begin(115200);
  Serial.println();

  Serial.printf("Connecting to %s ", ssid);
  WiFi.begin(ssid, password);
  while (WiFi.status() != WL_CONNECTED)
  {
    delay(500);
    Serial.print(".");
  }
  Serial.println(" connected");

  server.begin();
  Serial.printf("Web server started, open %s in a web browser\n", WiFi.localIP().toString().c_str());
}

// prepare a web page to be send to a client (web browser)
String prepareHtmlPage()
{
  String htmlPage =
     String("HTTP/1.1 200 OK\r\n") +
            "Content-Type: text/html\r\n" +
            "Connection: close\r\n" +  // the connection will be closed after completion of the response
            "Refresh: 5\r\n" +  // refresh the page automatically every 5 sec
            "\r\n" +
            "<!DOCTYPE HTML>" +
            "<html>" +
            "Analog input:  " + String(analogRead(A0)) +
            "</html>" +
            "\r\n";
  return htmlPage;
}


void loop()
{
  WiFiClient client = server.available();
  // wait for a client (web browser) to connect
  if (client)
  {
    Serial.println("\n[Client connected]");
    while (client.connected())
    {
      // read line by line what the client (web browser) is requesting
      if (client.available())
      {
        String line = client.readStringUntil('\r');
        Serial.print(line);
        // wait for end of client's request, that is marked with an empty line
        if (line.length() == 1 && line[0] == '\n')
        {
          client.println(prepareHtmlPage());
          break;
        }
      }
    }
    delay(1); // give the web browser time to receive the data

    // close the connection:
    client.stop();
    Serial.println("[Client disonnected]");
  }
}

[d-a-v]

Same observation as #5021
I updated your code as follow:

    // close the connection:
    int c = client.read();
    Serial.printf("c=%d n=%d r=%d\n", c, '\n', '\r');
    client.stop();

It's working and shows:

c=10 n=10 r=13

meaning that you read '\r\n\r' and not the final '\n'
Reading it makes the browser happy.

Yet,

• I can't explain yet why it has changed since previous versions
• This is a protocol handling error
• About backward portability I don't know what to think of this

[DonKracho]

Hi David

you are welcome. I also prepared a small scetch stripped down to the basic needs. It opens a WiFi_STA connection. The IP given in the serial monitor keyed in into a browser adress field should show up a web page with a table with 30 rows and the text 'Web page complete.' at the end. I'm using the arduino IDE 1.8.5. The target is a generic ESP-01, but is reproduceable with other targets, like WeMos mini too.

With 2.4.0 (rc2) the page shows up fine in 2.4.2 the table is broken at coincidentally positions due to missing content.
B.t.w. I'm cleaning the client request by calling client.flush(),

#include <ESP8266WiFi.h>

#define SSID "***"          // define your WiFi access point ssid here
#define PWD "***"     		// define your WiFi password here

WiFiServer server(80);

void setup() {
  Serial.begin(115200);
  delay(500);

  byte retries = 30;
  Serial.print(String("\nStarting WiFi in STA mode. Connecting to: ") + SSID + " ");
  WiFi.disconnect();
  WiFi.mode(WIFI_STA);
  WiFi.begin(SSID, PWD);
  while (WiFi.status() != WL_CONNECTED && --retries) {
    Serial.print("."); delay(1000);
  }
  if (retries > 0) {
    Serial.print(" success IP: "); 
    Serial.println(WiFi.localIP());
    server.begin();    
  } else { 
    Serial.println(" failed!");
  }
}

void loop() {
  WiFiClient client = server.available();
  if (client) {
    String req = client.readStringUntil('\r');  // read 1st line of request
    client.flush();
    if (req.startsWith("GET / ")) {             // return root (index.html)
      Serial.printf("Sending root page with %d bytes.\n", client.print(GetPage()));
    } else {
      client.print("HTTP/1.1 404 Not Found\n\Connection: close\n\n");
    }
    client.stop();  // this call does not wait for the previous client.print() to be finished
  }
}

String GetPage()
{
  String page = "HTTP/1.1 200 OK\nContent-Type: text/html\nConnection: close\n\n<!DOCTYPE html>\n<html>\n<head>\n<title>Dummy Table</title>\n";
  page += "<style>body{color:#eee;background:#346;align:center}table,th,td{font:0.9em Arial;white-space:nowrap;border:1px solid black;border-collapse:collapse;color:#222;background:#eee;padding:3px 10px}";
  page += "table{width:80%}th{padding:6px;font-weight:bold;color:#eee;background:linear-gradient(to bottom,#888,#000);}</style>\n</head>\n";
  page += "<body>\n<table>\n<tr>\n<th colspan='2'>Table Dancing</th>\n</tr>\n";
  for (int i = 2; i <= 30; i++) {
    page += String("<tr>\n<td>Row:  ") + i + "</td><td>The quick brown fox jumps over the lazy dog.</td>\n</tr>\n";
  }
  page += "</table>\n<p>Web page complete.</p></body>\n</html>\n";
  return page;
}

[devyte]

Some comments here:

• The http protocol is easy to understand and read, but implementing it has a long list of caveats. My own suggestion to users is to NOT IMPLEMENT IT, but rather rely on the webserver/httpclient classes instead, unless you:

1. Know what you're doing
2. Have special needs not covered in the available classes, and enhancing them to meet your needs doesn't make sense
3. Know what you're doing

• About backwards compatibility, we try to maintain it as much as possible in minor releases, but we don't keep bugs around to service apps that rely on them. In other words, if an app happens to work due to a bug in our core, and then that bug gets fixed, expect to have to update the app.

[d-a-v]

@DonKracho same issue (read the full header until an empty line terminated by \r\n before .stop()).

@ALL Why not use the integrated WebServer library ?

[mars000]

hi - being the originator of this issue I'm somewhat confused with the conclusion reached in this thread.

Is this in fact an error in V2.4.2 core ? I'm using very standard method of reading header information before I read the body.

@devyte, not 100% what you mean by "About backwards compatibility, we try to maintain it as much as possible in minor releases, but we don't keep bugs around to service apps that rely on them. In other words, if an app happens to work due to a bug in our core, and then that bug gets fixed, expect to have to update the app."

Code below. Using V2.4.2 line labelled [LINE100] sometimes returns a false BUT it works perfectly in V2.4.1

		 client.print(String("GET ") + url_local + " HTTP/1.1\r\n" +
                 "Host: " + host_local + "\r\n" +
                 "Connection: close\r\n\r\n");

				while (client.available() == 0)
				{
					if (millis() - timeout > 5000)
					{
						blynk_status(796, ">>> Client Timeout ! for CLOSE connection attempt");
						client.stop();
						return (blynk_return);
					}
				}
			}

		

				Serial.println("-- Waiting to read ALL headers --"); // commment to not send out to serial
				String find_1 = "content-length";
				String line;

				client.setTimeout(2000); 
				String content_length_body_String;
				int content_length_int;
				while (client.connected())  // <--- [LINE100] this is "false" on using "close" ...SOMETIMES 
				{
					line = client.readStringUntil('\n');
					if (line.startsWith("content-length"))   
					{
						Serial.println("found content-length");
						content_length_body_String =  line.substring(16);
						Serial.print ("length = "); Serial.println (content_length_body_String);
						content_length_int = content_length_body_String.toInt();
					}
					Serial.println(line); // comment to not sent out to serial

					if (line == "\r")
					{
						Serial.println("--- All headers received --- "); // commment to not send out to serial
						Serial.println(line);

						break;
					}
				} // end of while loop

				char body_content[content_length_int+1];  // + 1 to add the '\0' which is only necessary if I want to print it out to Serial.print and make it look nice
				client.readBytes(body_content, content_length_int);
				body_content[content_length_int] = '\0';  // this the last added character space as first space is cell [0]
				Serial.print ("body_content = "); Serial.println (body_content);

[d-a-v]

@mars000
There is a MCVE request in the issue template, and you did not provided it in the first place.
Meanwhile, others with similar issue came to yours and provided their MCVE. @devyte conclusion is about their issues.
Now that you are giving part of what would be your MCVE, we can see this is a different issue.

The ::connected() function has ideed changed, it was :

if (!_client)
        return 0;
...

It is now since #4626:

if (!_client || _client->state() == CLOSED)
        return 0;
...

Your MCVE not beeing complete, I can't reproduce neither say more about this.
We'd love to solve any backward compatibility issue, be we can't if we are blind.

Please provide an MCVE, or give as much details as you can.

Noticed that when I upgraded from V2.4.1 that regularly if I use the "close" connection vs "open" it fails to read HTTP headers correctly. Worked all fine on V2.4.0 and V2.4.1
Not sure what the issue it. As said - works perfectly if I use Connection: open on V2.4.2 but not "Connection: close"
while (client.connected()) // <--- [LINE100] this is "false" on using "close" ...SOMETIMES

This is not clear enough

...

Well, after re-reading, if the connection is supposed to close at any time, but there are still data to read, use

while (client.available())

@ALL please provide details of your issues, we are here to help you with the core, and we are not paid for that, help us help you !

[DonKracho]

@d-a-v did you have the chance to try my scetch?

Moving the client.flush() line or inserting your suggested code line with the client.read() before the client.stop() does not solve the issue.

This is the complete http get request of a current chrome browser:

GET / HTTP/1.1          <-- client.readStringUntil('\r'); is reading the input stream until 'end' of this line
Host: 192.168.0.230
Connection: keep-alive
Cache-Control: max-age=0
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Encoding: gzip, deflate
Accept-Language: de-DE,de;q=0.9,en-US;q=0.8,en;q=0.7

client.readStringUntil('\r'); breaks after the first line is read. If there is a '\n' depends on the client OS. To clear the entire input queue at least a client.flush() has to be done. One client.read() just reads one more char from the input and there remains a lot of input in the queue.

The Chrome browser just sends out all this stuff and really does not care if anybody is reading it! Chrome just waits for a well formed response. If this does not come in time it shows up a timeout.

The client.read() just puts a little delay before the client.stop() and therfore may solve the issue for small web pages. Intentionally I made my web page response much longer (at least it has to be larger than 1460 bytes).

In order to get my code to run 'stable' I have to put a delay of minimal 75us before the client.stop().

My conclusion: The behavior of client.print() or client.stop() has changed somewhere between 2.4.0(rc2) and 2.4.2 Either the clinet.print() does return before it is finished or client.stop() does not wait for all transmission activites to be finished.

[d-a-v]

@DonKracho

• I'm not an http expert
• I discovered yesterday with your issues that the web browser does not like closed connections when it can detect that the header has not been fully read.
• I proved myself with some tests that the previous assertion is true. At least with firefox.
• Your sketch does read only the first line of the header then closes the connection.
• You are thinking that client.flush() empties the input buffer (= read full header), while in fact it waits for the output buffer to be emptied (so in your case it does nothing). I reckon this name is misleading but we follow Arduino specs.
• \r\n is not OS-dependent in the HTTP protocol. Header's EOL is \r\n for every OS. It's an inter-OS protocol, not a text file.
• I think the browser did not even bother to read your data. Because you disrespectfully slam the door before it finishes speaking, so it equally closes it back. Respect the protocol, listen your interlocutor respectfully so you can answer back.
• There is absolutely no need of any kind of delay.
• You do not need to take 1460 into consideration. Note that by default in our core, MSS is 536 not 1460. You don't even need to know that. Protocols (here http over tcp) handle everything. Whether your data is 1 or 10000 bytes, you don't need to know the MSS value. It will just work. But you need to be protocol-respectful.
• Your conclusion is true, something has changed. These changes went IMHO in the right direction until proven wrong. If some kind of sketch were working and now not anymore, this could be a bug in the core, or this could have been a side effect (call it luck). The fact is that not any sketch that were presented in website doesn't show up with core 2.4.2, neither 2.4.1 ... - but correctly with 2.3.0 and 2.4.0 though #5021 or V2.4.2 fails regularly when sending "Connection: close" on client.print (GET... #5058 were protocol-respectful. This leads me to think that the sources of the issues are not in the core (I can always be proven wrong).
• The one client.read() solving another issue is right because in that sketch, the last \n of the final header line \r\n\r\n was left in the incoming buffer thus not acknowledged to the browser. Res-pect-ful until the last bit we need to be with protocols.
• There are libraries and examples. Read them, test them. Developpers take special care writing them, because having working examples is gold. They show how things are meant to be used, they prove things are working.

edit Once you have read all the header and written your answer, add a client.flush() just before client.stop()

[devyte]

Alright, per @d-a-v 's analysis, it looks to me like the user app was relying on a previous core bug which has since been fixed. The fact that client.readStringUntil("\r") was used to detect \r\n\r\n certainly points in that direction.
I have not seen a core bug identified after all this, discussion, so closing.

[mars000]

@d-a-v
thanks for your response. I got it working with V2.4.2 by following your suggestion.
I now use the following double check in the while loop in the case that the client gets disconnected quickly when using "close" connection:

while ( ( client.connected() ) || ( client.available() ) )

instead of just.....

while ( ( client.connected() )

[TheNitek]

What got me confused was this example:
https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266WebServer/examples/SDWebServer/SDWebServer.ino
In line 215 it does WiFiClient client = server.client(); but never uses it. I thought this was needed for some reason and found the client.close() somewhere else later on (don't remember where). So if I understand correctly now, there is no need for line 215?

[DonKracho]

@d-a-v
thaks a lot for all your efford and investigations. So it was my fault interpreting Wificlient::flush() wrong. I modified my example according your suggestions and it works with 2.4.2 and ESP-01 decive.

Working code:

#include <ESP8266WiFi.h>

#define SSID "***"		// define your WiFi access point ssid here
#define PWD "***"		// define your WiFi password here

WiFiServer server(80);

void setup() {
  Serial.begin(115200);
  delay(500);

  byte retries = 30;
  Serial.print(String("\nStarting WiFi in STA mode. Connecting to: ") + SSID + " ");
  WiFi.disconnect();
  WiFi.mode(WIFI_STA);
  WiFi.begin(SSID, PWD);
  while (WiFi.status() != WL_CONNECTED && --retries) {
    Serial.print("."); delay(1000);
  }
  if (retries > 0) {
    Serial.print(" success IP: "); 
    Serial.println(WiFi.localIP());
    server.begin();    
  } else { 
    Serial.println(" failed!");
  }
}

void loop() {
  WiFiClient client = server.available();
  if (client && client.connected()) {
    String req = client.readStringUntil('\r');    // read 1st line of request
    while (client.available()) client.read();     // discard but read rest of the input stream
    Serial.println(req);
    if (req.startsWith("GET / ")) {               // return root (index.html)
      Serial.printf("Sending root web page with %d bytes.\n", client.print(GetPage()));
    } else {
      Serial.printf("Sending 404 Not Found with %d bytes.\n", client.print("HTTP/1.1 404 Not Found\n\Connection: close\n\n"));
    }
    client.flush();
    client.stop();
  }
}

String GetPage()
{
  String page = "HTTP/1.1 200 OK\nContent-Type: text/html\nConnection: close\n\n<!DOCTYPE html>\n<html>\n<head>\n<title>Dummy Table</title>\n";
  page += "<style>body{color:#eee;background:#346;align:center}table,th,td{font:0.9em Arial;white-space:nowrap;border:1px solid black;border-collapse:collapse;color:#222;background:#eee;padding:3px 10px}";
  page += "table{width:80%}th{padding:6px;font-weight:bold;color:#eee;background:linear-gradient(to bottom,#888,#000);}</style>\n</head>\n";
  page += "<body>\n<table>\n<tr>\n<th colspan='2'>Table Dancing</th>\n</tr>\n";
  for (int i = 2; i <= 30; i++) {
    page += String("<tr>\n<td>Row:  ") + i + "</td><td>The quick brown fox jumps over the lazy dog.</td>\n</tr>\n";
  }
  page += "</table>\n<p>Web page complete.</p></body>\n</html>\n";
  return page;
}

and the same thing refactored using the ESP8266WebServer class is working with 2.4.2 too:

#include <ESP8266WiFi.h>
#include <ESP8266WebServer.h>

#define SSID "***" 		// define your WiFi access point ssid here
#define PWD "***"		// define your WiFi password here

ESP8266WebServer server(80);

void setup() {
  Serial.begin(115200);
  delay(500);

  byte retries = 30;
  Serial.print(String("\nStarting WiFi in STA mode. Connecting to: ") + SSID + " ");
  WiFi.disconnect();
  WiFi.mode(WIFI_STA);
  WiFi.begin(SSID, PWD);
  while (WiFi.status() != WL_CONNECTED && --retries) {
    Serial.print("."); delay(1000);
  }
  if (retries > 0) {
    Serial.print(" success IP: "); 
    Serial.println(WiFi.localIP());

    server.on("/", [](){
      server.send(200, "text/html", GetPage());
    });

    server.onNotFound( [](){
      server.send(404, "text/plain", "");
    });

    server.begin();    
  } else { 
    Serial.println(" failed!");
  }
}

void loop() {
  server.handleClient();
}

String GetPage()
{
  String page = "<!DOCTYPE html>\n<html>\n<head>\n<title>Dummy Table</title>\n";
  page += "<style>body{color:#eee;background:#346;align:center}table,th,td{font:0.9em Arial;white-space:nowrap;border:1px solid black;border-collapse:collapse;color:#222;background:#eee;padding:3px 10px}";
  page += "table{width:80%}th{padding:6px;font-weight:bold;color:#eee;background:linear-gradient(to bottom,#888,#000);}</style>\n</head>\n";
  page += "<body>\n<table>\n<tr>\n<th colspan='2'>Table Dancing</th>\n</tr>\n";
  for (int i = 2; i <= 30; i++) {
    page += String("<tr>\n<td>Row:  ") + i + "</td><td>The quick brown fox jumps over the lazy dog.</td>\n</tr>\n";
  }
  page += "</table>\n<p>Web page complete.</p></body>\n</html>\n";
  return page;
}