Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/http client wi fi client parameter #4979

Closed
wants to merge 15 commits into from
Closed

Feature/http client wi fi client parameter #4979

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
ff2858a
Function added to detect baudrate
Jul 29, 2018
f9ae2c4
Two begin() methods added to pass WiFiClient, updated examples
Jul 29, 2018
d0d7466
Merge branch 'master' into feature/HTTPClient_WiFiClient_parameter
Aug 3, 2018
96636f2
Merge branch 'master' into feature/HTTPClient_WiFiClient_parameter
Aug 3, 2018
5aff343
Merge branch 'master' into feature/HTTPClient_WiFiClient_parameter
Aug 20, 2018
d4db72d
add basicHttpsClient example, ensured in the backward compatible func…
Aug 20, 2018
2807373
Merge branch 'master' into feature/HTTPClient_WiFiClient_parameter
Aug 20, 2018
815fb0f
Adding #pragma's to suppress Travis Build warning for use of deprecat…
Aug 21, 2018
2047402
Adding #pragma's to suppress Travis Build warning for use of deprecat…
Aug 21, 2018
5ff9728
Adding #pragma's to suppress Travis Build warning for use of deprecat…
Aug 21, 2018
11ea32a
Adding #pragma's to suppress Travis Build warning for use of deprecat…
Aug 21, 2018
cfd977c
revert accidentally changed files ESP8266httpUpdate.h and .cpp to ori…
Aug 21, 2018
dc20696
Adding #pragma's to ESP8266httpUpdate.cpp to ignore use of deprecated…
Aug 21, 2018
29e7754
Allocate BearSSL::WiFiClientSecure dynamically in StreamHttpsClient.ino
Aug 22, 2018
fba638c
Bugfix in getStream() and getStreamPtr()
Aug 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions libraries/ESP8266HTTPClient/examples/Authorization/Authorization.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

#include <ESP8266HTTPClient.h>

#include <WiFiClient.h>

#define USE_SERIAL Serial

