Skip to content

Commit

Permalink
net: lib: downloader: limit number of redirects
Browse files Browse the repository at this point in the history
Limit the number of redirects before downloader fails the download.

Co-authored-by: Seppo Takalo <seppo.takalo@nordicsemi.no>
Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>
  • Loading branch information
eivindj-nordic and SeppoTakalo committed Feb 20, 2025
1 parent ab80a43 commit 9785857
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 1 deletion.
6 changes: 6 additions & 0 deletions include/net/downloader.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ enum downloader_evt_id {
* - -EAFNOSUPPORT: Unsupported address family (IPv4/IPv6).
* - -EHOSTUNREACH: Failed to resolve the target address.
* - -EMSGSIZE: TLS packet is larger than the nRF91 Modem can handle.
* - -EMLINK: Maximum number of redirects reached.
*
* In case of @c ECONNRESET errors, returning zero from the callback will let the
* library attempt to reconnect to the server and download the last fragment again.
Expand Down Expand Up @@ -196,6 +197,11 @@ struct downloader_host_cfg {
* This string is used in case you are requesting a proxied file from a CoAP server.
*/
const char *proxy_uri;
/**
* Maximum number of HTTP redirects.
* Use 0 to set the value of CONFIG_DOWNLOADER_MAX_REDIRECTS.
*/
uint8_t redirects_max;
};

/**
Expand Down
7 changes: 7 additions & 0 deletions subsys/net/lib/downloader/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ config DOWNLOADER_MAX_FILENAME_SIZE
range 8 2048
default 255

config DOWNLOADER_MAX_REDIRECTS
int "Number of redirects before download is aborted"
range 0 255
default 10
help
The maximum number of redirects can be overwritten in the host config.

config DOWNLOADER_SHELL
bool "Download client shell"
depends on SHELL
Expand Down
4 changes: 4 additions & 0 deletions subsys/net/lib/downloader/src/downloader.c
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,10 @@ static int downloader_start(struct downloader *dl, const struct downloader_host_
dl->buf_offset = 0;
dl->complete = false;

if (dl->host_cfg.redirects_max == 0) {
dl->host_cfg.redirects_max = CONFIG_DOWNLOADER_MAX_REDIRECTS;
}

dl->transport = NULL;
STRUCT_SECTION_FOREACH(dl_transport_entry, entry)
{
Expand Down
9 changes: 9 additions & 0 deletions subsys/net/lib/downloader/src/transports/http.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ struct transport_params_http {

/** Request new data */
bool new_data_req;
/** Redirect retries */
uint8_t redirects;
};

BUILD_ASSERT(CONFIG_DOWNLOADER_TRANSPORT_PARAMS_SIZE >= sizeof(struct transport_params_http));
Expand Down Expand Up @@ -293,6 +295,13 @@ static int http_header_parse(struct downloader *dl, size_t buf_len)
return -EBADMSG;
}

http->redirects++;

if (http->redirects > dl->host_cfg.redirects_max) {
LOG_ERR("Maximum redirections reached, aborting");
return -EMLINK;
}

return -ECONNRESET;
}
}
Expand Down
1 change: 1 addition & 0 deletions tests/subsys/net/lib/downloader/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,5 @@ target_compile_options(app
-DCONFIG_COAP_INIT_ACK_TIMEOUT_MS=100
-DCONFIG_COAP_BACKOFF_PERCENT=5
-DCONFIG_COAP_BLOCK_SIZE=5
-DCONFIG_DOWNLOADER_MAX_REDIRECTS=1
)
29 changes: 28 additions & 1 deletion tests/subsys/net/lib/downloader/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,8 @@ static ssize_t z_impl_zsock_recvfrom_http_redirect_and_close(
return 0;
}

return 0;
memcpy(buf, HTTP_HDR_REDIRECT, strlen(HTTP_HDR_REDIRECT));
return strlen(HTTP_HDR_REDIRECT);
}

int z_impl_zsock_socket_http_then_https(int family, int type, int proto)
Expand Down Expand Up @@ -1395,6 +1396,32 @@ void test_downloader_get_https_partial_content_partial_2nd_header(void)
dl_wait_for_event(DOWNLOADER_EVT_DEINITIALIZED, K_SECONDS(1));
}

void test_downloader_https_unlimited_redirect(void)
{
int err;
struct downloader_evt evt;

err = downloader_init(&dl, &dl_cfg);
TEST_ASSERT_EQUAL(0, err);

zsock_getaddrinfo_fake.custom_fake = zsock_getaddrinfo_server_ok;
zsock_freeaddrinfo_fake.custom_fake = zsock_freeaddrinfo_server_ipv6;
z_impl_zsock_socket_fake.custom_fake = z_impl_zsock_socket_https_ipv6_ok;
z_impl_zsock_connect_fake.custom_fake = z_impl_zsock_connect_ipv6_ok;
z_impl_zsock_setsockopt_fake.custom_fake = z_impl_zsock_setsockopt_https_ok;
z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_ok;
z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_http_redirect_and_close;


err = downloader_get(&dl, &dl_host_conf_w_sec_tags, HTTPS_URL, 0);
TEST_ASSERT_EQUAL(0, err);

evt = dl_wait_for_event(DOWNLOADER_EVT_ERROR, K_SECONDS(3));

downloader_deinit(&dl);
dl_wait_for_event(DOWNLOADER_EVT_DEINITIALIZED, K_SECONDS(1));
}

void test_downloader_get_http_connect_enetunreach(void)
{
int err;
Expand Down

0 comments on commit 9785857

Please sign in to comment.