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.

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

feat: adding possibility to manually set MD5 checksum and authorization for HTTP update #7629

Merged
merged 2 commits into from
Nov 29, 2023

Conversation

vlastahajek
Copy link
Contributor

@vlastahajek vlastahajek commented Dec 26, 2022

I've written a library that allows updating from Github releases.
To verify firmware integrity HTTPUpdater already supports MD5 checksum but it is only read from the server response header. This is not possible if there is no control of the server.
If the checksum is known from another source, it is not possible to set it currently.
This PR allows this.
The same addition was already done for ESP8266.

Update 23.2.2023:
Adding also setting of authorization to allow connecting to a private server.
This is already part of ESP8266 HTTPUpdate.

@@ -121,6 +126,7 @@ class HTTPUpdate
private:
int _httpClientTimeout;
followRedirects_t _followRedirects;
String _md5Sum;
Copy link
Member

Choose a reason for hiding this comment

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

_md5Sum = String(); should be added in the class constructor after _followRedirects is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But String _md5Sum; means it is automatically initialized empty String instance, isn't it?

Copy link
Member

Choose a reason for hiding this comment

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

the compiler will complain. The String is not necessarily initialized. You can initialize it in two general ways:

HTTPUpdate::HTTPUpdate(int httpClientTimeout): _httpClientTimeout(httpClientTimeout), _ledPin(-1), _md5Sum(){
  _followRedirects = HTTPC_DISABLE_FOLLOW_REDIRECTS;
}

Or

HTTPUpdate::HTTPUpdate(int httpClientTimeout): _httpClientTimeout(httpClientTimeout), _ledPin(-1){
  _followRedirects = HTTPC_DISABLE_FOLLOW_REDIRECTS;
  _md5Sum = String();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, understood. Thanks for the correction 👍. Fixed.

@vlastahajek
Copy link
Contributor Author

vlastahajek commented Feb 23, 2023

I added also authorization. This will finally make this on par with ESP8266 HTTPUpdate.

@vlastahajek vlastahajek changed the title feat: adding possibility to manually set MD5 checksum for HTTP update feat: adding possibility to manually set MD5 checksum and authorization for HTTP update Feb 23, 2023
@CLAassistant
Copy link

CLAassistant commented May 6, 2023

CLA assistant check
All committers have signed the CLA.

@vlastahajek
Copy link
Contributor Author

@me-no-dev, what with this PR?
I've seen another PR, that added a request update callback, was accepted even to the update release. Its feature allows also adding authorization, however, authorization is a common feature that deserves its own API functions.

@me-no-dev
Copy link
Member

Hi @vlastahajek ! We will have this merged for v3.0.0

@me-no-dev me-no-dev added this to the 3.0.0 milestone May 18, 2023
@VojtechBartoska VojtechBartoska added the Area: Libraries Issue is related to Library support. label Nov 28, 2023
@VojtechBartoska VojtechBartoska modified the milestones: 3.0.0, 3.0.0-Aplha3 Nov 28, 2023
@me-no-dev me-no-dev merged commit 8c150e2 into espressif:master Nov 29, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Libraries Issue is related to Library support.
Projects
Development

Successfully merging this pull request may close these issues.

4 participants