fix: Prevent Invalid location requests

simensrostad · simensrostad · commit b5443d7c4266 · 2025-10-31T18:08:27.000+01:00
Fixes test_shell failure caused by invalid Wi-Fi location data.

Root cause:
The location library can send cloud location requests with Wi-Fi data
containing 0 access points, causing CBOR encoding failures (error
13/EINVAL).

Occasionally, encoding of Wi-Fi APs fails when the maximum number of
APs (10) is obtained. This is likely due to an encoding buffer size
issue in the nRF Cloud encoding libraries.

Changes:
- Add validation in location module to reject cloud location requests
  from the location library that have no valid cellular or Wi-Fi data,
  preventing invalid requests from reaching the cloud module.

- Add validation in cloud module to check Wi-Fi data has cnt &gt; 0
  before including it in location requests, preventing CBOR encoding
  errors.

- Update tests and lower maximum
  CONFIG_LOCATION_METHOD_WIFI_SCANNING_RESULTS_MAX_CNT to 8 to prevent
  encoding errors seen when finding 10 APs due to potential memory
  issue with CBOR buffer being too low (LOCATION_GET_CBOR_MAX_SIZE
  hardcoded to 1024).

Signed-off-by: Simen S. Røstad &lt;simen.rostad@nordicsemi.no&gt;
diff --git a/app/boards/thingy91x_nrf9151_ns.conf b/app/boards/thingy91x_nrf9151_ns.conf
@@ -46,9 +46,9 @@ CONFIG_LOCATION_REQUEST_DEFAULT_WIFI_TIMEOUT=10000
 CONFIG_LOCATION_WORKQUEUE_STACK_SIZE=4096
 
 # Align this with CONFIG_NRF_WIFI_SCAN_MAX_BSS_CNT
-CONFIG_LOCATION_METHOD_WIFI_SCANNING_RESULTS_MAX_CNT=10
+CONFIG_LOCATION_METHOD_WIFI_SCANNING_RESULTS_MAX_CNT=8
 # Align this with CONFIG_LOCATION_METHOD_WIFI_SCANNING_RESULTS_MAX_CNT
-CONFIG_NRF_WIFI_SCAN_MAX_BSS_CNT=10
+CONFIG_NRF_WIFI_SCAN_MAX_BSS_CNT=8
 
 # Debug logging
 CONFIG_APP_POWER_LOG_LEVEL_DBG=y
