Skip to content

Wrong HTTPS example fixed. Will save others my #5734 hours of debugging #5739

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.

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -45,32 +45,36 @@ void loop() {

client->setFingerprint(fingerprint);

HTTPClient https;

Serial.print("[HTTPS] begin...\n");
if (https.begin(*client, "https://jigsaw.w3.org/HTTP/connection.html")) { // HTTPS

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

// httpCode will be negative on error
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 || httpCode == HTTP_CODE_MOVED_PERMANENTLY) {
String payload = https.getString();
Serial.println(payload);
/* in order to ensure the client object lives the entire time of the HTTPClient
the HTTPClient is in its own nested code block */
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

HTTPClient doesn't need to be in its own code block, it just needs to be instantiated after client. Destructors are called in reverse order, which ensures https gets destroyed first and client second.
I suggest removing the code block, and updating the comment to explain this.

Copy link
Author

Choose a reason for hiding this comment

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

I don't agree. It's way too dangerous to rely on order of code lines. Took me two days to debug that. It should be totally explicit, IMHO.

HTTPClient https;

Serial.print("[HTTPS] begin...\n");
if (https.begin(*client, "https://jigsaw.w3.org/HTTP/connection.html")) { // HTTPS

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

// httpCode will be negative on error
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 || httpCode == HTTP_CODE_MOVED_PERMANENTLY) {
String payload = https.getString();
Serial.println(payload);
}
} else {
Serial.printf("[HTTPS] GET... failed, error: %s\n", https.errorToString(httpCode).c_str());
}

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

https.end();
} else {
Serial.printf("[HTTPS] Unable to connect\n");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,56 +54,60 @@ void loop() {

client->setFingerprint(fingerprint);

HTTPClient https;
/* in order to ensure the client object lives the entire time of the HTTPClient
the HTTPClient is in its own nested code block */
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

HTTPClient https;

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

Serial.print("[HTTPS] GET...\n");
// start connection and send HTTP header
int httpCode = https.GET();
if (httpCode > 0) {
// HTTP header has been send and Server response header has been handled
Serial.printf("[HTTPS] GET... code: %d\n", httpCode);
Serial.print("[HTTPS] GET...\n");
// start connection and send HTTP header
int httpCode = https.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) {
// file found at server
if (httpCode == HTTP_CODE_OK) {

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

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

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

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

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

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

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

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

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

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

Expand Down