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

Jump to bottom

optionally allow redirects on HTTPClient & OTA updates #5009

Merged
merged 53 commits into from Mar 13, 2019
Merged

optionally allow redirects on HTTPClient & OTA updates #5009

merged 53 commits into from Mar 13, 2019

Conversation

liebman
Copy link
Contributor

@liebman liebman commented Aug 5, 2018 •
edited

Support is disabled by default. Call ESPhttpUpdate.followRedirects(true); to enable.

@d-a-v d-a-v mentioned this pull request Oct 18, 2018
Closed
@d-a-v d-a-v mentioned this pull request Nov 1, 2018
Closed
@d-a-v
Copy link
Collaborator

d-a-v commented Nov 1, 2018

Redirection code is currently handled in ESP8266httpUpdate.cpp in your PR.
I was wondering why not moving it straight into ESP8266HTTPClient.cpp ?
After a discussion with @earlephilhower, this is doable if the protocol does not change after redirection (http vs https).
If it changes (security involved), then you can return the same code as before (301 or 302).
What do you think ?

@liebman
moved redirect handling code to HTTPClient.
4e62aec 
only GET and HEAD requests are currently handled automatically
Redirects that fail to be automatically handled return the redirect code as before
@liebman
Copy link
Contributor Author

liebman commented Nov 4, 2018

@d-a-v Its still a work in progress but its functional. Before I clean up a few optional TBDs I want to see what others think. Also I'm thinking of allowing redirects for POST requests by turning them into GET requests at the new location if redirects are enabled. (https://en.wikipedia.org/wiki/Post/Redirect/Get)

@liebman
Copy link
Contributor Author

liebman commented Nov 8, 2018

@d-a-v Updated. Added support for POST 303 redirect. Also added device tests for various redirect features.

liebman and others added 11 commits November 7, 2018 18:40
@liebman
add missing getLocation() implementation
12f7ad7
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
1dc0c83
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
deb7ac3
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
eb3e3e8
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
5199ef9
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
3c81567
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
8fdfa52
@liebman
Merge branch 'feature/http_update_follow_redirects' of github.com:lie…
99f2b9a 
…bman/Arduino into feature/http_update_follow_redirects
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
0aafb71
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
94a448e
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
78629b0
@liebman liebman changed the title optionally allow redirects on http OTA updates optionally allow redirects on HTTPClient & OTA updates Dec 16, 2018
liebman and others added 21 commits December 16, 2018 06:03
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
0a8f23d
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
c34807a
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
f022a61
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
b65433c
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
4ef1450
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
18016eb
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
7870245
@liebman
if the new location is only a path then only update the URI
e9a2c23
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
feebd7f
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
f7e0d3f
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
0b4cdbf
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
5b4a184
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
dc11d31
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
8521153
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
06d4434
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
edb75e5
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
1032ab3
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
d3643d0
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
03a2cfa
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
b959a48
@liebman
Merge branch 'master' into feature/http_update_follow_redirects
fdc2b22
@liebman
Copy link
Contributor Author

liebman commented Mar 9, 2019

@d-a-v any more changes needed?

d-a-v
d-a-v approved these changes Mar 10, 2019
View reviewed changes
Copy link
Collaborator

@d-a-v d-a-v left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@d-a-v d-a-v requested a review from devyte March 10, 2019 17:50
devyte
devyte approved these changes Mar 12, 2019
View reviewed changes
Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

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

Nothing noteworthy jumps out at me code-wise.
Approved.

@d-a-v d-a-v merged commit e46ccae into esp8266:master Mar 13, 2019
@liebman liebman deleted the feature/http_update_follow_redirects branch September 10, 2019 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d-a-v d-a-v d-a-v approved these changes

@devyte devyte devyte approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@liebman @d-a-v @devyte @earlephilhower