feat(HTTPClient): add support for collecting all HTTP headers #11768
Conversation
👋 Hello JakubAndrysek, we appreciate your contribution to this project! 📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more. 🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project. Click to see more instructions ...
Review and merge process you can expect ...
|
Memory usage test (comparing PR against master branch)The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.
Click to expand the detailed deltas report [usage change in BYTES]
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Test Results 76 files 76 suites 13m 11s ⏱️ Results for commit 3bd54ba. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull Request Overview
This pull request adds support for collecting all HTTP headers in the HTTPClient library by introducing a new collectAllHeaders() method. The changes refactor the header collection mechanism from manual memory management using arrays to using std::vector for improved safety and flexibility.
- Adds
collectAllHeaders()method for collecting all response headers automatically - Refactors header storage from raw arrays to
std::vector<RequestArgument> - Updates example code to demonstrate both specific and all-header collection modes
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| HTTPClient.h | Adds collectAllHeaders() method declaration and updates member variables to use vector storage |
| HTTPClient.cpp | Implements new header collection logic and refactors existing methods to use vector-based storage |
| ci.json | Adds CI configuration for WiFi requirements |
| CustomHeaders.ino | New example demonstrating both specific and all-header collection modes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| #include <assert.h> | ||
| #include <HTTPClient.h> | ||
|
|
||
| #define USE_SERIAL Serial |
There was a problem hiding this comment.
why is this needed? Please remove
There was a problem hiding this comment.
I have created this example based on this BasicHttpClient, which uses the same macro. Now I have looked at the implementation of next HTTPClient examples and some of them use it as well.
Do you want me to remove it in the next PR?
f8100aa to
a2df766
Compare
| @@ -304,8 +306,7 @@ class HTTPClient { | |||
| String _acceptEncoding = "identity;q=1,chunked;q=0.1,*;q=0"; | |||
|
|
|||
| /// Response handling | |||
| RequestArgument *_currentHeaders = nullptr; | |||
| size_t _headerKeysCount = 0; | |||
| std::vector<RequestArgument> _currentHeaders; | |||
There was a problem hiding this comment.
Why, vector probably needs more bytes.
There was a problem hiding this comment.
You are right, the vector probably needs more bytes, but since I do not know the exact size and I am reading until the _client has some data, it is a lot easier to just use a vector and let it grow as needed
The previous implementation used only a fixed-size array, but after the update, I need to account for dynamic resizing to allow storing multiple headers. It is possible to do it with a resize operation of a static vector, but I think it is more convenient to use a dynamic vector for this purpose.
There was a problem hiding this comment.
Fully understand, over time every part of the core does need more resources. One day we will have a core which can not be used for anything more than just "blink".
Description of Change
This pull request refactors the HTTP header collection logic in the
HTTPClientlibrary to use astd::vectorfor managing response headers, adds support for collecting all headers, and updates the example and CI configuration accordingly. The changes simplify memory management, improve flexibility, and make it easier to work with dynamic sets of headers.Test Scenarios
Code was tested on ESP32 and ESP32S3
Related links
Closes issue #10802 … example, close #10802