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

Add url redirect #8970

Merged
merged 16 commits into from
Mar 24, 2024
Merged

Add url redirect #8970

merged 16 commits into from
Mar 24, 2024

Conversation

hmueller01
Copy link
Contributor

Add redirect function like ESPAsyncWebServer.
Sorry for the wrong commits/reverts. Obviously my master wasn't clean.

@hmueller01
Copy link
Contributor Author

Is it possible to merge this, also there is a build problem on Linux? Seems to be not the fault of this PR ...

@d-a-v
Copy link
Collaborator

d-a-v commented Nov 7, 2023

What about using this API in our examples ?

@hmueller01
Copy link
Contributor Author

@d-a-v Sorry, but I do not understand your comment. These are examples that do a redirect. My proposal was to add this out-of-the-box of the ESP8266WebServer ...

@d-a-v
Copy link
Collaborator

d-a-v commented Nov 12, 2023

Sorry for lack of clarity.
You added a new API.
I propose you to update our examples to use it.
They would become examples of how to use your new API, examples are meant for that.
Lots of users take them as basis to start their project.
My link above shows you where you could make these changes.

@hmueller01
Copy link
Contributor Author

Oh, sry for being so stupid. Sure, I'll do this in the next few days.

@hmueller01
Copy link
Contributor Author

Looking through the examples I found this:

  server.send(302, "text/plain", "");  // Empty content inhibits Content-length header so we have to close the socket ourselves.
  server.client().stop();  // Stop is needed because we sent no content length

Why is it needed to stop the client? Should this be done in redirect() as well, if I do not send content?

@d-a-v
Copy link
Collaborator

d-a-v commented Nov 17, 2023

Why is it needed to stop the client? Should this be done in redirect() as well, if I do not send content?

It seems to be a good idea.

@hmueller01 hmueller01 changed the title Add url redirect (WIP) Add url redirect Dec 18, 2023
@hmueller01 hmueller01 changed the title (WIP) Add url redirect Add url redirect Dec 18, 2023
@hmueller01
Copy link
Contributor Author

hmueller01 commented Dec 18, 2023 •
edited
Loading

@d-a-v Ok, I finished my work on this. Also did some tests and it looks good.
Damn it. Just saw some checks are failed. Will fix this first ...

@hmueller01
Copy link
Contributor Author

Fixed. All checks have passed. 👍

@hmueller01
Copy link
Contributor Author

I just merged with master and it still looks clean.
@d-a-v Is this ready form merging now (and adding to a new release you just mentioned)?

@d-a-v d-a-v merged commit 877d440 into esp8266:master Mar 24, 2024
28 checks passed
hasenradball pushed a commit to hasenradball/Arduino that referenced this pull request Nov 18, 2024
@hmueller01 @hasenradball
Add url redirect (esp8266#8970)
3ae4d6f 
* added getAvailableVersion(), moved _httpClientTimeout and _followRedirects to protected, added enum HTTPUpdateError

* auto numbering of HTTPUpdateError enum

* added getAvailableVersion(), debug output current current Sketch MD5

* Revert "added getAvailableVersion(), debug output current current Sketch MD5"

This reverts commit 60d2c77.

* Revert "auto numbering of HTTPUpdateError enum"

This reverts commit 61785b2.

* Revert "added getAvailableVersion(), moved _httpClientTimeout and _followRedirects to protected, added enum HTTPUpdateError"

This reverts commit cec84ed.

* add redirect function

* enhanced redirect() by cache control and client stop

* updated redirect() comment

* replaced redirect() API calls in examples

* server.client().stop() not needed, redirect() does this
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

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

Successfully merging this pull request may close these issues.
2 participants
@hmueller01 @d-a-v