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

Fix support for following redirects added by previous merge #4385

Merged
merged 1 commit into from
Oct 14, 2020

Conversation

trandi
Copy link
Contributor

@trandi trandi commented Oct 3, 2020

ee88c42 added support for following redirects (#4240)

However I couldn't make it work because of 2 issues in HTTPClient.cpp:

  1. in the newly added setURL() we do "_canReuse = true" which means we preserve the client but kill the connection. The latter invalidates some of the memory which means that later _client.available() or similar blow up completely

  2. getString() doesn't deal with when _size == -1 which can be a valid state (see existing comment re "Server not sending Content-Length header"). I've changed that condition to how it was in one of the previous versions and removed the optimisation around reserving memory upfront, as this seems to be done later on just fine..

It's my first time I contribute to this project so please bear with me if I got things wrong 👍
Dan

@trandi
Copy link
Contributor Author

trandi commented Oct 6, 2020

@atanisoft / @me-no-dev or others more knowledgeable, what is the process now in order to have this merged ?
Are there more checks / reviews to be done ?

@me-no-dev me-no-dev merged commit ee3bb16 into espressif:master Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants