Re: [PATCH] Configurable IPv4/6-status urls support for http-status response code 204 for online checks.
Hi Pasi,
Could you resend the patch, through git send-email, so it's under the
proper format?
With a proper commit message and so on.
My comments below. Mostly code style issues but also one fix that should
be in another patch:
---
diff --git a/include/setting.h b/include/setting.h
index a882021..32ccf56 100644
--- a/include/setting.h
+++ b/include/setting.h
@@ -28,6 +28,9 @@
extern C {
#endif
+#define CONF_STATUS_URL_IPV6Ipv6StatusUrl
+#define CONF_STATUS_URL_IPV4Ipv4StatusUrl
+
bool connman_setting_get_bool(const char *key);
char **connman_setting_get_string_list(const char *key);
unsigned int *connman_setting_get_uint_list(const char *key);
diff --git a/src/6to4.c b/src/6to4.c
index 0e3a7a1..c32cef9 100644
--- a/src/6to4.c
+++ b/src/6to4.c
@@ -53,8 +53,6 @@ static unsigned int newlink_watch;
static unsigned int newlink_flags;
static int newlink_timeout_id;
-#define STATUS_URL http://ipv6.connman.net/online/status.html;
-
#ifndef IP_DF
#define IP_DF 0x4000 /* Flag: Don't Fragment */
#endif
@@ -317,7 +315,9 @@ static void tun_newlink(unsigned flags, unsigned change,
void *user_data)
if (getenv(CONNMAN_WEB_DEBUG))
g_web_set_debug(web, web_debug, 6to4);
- web_request_id = g_web_request_get(web, STATUS_URL,
+ const char *url =
connman_option_get_string(CONF_STATUS_URL_IPV6);
Too long line: the limit is 80 characters.
+
+ web_request_id = g_web_request_get(web, url,
web_result, NULL, NULL);
two tabs more here (ok it was like that before, but since you are
refactoring this part, let's fix it)
newlink_timeout(NULL);
diff --git a/src/main.c b/src/main.c
index 4f635de..0151d39 100644
--- a/src/main.c
+++ b/src/main.c
@@ -73,6 +73,8 @@ static struct {
bool single_tech;
char **tethering_technologies;
bool persistent_tethering_mode;
+ char *ipv6_status_url;
+ char *ipv4_status_url;
} connman_settings = {
.bg_scan = true,
.pref_timeservers = NULL,
@@ -86,6 +88,8 @@ static struct {
.single_tech = false,
.tethering_technologies = NULL,
.persistent_tethering_mode = false,
+ .ipv4_status_url = NULL,
+ .ipv6_status_url = NULL,
};
#define CONF_BG_SCANBackgroundScanning
@@ -98,8 +102,10 @@ static struct {
#define CONF_BLACKLISTED_INTERFACES NetworkInterfaceBlacklist
#define CONF_ALLOW_HOSTNAME_UPDATES AllowHostnameUpdates
#define CONF_SINGLE_TECHSingleConnectedTechnology
-#define CONF_TETHERING_TECHNOLOGIES TetheringTechnologies
+#define CONF_TETHERING_TECHNOLOGIES TetheringTechnologies
#define CONF_PERSISTENT_TETHERING_MODE PersistentTetheringMode
+#define CONF_STATUS_URL_IPV6Ipv6StatusUrl
+#define CONF_STATUS_URL_IPV4Ipv4StatusUrl
static const char *supported_options[] = {
CONF_BG_SCAN,
@@ -114,6 +120,8 @@ static const char *supported_options[] = {
CONF_SINGLE_TECH,
CONF_TETHERING_TECHNOLOGIES,
CONF_PERSISTENT_TETHERING_MODE,
+ CONF_STATUS_URL_IPV4,
+ CONF_STATUS_URL_IPV6,
NULL
};
@@ -236,6 +244,8 @@ static void parse_config(GKeyFile *config)
char **interfaces;
char **str_list;
char **tethering;
+ char *ipv4url;
+ char *ipv6url;
You can use only one variable named status_url here (for instance), no
need to specify 2 different ones.
gsize len;
int timeout;
@@ -354,6 +364,23 @@ static void parse_config(GKeyFile *config)
connman_settings.persistent_tethering_mode = boolean;
g_clear_error(error);
+
+ipv4url = __connman_config_get_string(config, General,
CONF_STATUS_URL_IPV4, error);
+if (!error)
+connman_settings.ipv4_status_url = ipv4url;
+else
+connman_settings.ipv4_status_url =
http://ipv4.connman.net/online/status.html;;
+
+g_clear_error(error);
+
+ipv6url = __connman_config_get_string(config, General,
CONF_STATUS_URL_IPV6, error);
+if (!error)
+connman_settings.ipv6_status_url = ipv6url;
+else
+connman_settings.ipv6_status_url =
http://ipv6.connman.net/online/status.html;;
+
+g_clear_error(error);
Too long lines here too.
+
}
static int config_init(const char *file)
@@ -510,6 +537,11 @@ const char *connman_option_get_string(const char *key)
else
return option_wifi;
}
One empty line needed here
+ if (g_str_equal(key, CONF_STATUS_URL_IPV4) == TRUE) {
+ return connman_settings.ipv4_status_url;
+ }
No need of the { }
and add an empty line also
+ if (g_str_equal(key, CONF_STATUS_URL_IPV6) == TRUE)
+ return connman_settings.ipv6_status_url;
As you did
[PATCH] Configurable IPv4/6-status urls support for http-status response code 204 for online checks.
Hi,
here is a patch which provides configurable IPv4/6-status urls for ConnMan
together with support for using http-status response code 204 instead of the
default X-ConnMan-Status: online-check.
Why? Unfortunately there are numerous of ISP's or organizations which want to
modify/strip X-ConnMan-Status-header away from the http-responses even the
connection is otherwise just fine (eg. no portals). However they do not modify
http-status response codes and that's when the 204 comes in handy. With this
patch ConnMan-users can use alternative way to check whether their devices are
actually online.
Until ipv4.connman.net and ipv6.connman.net provides urls which return 204, you
can try this with these two:
http://ipv4.jolla.com/return_204
http://ipv6.jolla.com/return_204
Br,
Pasi Sjöholm
---
diff --git a/include/setting.h b/include/setting.h
index a882021..32ccf56 100644
--- a/include/setting.h
+++ b/include/setting.h
@@ -28,6 +28,9 @@
extern C {
#endif
+#define CONF_STATUS_URL_IPV6Ipv6StatusUrl
+#define CONF_STATUS_URL_IPV4Ipv4StatusUrl
+
bool connman_setting_get_bool(const char *key);
char **connman_setting_get_string_list(const char *key);
unsigned int *connman_setting_get_uint_list(const char *key);
diff --git a/src/6to4.c b/src/6to4.c
index 0e3a7a1..c32cef9 100644
--- a/src/6to4.c
+++ b/src/6to4.c
@@ -53,8 +53,6 @@ static unsigned int newlink_watch;
static unsigned int newlink_flags;
static int newlink_timeout_id;
-#define STATUS_URL http://ipv6.connman.net/online/status.html;
-
#ifndef IP_DF
#define IP_DF 0x4000 /* Flag: Don't Fragment */
#endif
@@ -317,7 +315,9 @@ static void tun_newlink(unsigned flags, unsigned change,
void *user_data)
if (getenv(CONNMAN_WEB_DEBUG))
g_web_set_debug(web, web_debug, 6to4);
- web_request_id = g_web_request_get(web, STATUS_URL,
+ const char *url =
connman_option_get_string(CONF_STATUS_URL_IPV6);
+
+ web_request_id = g_web_request_get(web, url,
web_result, NULL, NULL);
newlink_timeout(NULL);
diff --git a/src/main.c b/src/main.c
index 4f635de..0151d39 100644
--- a/src/main.c
+++ b/src/main.c
@@ -73,6 +73,8 @@ static struct {
bool single_tech;
char **tethering_technologies;
bool persistent_tethering_mode;
+ char *ipv6_status_url;
+ char *ipv4_status_url;
} connman_settings = {
.bg_scan = true,
.pref_timeservers = NULL,
@@ -86,6 +88,8 @@ static struct {
.single_tech = false,
.tethering_technologies = NULL,
.persistent_tethering_mode = false,
+ .ipv4_status_url = NULL,
+ .ipv6_status_url = NULL,
};
#define CONF_BG_SCANBackgroundScanning
@@ -98,8 +102,10 @@ static struct {
#define CONF_BLACKLISTED_INTERFACES NetworkInterfaceBlacklist
#define CONF_ALLOW_HOSTNAME_UPDATES AllowHostnameUpdates
#define CONF_SINGLE_TECHSingleConnectedTechnology
-#define CONF_TETHERING_TECHNOLOGIES TetheringTechnologies
+#define CONF_TETHERING_TECHNOLOGIES TetheringTechnologies
#define CONF_PERSISTENT_TETHERING_MODE PersistentTetheringMode
+#define CONF_STATUS_URL_IPV6Ipv6StatusUrl
+#define CONF_STATUS_URL_IPV4Ipv4StatusUrl
static const char *supported_options[] = {
CONF_BG_SCAN,
@@ -114,6 +120,8 @@ static const char *supported_options[] = {
CONF_SINGLE_TECH,
CONF_TETHERING_TECHNOLOGIES,
CONF_PERSISTENT_TETHERING_MODE,
+ CONF_STATUS_URL_IPV4,
+ CONF_STATUS_URL_IPV6,
NULL
};
@@ -236,6 +244,8 @@ static void parse_config(GKeyFile *config)
char **interfaces;
char **str_list;
char **tethering;
+ char *ipv4url;
+ char *ipv6url;
gsize len;
int timeout;
@@ -354,6 +364,23 @@ static void parse_config(GKeyFile *config)
connman_settings.persistent_tethering_mode = boolean;
g_clear_error(error);
+
+ipv4url = __connman_config_get_string(config, General,
CONF_STATUS_URL_IPV4, error);
+if (!error)
+connman_settings.ipv4_status_url = ipv4url;
+else
+connman_settings.ipv4_status_url =
http://ipv4.connman.net/online/status.html;;
+
+g_clear_error(error);
+
+ipv6url = __connman_config_get_string(config, General,
CONF_STATUS_URL_IPV6, error);
+if (!error)
+connman_settings.ipv6_status_url = ipv6url;
+else
+connman_settings.ipv6_status_url =
http://ipv6.connman.net/online/status.html;;
+
+g_clear_error(error);
+
}
static int config_init(const char *file)
@@ -510,6 +537,11 @@ const char *connman_option_get_string(const char *key)
else
return option_wifi;
}
+ if (g_str_equal(key, CONF_STATUS_URL_IPV4) == TRUE) {
+
