[PATCH 1/2] gprs-context: set apn length to 100 bytes

[PATCH 1/2] gprs-context: set apn length to 100 bytes

[Alexander Couzens]

[-- Attachment #1: Type: text/plain, Size: 553 bytes --]

According to ETSI TS 123 003 version 9.15.0 Chapter 9.1
---
 include/gprs-context.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/gprs-context.h b/include/gprs-context.h
index ab673265..7e163cc9 100644
--- a/include/gprs-context.h
+++ b/include/gprs-context.h
@@ -30,7 +30,7 @@ extern "C" {
 
 struct ofono_gprs_context;
 
-#define OFONO_GPRS_MAX_APN_LENGTH 127
+#define OFONO_GPRS_MAX_APN_LENGTH 100
 #define OFONO_GPRS_MAX_USERNAME_LENGTH 63
 #define OFONO_GPRS_MAX_PASSWORD_LENGTH 255
 
-- 
2.13.4

[PATCH 2/2] common.c: move strlen(apn) check into is_valid_apn()

[Alexander Couzens]

[-- Attachment #1: Type: text/plain, Size: 2266 bytes --]

---
 src/common.c | 5 +++++
 src/gprs.c   | 9 ---------
 src/lte.c    | 3 ---
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/src/common.c b/src/common.c
index 17d1d58a..013e2452 100644
--- a/src/common.c
+++ b/src/common.c
@@ -31,8 +31,10 @@
 
 #include <ofono/types.h>
 #include "common.h"
+#include "gprs-context.h"
 #include "util.h"
 
+
 struct error_entry {
 	int error;
 	const char *str;
@@ -705,6 +707,9 @@ gboolean is_valid_apn(const char *apn)
 	if (apn[0] == '.' || apn[0] == '\0')
 		return FALSE;
 
+	if (strlen(apn) > OFONO_GPRS_MAX_APN_LENGTH)
+		return FALSE;
+
 	for (i = 0; apn[i] != '\0'; i++) {
 		if (g_ascii_isalnum(apn[i]))
 			continue;
diff --git a/src/gprs.c b/src/gprs.c
index 098ba3d4..a4132cc0 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -1027,9 +1027,6 @@ static DBusMessage *pri_set_apn(struct pri_context *ctx, DBusConnection *conn,
 {
 	GKeyFile *settings = ctx->gprs->settings;
 
-	if (strlen(apn) > OFONO_GPRS_MAX_APN_LENGTH)
-		return __ofono_error_invalid_format(msg);
-
 	if (g_str_equal(apn, ctx->context.apn))
 		return dbus_message_new_method_return(msg);
 
@@ -2376,9 +2373,6 @@ static void provision_context(const struct ofono_gprs_provision_data *ap,
 	if (ap->name && strlen(ap->name) > MAX_CONTEXT_NAME_LENGTH)
 		return;
 
-	if (ap->apn == NULL || strlen(ap->apn) > OFONO_GPRS_MAX_APN_LENGTH)
-		return;
-
 	if (is_valid_apn(ap->apn) == FALSE)
 		return;
 
@@ -3228,9 +3222,6 @@ static gboolean load_context(struct ofono_gprs *gprs, const char *group)
 	if (apn == NULL)
 		goto error;
 
-	if (strlen(apn) > OFONO_GPRS_MAX_APN_LENGTH)
-		goto error;
-
 	if (type == OFONO_GPRS_CONTEXT_TYPE_MMS) {
 		msgproxy = g_key_file_get_string(gprs->settings, group,
 						"MessageProxy", NULL);
diff --git a/src/lte.c b/src/lte.c
index 70e0c18a..9b20749c 100644
--- a/src/lte.c
+++ b/src/lte.c
@@ -152,9 +152,6 @@ static DBusMessage *lte_set_default_apn(struct ofono_lte *lte,
 	if (lte->pending)
 		return __ofono_error_busy(msg);
 
-	if (strlen(apn) > OFONO_GPRS_MAX_APN_LENGTH)
-		return __ofono_error_invalid_format(msg);
-
 	if (g_str_equal(apn, lte->info.apn))
 		return dbus_message_new_method_return(msg);
 
-- 
2.13.4

Re: [PATCH 1/2] gprs-context: set apn length to 100 bytes

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 336 bytes --]

Hi Alexander,

On 08/08/2017 08:53 AM, Alexander Couzens wrote:
> According to ETSI TS 123 003 version 9.15.0 Chapter 9.1
> ---
>   include/gprs-context.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 

Applied, thanks.  I added a comment about the relevant section in a 
follow on commit.

Regards,
-Denis

Re: [PATCH 2/2] common.c: move strlen(apn) check into is_valid_apn()

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]

Hi Alexander,

On 08/08/2017 08:53 AM, Alexander Couzens wrote:
> ---
>   src/common.c | 5 +++++
>   src/gprs.c   | 9 ---------
>   src/lte.c    | 3 ---
>   3 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/src/common.c b/src/common.c
> index 17d1d58a..013e2452 100644
> --- a/src/common.c
> +++ b/src/common.c
> @@ -31,8 +31,10 @@
>   
>   #include <ofono/types.h>
>   #include "common.h"
> +#include "gprs-context.h"
>   #include "util.h"
>   
> +

Stray whitespace here...

>   struct error_entry {
>   	int error;
>   	const char *str;
> @@ -705,6 +707,9 @@ gboolean is_valid_apn(const char *apn)
>   	if (apn[0] == '.' || apn[0] == '\0')
>   		return FALSE;
>   
> +	if (strlen(apn) > OFONO_GPRS_MAX_APN_LENGTH)
> +		return FALSE;
> +
>   	for (i = 0; apn[i] != '\0'; i++) {
>   		if (g_ascii_isalnum(apn[i]))
>   			continue;
> diff --git a/src/gprs.c b/src/gprs.c
> index 098ba3d4..a4132cc0 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -1027,9 +1027,6 @@ static DBusMessage *pri_set_apn(struct pri_context *ctx, DBusConnection *conn,
>   {
>   	GKeyFile *settings = ctx->gprs->settings;
>   
> -	if (strlen(apn) > OFONO_GPRS_MAX_APN_LENGTH)
> -		return __ofono_error_invalid_format(msg);
> -
>   	if (g_str_equal(apn, ctx->context.apn))
>   		return dbus_message_new_method_return(msg);
>   
> @@ -2376,9 +2373,6 @@ static void provision_context(const struct ofono_gprs_provision_data *ap,
>   	if (ap->name && strlen(ap->name) > MAX_CONTEXT_NAME_LENGTH)
>   		return;
>   
> -	if (ap->apn == NULL || strlen(ap->apn) > OFONO_GPRS_MAX_APN_LENGTH)
> -		return;
> -

Don't we still need to check for ap->apn being NULL?

>   	if (is_valid_apn(ap->apn) == FALSE)
>   		return;
>   

Regards,
-Denis

Re: [PATCH 2/2] common.c: move strlen(apn) check into is_valid_apn()

[Alexander Couzens]

[-- Attachment #1: Type: text/plain, Size: 132 bytes --]

Hi Dennis,

> 
> Don't we still need to check for ap->apn being NULL?
Right. Will send a v2 of this patch.

> Regards,
Alex

[-- Attachment #2: attachment.sig --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH][v2] common.c: move strlen(apn) check into is_valid_apn()

[Alexander Couzens]

[-- Attachment #1: Type: text/plain, Size: 2372 bytes --]

---
 src/common.c | 8 ++++++++
 src/gprs.c   | 9 ---------
 src/lte.c    | 3 ---
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/src/common.c b/src/common.c
index 17d1d58a0be3..46b59223272e 100644
--- a/src/common.c
+++ b/src/common.c
@@ -31,8 +31,10 @@
 
 #include <ofono/types.h>
 #include "common.h"
+#include "gprs-context.h"
 #include "util.h"
 
+
 struct error_entry {
 	int error;
 	const char *str;
@@ -702,9 +704,15 @@ gboolean is_valid_apn(const char *apn)
 	int i;
 	int last_period = 0;
 
+	if (apn == NULL)
+		return FALSE;
+
 	if (apn[0] == '.' || apn[0] == '\0')
 		return FALSE;
 
+	if (strlen(apn) > OFONO_GPRS_MAX_APN_LENGTH)
+		return FALSE;
+
 	for (i = 0; apn[i] != '\0'; i++) {
 		if (g_ascii_isalnum(apn[i]))
 			continue;
diff --git a/src/gprs.c b/src/gprs.c
index 098ba3d49155..a4132cc06760 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -1027,9 +1027,6 @@ static DBusMessage *pri_set_apn(struct pri_context *ctx, DBusConnection *conn,
 {
 	GKeyFile *settings = ctx->gprs->settings;
 
-	if (strlen(apn) > OFONO_GPRS_MAX_APN_LENGTH)
-		return __ofono_error_invalid_format(msg);
-
 	if (g_str_equal(apn, ctx->context.apn))
 		return dbus_message_new_method_return(msg);
 
@@ -2376,9 +2373,6 @@ static void provision_context(const struct ofono_gprs_provision_data *ap,
 	if (ap->name && strlen(ap->name) > MAX_CONTEXT_NAME_LENGTH)
 		return;
 
-	if (ap->apn == NULL || strlen(ap->apn) > OFONO_GPRS_MAX_APN_LENGTH)
-		return;
-
 	if (is_valid_apn(ap->apn) == FALSE)
 		return;
 
@@ -3228,9 +3222,6 @@ static gboolean load_context(struct ofono_gprs *gprs, const char *group)
 	if (apn == NULL)
 		goto error;
 
-	if (strlen(apn) > OFONO_GPRS_MAX_APN_LENGTH)
-		goto error;
-
 	if (type == OFONO_GPRS_CONTEXT_TYPE_MMS) {
 		msgproxy = g_key_file_get_string(gprs->settings, group,
 						"MessageProxy", NULL);
diff --git a/src/lte.c b/src/lte.c
index 70e0c18a0027..9b20749c6f9a 100644
--- a/src/lte.c
+++ b/src/lte.c
@@ -152,9 +152,6 @@ static DBusMessage *lte_set_default_apn(struct ofono_lte *lte,
 	if (lte->pending)
 		return __ofono_error_busy(msg);
 
-	if (strlen(apn) > OFONO_GPRS_MAX_APN_LENGTH)
-		return __ofono_error_invalid_format(msg);
-
 	if (g_str_equal(apn, lte->info.apn))
 		return dbus_message_new_method_return(msg);
 
-- 
2.14.1

Re: [PATCH][v2] common.c: move strlen(apn) check into is_valid_apn()

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 665 bytes --]

Hi Alexander,

On 09/05/2017 09:27 AM, Alexander Couzens wrote:
> ---
>   src/common.c | 8 ++++++++
>   src/gprs.c   | 9 ---------
>   src/lte.c    | 3 ---
>   3 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/src/common.c b/src/common.c
> index 17d1d58a0be3..46b59223272e 100644
> --- a/src/common.c
> +++ b/src/common.c
> @@ -31,8 +31,10 @@
>   
>   #include <ofono/types.h>
>   #include "common.h"
> +#include "gprs-context.h"
>   #include "util.h"
>   
> +
>   struct error_entry {
>   	int error;
>   	const char *str;

I squashed the above spurious empty line and applied this patch.  Thanks.

Regards,
-Denis