Skip to content

Conversation

glmfe
Copy link
Collaborator

@glmfe glmfe commented Feb 21, 2025

Description

  • Handle the HTTP 3xx status family and redirect the connection to the new host.

Copy link
Collaborator

@david-cermak david-cermak left a comment

Choose a reason for hiding this comment

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

LGTM in general, but

we have to handle old IDFs, i.e. this component needs to work with tcp_transport component which doesnt' support redirects.

we usually do it with feature list macros, which are defined depending on IDF versions (you can check mqtt client component for example: https://github.com/espressif/esp-mqtt/blob/master/include/mqtt_supported_features.h)

@glmfe glmfe force-pushed the feat/add-http-redir branch from d98ecab to 6afad51 Compare February 25, 2025 14:46
@glmfe glmfe force-pushed the feat/add-http-redir branch from 6afad51 to 2613138 Compare February 25, 2025 16:35
Copy link
Collaborator

@david-cermak david-cermak left a comment

Choose a reason for hiding this comment

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

changes in this PR LGTM!

 - Handle 301 status (moved permanently) and redirect the connection to the new host.
@glmfe glmfe force-pushed the feat/add-http-redir branch from 2613138 to 098f6ba Compare March 12, 2025 01:13
@glmfe glmfe marked this pull request as ready for review April 2, 2025 02:33
else if (WS_HTTP_REDIRECT(result)) {
// Redirecting to a new URI
client->config->port = 0;
client->config->uri = esp_transport_ws_get_redir_uri(client->transport);
Copy link
Collaborator

Choose a reason for hiding this comment

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

How long this will be valid? Shall we strdup here?

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