ESP8266WiFiMulti WiFiMulti;
Expand Down Expand Up @@ -40,22 +42,24 @@ void loop() {
// wait for WiFi connection
if ((WiFiMulti.run() == WL_CONNECTED)) {

WiFiClient client;

HTTPClient http;

USE_SERIAL.print("[HTTP] begin...\n");
// configure traged server and url


http.begin("http://user:password@192.168.1.12/test.html");
http.begin(client, "http://guest:guest@jigsaw.w3.org/HTTP/Basic/");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test, moving it to a public web service!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)


/*
// or
http.begin("http://192.168.1.12/test.html");
http.setAuthorization("user", "password");
http.begin(client, "http://jigsaw.w3.org/HTTP/Basic/");
http.setAuthorization("guest", "guest");

// or
http.begin("http://192.168.1.12/test.html");
http.setAuthorization("dXNlcjpwYXN3b3Jk");
http.begin(client, "http://jigsaw.w3.org/HTTP/Basic/");
http.setAuthorization("Z3Vlc3Q6Z3Vlc3Q=");
*/


Expand All @@ -82,4 +86,3 @@ void loop() {

delay(10000);
}

39 changes: 23 additions & 16 deletions libraries/ESP8266HTTPClient/examples/BasicHttpClient/BasicHttpClient.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

#include <ESP8266HTTPClient.h>

#include <WiFiClient.h>

#define USE_SERIAL Serial

ESP8266WiFiMulti WiFiMulti;
Expand Down Expand Up @@ -40,34 +42,39 @@ void loop() {
// wait for WiFi connection
if ((WiFiMulti.run() == WL_CONNECTED)) {

WiFiClient client;

HTTPClient http;

USE_SERIAL.print("[HTTP] begin...\n");
// configure traged server and url
//http.begin("https://192.168.1.12/test.html", "7a 9c f4 db 40 d3 62 5a 6e 21 bc 5c cc 66 c8 3e a1 45 59 38"); //HTTPS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's remove the comment, maybe it's better to add "BasicHTTPSClient.ino" instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing comment and adding a BasicHTTPSClient.ino with fingerprint security check

http.begin("http://192.168.1.12/test.html"); //HTTP
if (http.begin(client, "http://jigsaw.w3.org/HTTP/connection.html")) { // HTTP


USE_SERIAL.print("[HTTP] GET...\n");
// start connection and send HTTP header
int httpCode = http.GET();
USE_SERIAL.print("[HTTP] GET...\n");
// start connection and send HTTP header
int httpCode = http.GET();

// httpCode will be negative on error
if (httpCode > 0) {
// HTTP header has been send and Server response header has been handled
USE_SERIAL.printf("[HTTP] GET... code: %d\n", httpCode);
// httpCode will be negative on error
if (httpCode > 0) {
// HTTP header has been send and Server response header has been handled
USE_SERIAL.printf("[HTTP] GET... code: %d\n", httpCode);

// file found at server
if (httpCode == HTTP_CODE_OK) {
String payload = http.getString();
USE_SERIAL.println(payload);
// file found at server
if (httpCode == HTTP_CODE_OK || httpCode == HTTP_CODE_MOVED_PERMANENTLY) {
String payload = http.getString();
USE_SERIAL.println(payload);
}
} else {
USE_SERIAL.printf("[HTTP] GET... failed, error: %s\n", http.errorToString(httpCode).c_str());
}

http.end();
} else {
USE_SERIAL.printf("[HTTP] GET... failed, error: %s\n", http.errorToString(httpCode).c_str());
USE_SERIAL.printf("Unable to connect\n");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor thing, but suggest adding "[HTTP]" to this message to match formatting of other outputs in the example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding [HTTP] in basicHttpClient.ino and [HTTPS] in the new basicHttpsClient.ino

}

http.end();
}

delay(10000);
}

10 changes: 6 additions & 4 deletions libraries/ESP8266HTTPClient/examples/DigestAuthorization/DigestAuthorization.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

#include <ESP8266HTTPClient.h>

const char* ssid = "........";
const char* ssidPassword = "........";
const char* ssid = "SSID";
const char* ssidPassword = "PASSWORD";

const char *username = "admin";
const char *password = "admin";
Expand Down Expand Up @@ -76,7 +76,7 @@ String getDigestAuth(String& authReq, const String& username, const String& pass
}

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

WiFi.mode(WIFI_STA);
WiFi.begin(ssid, ssidPassword);
Expand All @@ -95,10 +95,12 @@ void setup() {
void loop() {
HTTPClient http;

WiFiClient client;

Serial.print("[HTTP] begin...\n");

// configure traged server and url
http.begin(String(server) + String(uri));
http.begin(client, String(server) + String(uri));


const char *keys[] = {"WWW-Authenticate"};
Expand Down
9 changes: 4 additions & 5 deletions libraries/ESP8266HTTPClient/examples/ReuseConnection/ReuseConnection.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ void loop() {
// wait for WiFi connection
if ((WiFiMulti.run() == WL_CONNECTED)) {

http.begin("http://192.168.1.12/test.html");
//http.begin("192.168.1.12", 80, "/test.html");
WiFiClient client;

http.begin(client, "http://192.168.1.12/test.html");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe move this to jigsaw as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving to jigsaw

//http.begin(client, "192.168.1.12", 80, "/test.html");

int httpCode = http.GET();
if (httpCode > 0) {
Expand All @@ -65,6 +67,3 @@ void loop() {

delay(1000);
}



9 changes: 5 additions & 4 deletions libraries/ESP8266HTTPClient/examples/StreamHttpClient/StreamHttpClient.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,13 @@ void loop() {

HTTPClient http;

WiFiClient client;

USE_SERIAL.print("[HTTP] begin...\n");

// configure server and url
http.begin("http://192.168.1.12/test.html");
//http.begin("192.168.1.12", 80, "/test.html");
http.begin(client, "http://jigsaw.w3.org/HTTP/connection.html");
//http.begin(client, "jigsaw.w3.org", 80, "/HTTP/connection.html");

USE_SERIAL.print("[HTTP] GET...\n");
// start connection and send HTTP header
Expand All @@ -65,7 +67,7 @@ void loop() {
uint8_t buff[128] = { 0 };

// get tcp stream
WiFiClient * stream = http.getStreamPtr();
WiFiClient * stream = &client;

// read all data from server
while (http.connected() && (len > 0 || len == -1)) {
Expand Down Expand Up @@ -99,4 +101,3 @@ void loop() {

delay(10000);
}

116 changes: 116 additions & 0 deletions libraries/ESP8266HTTPClient/examples/StreamHttpsClient/StreamHttpsClient.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/**
StreamHTTPClient.ino

Created on: 24.05.2015

*/

#include <Arduino.h>

#include <ESP8266WiFi.h>
#include <ESP8266WiFiMulti.h>

#include <ESP8266HTTPClient.h>

#define USE_SERIAL Serial

ESP8266WiFiMulti WiFiMulti;

void setup() {

USE_SERIAL.begin(115200);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure that Serial1 is such a common use case to justify USE_SERIAL. I think using Serial makes for more obvious examples which are easier to copy-paste and combine. (This applied to many existing examples, just wanted to give an opinion.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

USE_SERIAL was already present in the examples of the current 1.1 version of the library. Replacing USE_SERIAL with Serial in all examples

// USE_SERIAL.setDebugOutput(true);

USE_SERIAL.println();
USE_SERIAL.println();
USE_SERIAL.println();

for (uint8_t t = 4; t > 0; t--) {
USE_SERIAL.printf("[SETUP] WAIT %d...\n", t);
USE_SERIAL.flush();
delay(1000);
}

WiFi.mode(WIFI_STA);
WiFiMulti.addAP("SSID", "PASSWORD");

}

void loop() {
// wait for WiFi connection
if ((WiFiMulti.run() == WL_CONNECTED)) {

HTTPClient http;

BearSSL::WiFiClientSecure client;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reinforcing @igrr's comment below, the SSL client object is pretty big and using BearSSL::WiFiClientSecure *client = new BearSSL::WiFiClientSecure ; would help new users. Obviously there will be a delete client; somewhere later on...

Copy link
Contributor Author

@Jeroen88 Jeroen88 Aug 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@earlephilhower I tried to use new and delete, however I get a stack dump (zero pointer reference with a delete). Is it possible that BearSLL destructor is causing the problem?
Stack decode:
Decoding stack results
0x401006dc: free at /home/jeroen/Downloads/arduino-PR-beta1.9-BUILD-69/hardware/esp8266com/esp8266/cores/esp8266/umm_malloc/umm_malloc.c line 1755
0x4020539c: std::__shared_ptr ::operator=(std::__shared_ptr &&) at /home/jeroen/.arduino15/packages/esp8266/tools/xtensa-lx106-elf-gcc/1.20.0-26-gb404fb9-2/xtensa-lx106-elf/include/c++/4.8.2/bits/shared_ptr_base.h line 862
0x4010020c: _umm_free at /home/jeroen/Downloads/arduino-PR-beta1.9-BUILD-69/hardware/esp8266com/esp8266/cores/esp8266/umm_malloc/umm_malloc.c line 1295
0x401006dc: free at /home/jeroen/Downloads/arduino-PR-beta1.9-BUILD-69/hardware/esp8266com/esp8266/cores/esp8266/umm_malloc/umm_malloc.c line 1755
0x40207ca0: operator delete(void*) at /home/jeroen/Downloads/arduino-PR-beta1.9-BUILD-69/hardware/esp8266com/esp8266/cores/esp8266/abi.cpp line 54
0x40201592: delay at /home/jeroen/Downloads/arduino-PR-beta1.9-BUILD-69/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_wiring.c line 54
0x402055c4: BearSSL::WiFiClientSecure::~WiFiClientSecure() at /home/jeroen/Downloads/arduino-PR-beta1.9-BUILD-69/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp line 110
0x402028d9: loop() at /home/jeroen/Downloads/arduino-PR-beta1.9-BUILD-69/hardware/esp8266com/esp8266/libraries/ESP8266HTTPClient/examples/StreamHttpsClient/StreamHttpsClient.ino line 114
0x40207ebc: esp_yield() at /home/jeroen/Downloads/arduino-PR-beta1.9-BUILD-69/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_main.cpp line 91
0x40203b2c: ESP8266WiFiMulti::addAP(char const*, char const*) at /home/jeroen/Downloads/arduino-PR-beta1.9-BUILD-69/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/ESP8266WiFiMulti.cpp line 39
0x40207f48: loop_wrapper() at /home/jeroen/Downloads/arduino-PR-beta1.9-BUILD-69/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_main.cpp line 125

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're getting that kind of crash, you've probably got a use-after-free problem in the code you changed. Without the source that is crashing, can't really get deeper than that. The constructor/destructor is pretty solid and been used with new/delete especially due to the size of the object involved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@earlephilhower I am almost sure that the problem is in WiFiClientSecureBearSSL.cpp.

This is the code that crashes:

/**
StreamHTTPClient.ino

Created on: 24.05.2015

*/

#include <Arduino.h>

#include <ESP8266WiFi.h>
#include <ESP8266WiFiMulti.h>

#include <ESP8266HTTPClient.h>

ESP8266WiFiMulti WiFiMulti;

void setup() {

Serial.begin(115200);
// Serial.setDebugOutput(true);

Serial.println();
Serial.println();
Serial.println();

for (uint8_t t = 4; t > 0; t--) {
Serial.printf("[SETUP] WAIT %d...\n", t);
Serial.flush();
delay(1000);
}

WiFi.mode(WIFI_STA);
WiFiMulti.addAP("SSID", "PASSWORD");

}

void loop() {
// wait for WiFi connection
if ((WiFiMulti.run() == WL_CONNECTED)) {

HTTPClient http;

// BearSSL::WiFiClientSecure client;
BearSSL::WiFiClientSecure *client = new BearSSL::WiFiClientSecure ;

// bool mfln = client.probeMaxFragmentLength("tls.mbed.org", 443, 1024);
bool mfln = client->probeMaxFragmentLength("tls.mbed.org", 443, 1024);
Serial.printf("\nConnecting to https://tls.mbed.org\n");
Serial.printf("Maximum fragment Length negotiation supported: %s\n", mfln ? "yes" : "no");
if (mfln) {
// client.setBufferSizes(1024, 1024);
client->setBufferSizes(1024, 1024);
}

Serial.print("[HTTPS] begin...\n");

// configure server and url
const uint8_t fingerprint[20] = {0xEB, 0xD9, 0xDF, 0x37, 0xC2, 0xCC, 0x84, 0x89, 0x00, 0xA0, 0x58, 0x52, 0x24, 0x04, 0xE4, 0x37, 0x3E, 0x2B, 0xF1, 0x41};

// client.setFingerprint(fingerprint);
client->setFingerprint(fingerprint);

// if (http.begin(client, "https://tls.mbed.org/")) {
if (http.begin(*client, "https://tls.mbed.org/")) {

  Serial.print("[HTTPS] GET...\n");
  // start connection and send HTTP header
  int httpCode = http.GET();
  if (httpCode > 0) {
    // HTTP header has been send and Server response header has been handled
    Serial.printf("[HTTPS] GET... code: %d\n", httpCode);

    // file found at server
    if (httpCode == HTTP_CODE_OK) {

      // get lenght of document (is -1 when Server sends no Content-Length header)
      int len = http.getSize();

      // create buffer for read
      static uint8_t buff[128] = { 0 };

      // get tcp stream

// WiFiClient * stream = &client;
WiFiClient * stream = client;

      // read all data from server
      while (http.connected() && (len > 0 || len == -1)) {
        // get available data size
        size_t size = stream->available();

        if (size) {
          // read up to 128 byte
          int c = stream->readBytes(buff, ((size > sizeof(buff)) ? sizeof(buff) : size));

          // write it to Serial
          Serial.write(buff, c);

          if (len > 0) {
            len -= c;
          }
        }
        delay(1);
        yield(); // ADDED
      }

      Serial.println();
      Serial.print("[HTTPS] connection closed or file end.\n");

    }
  } else {
    Serial.printf("[HTTPS] GET... failed, error: %s\n", http.errorToString(httpCode).c_str());
  }

  http.end();
} else {
  Serial.printf("Unable to connect\n");
}

delete client;

}

delay(10000);
}

I nailed the problem down to the destructor ~HTTPClient() in ESP8266HTTPClient. This calls _client->stop().
In WiFiClientSecure::stop() the function _clearAuthenticationSettings() is called. This function clears the pointers _sk and _ta. Next the destructor ~WiFiClientSecure() is called. This calls delete _sk and delete _ta (if _deleteChainKeyTA is true, which I asume is) but these pointers were cleared already.
If I skip _client->stop() in ~HTTPClient() in ESP8266HTTPClient everything works fine.


bool mfln = client.probeMaxFragmentLength("tls.mbed.org", 443, 1024);
USE_SERIAL.printf("\nConnecting to https://tls.mbed.org\n");
USE_SERIAL.printf("MFLN supported: %s\n", mfln ? "yes" : "no");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MFLN is not a commonly used acronym, suggest spelling out "Maximum fragment length negotiation".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replacing MFLN with "Maximum fragment length negotiation"

if (mfln) {
client.setBufferSizes(1024, 1024);
}

USE_SERIAL.print("[HTTP] begin...\n");

// configure server and url
const uint8_t fingerprint[20] = {0xEB, 0xD9, 0xDF, 0x37, 0xC2, 0xCC, 0x84, 0x89, 0x00, 0xA0, 0x58, 0x52, 0x24, 0x04, 0xE4, 0x37, 0x3E, 0x2B, 0xF1, 0x41};
client.setFingerprint(fingerprint);

//if (http.begin(client, "jigsaw.w3.org", 443, "/HTTP/connection.html", true)) {
if (http.begin(client, "https://tls.mbed.org/")) {

USE_SERIAL.print("[HTTP] GET...\n");
// start connection and send HTTP header
int httpCode = http.GET();
if (httpCode > 0) {
// HTTP header has been send and Server response header has been handled
USE_SERIAL.printf("[HTTP] GET... code: %d\n", httpCode);

// file found at server
if (httpCode == HTTP_CODE_OK) {

// get lenght of document (is -1 when Server sends no Content-Length header)
int len = http.getSize();

// create buffer for read
uint8_t buff[128] = { 0 };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest moving this from stack to global static. I can see how a user might think "i need to make this a bit bigger" and get a stack overflow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing into static uint8_t buff[128]


// get tcp stream
WiFiClient * stream = &client;

// read all data from server
while (http.connected() && (len > 0 || len == -1)) {
// get available data size
size_t size = stream->available();

if (size) {
// read up to 128 byte
int c = stream->readBytes(buff, ((size > sizeof(buff)) ? sizeof(buff) : size));

// write it to Serial
USE_SERIAL.write(buff, c);

if (len > 0) {
len -= c;
}
}
delay(1);
}

USE_SERIAL.println();
USE_SERIAL.print("[HTTP] connection closed or file end.\n");

}
} else {
USE_SERIAL.printf("[HTTP] GET... failed, error: %s\n", http.errorToString(httpCode).c_str());
}

http.end();
} else {
USE_SERIAL.printf("Unable to connect\n");
}
}

delay(10000);
}
Loading