diff --git a/app/src/modules/cloud/cloud_location.c b/app/src/modules/cloud/cloud_location.c
@@ -63,13 +63,15 @@ static void handle_cloud_location_request(const struct location_data_cloud *requ
 #endif
 
 #if defined(CONFIG_LOCATION_METHOD_WIFI)
-	if (request->wifi_data != NULL) {
+	if (request->wifi_data != NULL && request->wifi_data->cnt > 0) {
 		/* Cast away const: nRF Cloud API limitation - struct fields are non-const
 		 * but the data is not modified by the API
 		 */
 		loc_req.wifi_info = (struct wifi_scan_info *)request->wifi_data; /* NOSONAR */
 
 		LOG_DBG("Wi-Fi data present: %d APs", request->wifi_data->cnt);
+	} else if (request->wifi_data != NULL && request->wifi_data->cnt == 0) {
+		LOG_DBG("Wi-Fi scan found 0 APs, omitting Wi-Fi data from location request");
 	}
 #endif
 
diff --git a/app/src/modules/location/location.c b/app/src/modules/location/location.c
@@ -415,10 +415,39 @@ static void location_event_handler(const struct location_event_data *event_data)
 
 		location_print_data_details(event_data->method, &event_data->fallback.details);
 		break;
-	case LOCATION_EVT_CLOUD_LOCATION_EXT_REQUEST:
+	case LOCATION_EVT_CLOUD_LOCATION_EXT_REQUEST: {
 		LOG_DBG("Cloud location request received from location library");
 
-		/* Cancel the current location request since we're now using cloud-based location */
+		bool has_valid_data = false;
+
+#if defined(CONFIG_LOCATION_METHOD_CELLULAR)
+		if (event_data->cloud_location_request.cell_data != NULL) {
+			has_valid_data = true;
+			LOG_DBG("Cellular data present");
+		}
+#endif
+#if defined(CONFIG_LOCATION_METHOD_WIFI)
+		if (event_data->cloud_location_request.wifi_data != NULL &&
+		    event_data->cloud_location_request.wifi_data->cnt > 0) {
+			has_valid_data = true;
+			LOG_DBG("Wi-Fi data present: %d APs",
+				event_data->cloud_location_request.wifi_data->cnt);
+		}
+#endif
+
+		if (!has_valid_data) {
+			LOG_WRN("Cloud location request has no valid cellular or Wi-Fi data, "
+				"letting the library fall back to the next method");
+			/* Send error result to allow fallback to next method */
+			location_cloud_location_ext_result_set(LOCATION_EXT_RESULT_ERROR, NULL);
+			break;
+		}
+
+		/* Cancel the current location request to avoid falling back to the next
+		 * location source. Treat the fact that we have found Wi-Fi APs and/or cellular data
+		 * as a successful location request, even if we don't know whether the
+		 * cloud is able to resolve data to a location or not.
+		 */
 		int err = location_request_cancel();
 
 		if (err) {
@@ -430,6 +459,7 @@ static void location_event_handler(const struct location_event_data *event_data)
 		cloud_request_send(&event_data->cloud_location_request);
 		status_send(LOCATION_SEARCH_DONE);
 		break;
+	}
 #if defined(CONFIG_NRF_CLOUD_AGNSS)
 	case LOCATION_EVT_GNSS_ASSISTANCE_REQUEST:
 		LOG_DBG("A-GNSS assistance request received from location library");
diff --git a/tests/module/cloud/src/cloud_module_test.c b/tests/module/cloud/src/cloud_module_test.c
@@ -246,13 +246,15 @@ void setUp(void)
 	RESET_FAKE(nrf_cloud_coap_disconnect);
 	RESET_FAKE(nrf_cloud_coap_json_message_send);
 	RESET_FAKE(nrf_cloud_coap_location_send);
+	RESET_FAKE(nrf_cloud_coap_location_get);
 	RESET_FAKE(nrf_cloud_coap_shadow_get);
 	RESET_FAKE(nrf_cloud_coap_patch);
 	RESET_FAKE(date_time_now);
 	RESET_FAKE(nrf_provisioning_init);
 	RESET_FAKE(nrf_provisioning_trigger_manually);
 	RESET_FAKE(storage_batch_read);
 	RESET_FAKE(nrf_cloud_coap_sensor_send);
+	RESET_FAKE(location_cloud_location_ext_result_set);
 
 	nrf_cloud_client_id_get_fake.custom_fake = nrf_cloud_client_id_get_custom_fake;
 	nrf_provisioning_init_fake.custom_fake = nrf_provisioning_init_custom_fake;
@@ -263,6 +265,7 @@ void setUp(void)
 
 	/* Set default return values */
 	nrf_cloud_coap_location_send_fake.return_val = 0;
+	nrf_cloud_coap_location_get_fake.return_val = 0;
 	storage_batch_read_fake.return_val = -EAGAIN;
 
 	/* Clear all channels */
@@ -608,6 +611,161 @@ void test_gnss_location_data_handling(void)
 	}
 }
 
+/* Test cloud location request with valid Wi-Fi data */
+void test_cloud_location_request_with_valid_wifi(void)
+{
+	int err;
+	struct wifi_scan_result mock_aps[2] = {
+		{.ssid = {0}, .ssid_length = 0, .rssi = -50, .channel = 1},
+		{.ssid = {0}, .ssid_length = 0, .rssi = -60, .channel = 6}
+	};
+	struct wifi_scan_info mock_wifi_info = {
+		.ap_info = mock_aps,
+		.cnt = 2
+	};
+	struct location_data_cloud mock_cloud_request = {
+		.cell_data = NULL,
+		.wifi_data = &mock_wifi_info
+	};
+	struct location_msg location_msg = {
+		.type = LOCATION_CLOUD_REQUEST,
+		.cloud_request = mock_cloud_request
+	};
+	struct storage_msg passthrough_msg = {
+		.type = STORAGE_MODE_PASSTHROUGH
+	};
+	struct storage_msg storage_data_msg = {
+		.type = STORAGE_DATA,
+		.data_type = STORAGE_TYPE_LOCATION,
+		.data_len = sizeof(struct location_msg)
+	};
+
+	memcpy(storage_data_msg.buffer, &location_msg, sizeof(location_msg));
+
+	connect_cloud();
+
+	err = zbus_chan_pub(&STORAGE_CHAN, &passthrough_msg, K_NO_WAIT);
+	TEST_ASSERT_EQUAL(0, err);
+	k_sleep(K_MSEC(10));
+
+	/* Send cloud location request with valid Wi-Fi data */
+	err = zbus_chan_pub(&STORAGE_DATA_CHAN, &storage_data_msg, K_NO_WAIT);
+	TEST_ASSERT_EQUAL(0, err);
+	k_sleep(K_MSEC(PROCESSING_DELAY_MS));
+
+	/* Verify that location request was sent to nRF Cloud */
+	TEST_ASSERT_EQUAL(1, nrf_cloud_coap_location_get_fake.call_count);
+
+	/* Verify request was not NULL */
+	if (nrf_cloud_coap_location_get_fake.call_count > 0) {
+		TEST_ASSERT_NOT_NULL(nrf_cloud_coap_location_get_fake.arg0_val);
+	}
+}
+
+/* Test cloud location request with 0 Wi-Fi APs still sends request */
+void test_cloud_location_request_with_zero_wifi_aps(void)
+{
+	int err;
+	struct wifi_scan_info mock_wifi_info = {
+		.ap_info = NULL,
+		.cnt = 0
+	};
+	struct location_data_cloud mock_cloud_request = {
+		.cell_data = NULL,
+		.wifi_data = &mock_wifi_info
+	};
+	struct location_msg location_msg = {
+		.type = LOCATION_CLOUD_REQUEST,
+		.cloud_request = mock_cloud_request
+	};
+	struct storage_msg passthrough_msg = {
+		.type = STORAGE_MODE_PASSTHROUGH
+	};
+	struct storage_msg storage_data_msg = {
+		.type = STORAGE_DATA,
+		.data_type = STORAGE_TYPE_LOCATION,
+		.data_len = sizeof(struct location_msg)
+	};
+
+	memcpy(storage_data_msg.buffer, &location_msg, sizeof(location_msg));
+
+	connect_cloud();
+
+	err = zbus_chan_pub(&STORAGE_CHAN, &passthrough_msg, K_NO_WAIT);
+	TEST_ASSERT_EQUAL(0, err);
+	k_sleep(K_MSEC(10));
+
+	/* Send cloud location request with 0 Wi-Fi APs */
+	err = zbus_chan_pub(&STORAGE_DATA_CHAN, &storage_data_msg, K_NO_WAIT);
+	TEST_ASSERT_EQUAL(0, err);
+	k_sleep(K_MSEC(PROCESSING_DELAY_MS));
+
+	/* Verify that location request was still sent (Wi-Fi omitted internally) */
+	TEST_ASSERT_EQUAL(1, nrf_cloud_coap_location_get_fake.call_count);
+
+	/* Verify request was not NULL */
+	if (nrf_cloud_coap_location_get_fake.call_count > 0) {
+		TEST_ASSERT_NOT_NULL(nrf_cloud_coap_location_get_fake.arg0_val);
+	}
+}
+
+/* Test cloud location request with cellular data and 0 Wi-Fi APs */
+void test_cloud_location_request_cellular_with_zero_wifi_aps(void)
+{
+	int err;
+	struct lte_lc_cell mock_cell = {
+		.mcc = 242,
+		.mnc = 1,
+		.id = 12345,
+		.tac = 678
+	};
+	struct lte_lc_cells_info mock_cells_info = {
+		.current_cell = mock_cell,
+		.ncells_count = 0
+	};
+	struct wifi_scan_info mock_wifi_info = {
+		.ap_info = NULL,
+		.cnt = 0
+	};
+	struct location_data_cloud mock_cloud_request = {
+		.cell_data = &mock_cells_info,
+		.wifi_data = &mock_wifi_info
+	};
+	struct location_msg location_msg = {
+		.type = LOCATION_CLOUD_REQUEST,
+		.cloud_request = mock_cloud_request
+	};
+	struct storage_msg passthrough_msg = {
+		.type = STORAGE_MODE_PASSTHROUGH
+	};
+	struct storage_msg storage_data_msg = {
+		.type = STORAGE_DATA,
+		.data_type = STORAGE_TYPE_LOCATION,
+		.data_len = sizeof(struct location_msg)
+	};
+
+	memcpy(storage_data_msg.buffer, &location_msg, sizeof(location_msg));
+
+	connect_cloud();
+
+	err = zbus_chan_pub(&STORAGE_CHAN, &passthrough_msg, K_NO_WAIT);
+	TEST_ASSERT_EQUAL(0, err);
+	k_sleep(K_MSEC(10));
+
+	/* Send cloud location request with cellular data and 0 Wi-Fi APs */
+	err = zbus_chan_pub(&STORAGE_DATA_CHAN, &storage_data_msg, K_NO_WAIT);
+	TEST_ASSERT_EQUAL(0, err);
+	k_sleep(K_MSEC(PROCESSING_DELAY_MS));
+
+	/* Verify that location request was sent (cellular used, Wi-Fi omitted) */
+	TEST_ASSERT_EQUAL(1, nrf_cloud_coap_location_get_fake.call_count);
+
+	/* Verify request was not NULL */
+	if (nrf_cloud_coap_location_get_fake.call_count > 0) {
+		TEST_ASSERT_NOT_NULL(nrf_cloud_coap_location_get_fake.arg0_val);
+	}
+}
+
 void test_storage_data_battery_sent_to_cloud(void)
 {
 	int err;
diff --git a/tests/module/location/src/location_module_test.c b/tests/module/location/src/location_module_test.c
