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

httpd_req_get_url_query_str does not work with websockets (IDFGH-10685) #11911

Open
3 tasks done
kevinhikaruevans opened this issue Jul 19, 2023 · 4 comments
Open
3 tasks done
Assignees
Labels
Awaiting Response awaiting a response from the author Status: In Progress Work is in progress Type: Bug bugs in IDF

Comments

@kevinhikaruevans
Copy link
Contributor

kevinhikaruevans commented Jul 19, 2023

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v5.0.0

Operating System used.

Linux

How did you build your project?

Other (please specify in More Information)

If you are using Windows, please specify command line type.

None

Development Kit.

esp32-s3-devkitc-1

Power Supply used.

USB

What is the expected behavior?

If it returns ESP_OK, it should write the query string to the buffer passed in.

If it is not supported with websockets, it should return an error code.

What is the actual behavior?

It does not write anything to the passed buffer (or maybe it's writing an empty string).

Steps to reproduce.

  1. Create HTTP server with a websocket endpoint
  2. Open a websocket with a query string on the ws endpoint, e.g. /ws?something=abcd
  3. After receiving a frame, observe that httpd_req_get_url_query_len(req) > 0 and httpd_req_get_url_query_str == ESP_OK, but the query string is empty

Debug Logs.

N/A

More Information.

Edit: This only seems to happen when receiving a frame. Prior to the handshake, it seems that the query string is accessible.

@kevinhikaruevans kevinhikaruevans added the Type: Bug bugs in IDF label Jul 19, 2023
@espressif-bot espressif-bot added the Status: Opened Issue is new label Jul 19, 2023
@github-actions github-actions bot changed the title httpd_req_get_url_query_str does not work with websockets httpd_req_get_url_query_str does not work with websockets (IDFGH-10685) Jul 19, 2023
@kevinhikaruevans
Copy link
Contributor Author

kevinhikaruevans commented Jul 19, 2023

Here's roughly the code that I'm using:

static esp_err_t ws_get_identifier(httpd_req_t* req, char* dest, size_t dest_size) {
    if (req == NULL) {
        return ESP_OK;
    }

    char buffer[128] = {0};
    size_t buffer_length = 0;

    // first check query string
    buffer_length = httpd_req_get_url_query_len(req);

    if (buffer_length > sizeof(buffer)) {
        // TODO: off by 1 err here?
        ESP_LOGW(TAG, "query length exceeds buffer length");
        return ESP_ERR_INVALID_SIZE;
    }

    if (buffer_length > 0) {
        ESP_RETURN_ON_ERROR(httpd_req_get_url_query_str(req, buffer, buffer_length + 1), TAG, "cannot get query string");

        ESP_LOGI(TAG, "query string = '%s'", buffer);  // this is printing "query string = ''"
    }

    return ESP_OK;
}

@chipweinberger
Copy link
Contributor

chipweinberger commented Jul 19, 2023

Ya it's pretty annoying and should be improved imo. I use a workaround using the user_ctx to store the uri.

    httpd_uri_t http_uri = {
        .uri = uri,
        .method = ....,
        .handler =  ....,
        .user_ctx = uri,
        .is_websocket = true,
        .handle_ws_control_frames = ...,
        .supported_subprotocol = ...,
    };

    esp_err_t err = httpd_register_uri_handler(server, &http_uri);

It's not super easy to fix this in ESP-IDF. The URI is dynamic. We only know the URI when the websocket is first opened because it is sent in the HTTP request header. ESP-IDF would need to store this dynamic URI on the heap somewhere, and then free it when the websocket is closed.

If you were to open a PR to fix it, the most natural place to store this would be in the sock_db struct, by adding a char* ws_uri variable.

struct sock_db {
    int fd;                                 /*!< The file descriptor for this socket */
    void *ctx;                              /*!< A custom context for this socket */
    bool ignore_sess_ctx_changes;           /*!< Flag indicating if session context changes should be ignored */
    void *transport_ctx;                    /*!< A custom 'transport' context for this socket, to be used by send/recv/pending */
    httpd_handle_t handle;                  /*!< Server handle */
    httpd_free_ctx_fn_t free_ctx;      /*!< Function for freeing the context */
    httpd_free_ctx_fn_t free_transport_ctx; /*!< Function for freeing the 'transport' context */
    httpd_send_func_t send_fn;              /*!< Send function for this socket */
    httpd_recv_func_t recv_fn;              /*!< Receive function for this socket */
    httpd_pending_func_t pending_fn;        /*!< Pending function for this socket */
    uint64_t lru_counter;                   /*!< LRU Counter indicating when the socket was last used */
    bool lru_socket;                        /*!< Flag indicating LRU socket */
    char pending_data[PARSER_BLOCK_SIZE];   /*!< Buffer for pending data to be received */
    size_t pending_len;                     /*!< Length of pending data to be received */
    bool for_async_req;                     /*!< If true, the socket will not be LRU purged */
#ifdef CONFIG_HTTPD_WS_SUPPORT
    bool ws_handshake_done;                 /*!< True if it has done WebSocket handshake (if this socket is a valid WS) */
    bool ws_close;                          /*!< Set to true to close the socket later (when WS Close frame received) */
    esp_err_t (*ws_handler)(httpd_req_t *r);   /*!< WebSocket handler, leave to null if it's not WebSocket */
    bool ws_control_frames;                         /*!< WebSocket flag indicating that control frames should be passed to user handlers */
    void *ws_user_ctx;                         /*!< Pointer to user context data which will be available to handler for websocket*/
#endif
};

@ESP-YJM
Copy link
Collaborator

ESP-YJM commented Oct 18, 2023

@kevinhikaruevans Any update for this issue. I think @chipweinberger solution could solve your issue.

@espressif-bot espressif-bot added Status: In Progress Work is in progress Awaiting Response awaiting a response from the author and removed Status: Opened Issue is new labels Oct 18, 2023
@kevinhikaruevans
Copy link
Contributor Author

kevinhikaruevans commented Oct 18, 2023

I found an alternative work around.

I think httpd_req_get_url_query_str should not return ESP_OK in this case however, since it's not actually working. Maybe it should return ESP_ERR_HTTPD_RESULT_TRUNC or ESP_FAIL.

This behavior doesn't appear to be documented too.

@espressif-bot espressif-bot added Status: Opened Issue is new and removed Status: In Progress Work is in progress labels Aug 5, 2024
@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Response awaiting a response from the author Status: In Progress Work is in progress Type: Bug bugs in IDF
Projects
None yet
Development

No branches or pull requests

5 participants