[ABRT PATCH] abrt-*-client: simplify formatting of locale-related headers

[Denys Vlasenko]

Signed-off-by: Denys Vlasenko <dvlasenk-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
src/plugins/abrt-dedup-client.c | 24 ++++---
src/plugins/abrt-retrace-client.c | 134 ++++++++++++--------------------------
src/plugins/https-utils.c | 17 ++++-
src/plugins/https-utils.h | 4 +-
4 files changed, 72 insertions(+), 107 deletions(-)

diff --git a/src/plugins/abrt-dedup-client.c b/src/plugins/abrt-dedup-client.c
index 835d551..830efda 100644
--- a/src/plugins/abrt-dedup-client.c
+++ b/src/plugins/abrt-dedup-client.c
@@ -171,19 +171,17 @@ int main(int argc, char **argv)
"Host: %s\r\n"
"Content-Type: application/x-www-form-urlencoded\r\n"
"Content-Length: %d\r\n"
- "Connection: close\r\n",
- cfg.url, request_body->len);
-
- if (lang.encoding)
- strbuf_append_strf(request, "Accept-Charset: %s\r\n", lang.encoding);
-
- if (lang.locale)
- {
- strbuf_append_strf(request, "Accept-Language: %s\r\n", lang.locale);
- free(lang.locale);
- }
-
- strbuf_append_strf(request, "\r\n%s", request_body->buf);
+ "Connection: close\r\n"
+ "%s"
+ "%s"
+ "\r\n",
+ cfg.url,
+ request_body->len,
+ lang.accept_charset,
+ lang.accept_language
+ );
+
+ strbuf_append_str(request, request_body->buf);
strbuf_free(request_body);

/* Initialize NSS */
diff --git a/src/plugins/abrt-retrace-client.c b/src/plugins/abrt-retrace-client.c
index 940939b..cc0b802 100644
--- a/src/plugins/abrt-retrace-client.c
+++ b/src/plugins/abrt-retrace-client.c
@@ -29,6 +29,8 @@ enum
TASK_VMCORE,
};

+static struct language lang;
+
struct retrace_settings
{
int running_tasks;
@@ -369,8 +371,6 @@ static int check_package(const char *nvr, const char *arch, map_string_t *osinfo
{
char *releaseid = get_release_id(osinfo, arch);

- struct language lang;
- get_language(&lang);
PRFileDesc *tcp_sock, *ssl_sock;
ssl_connect(&cfg, &tcp_sock, &ssl_sock);
struct strbuf *http_request = strbuf_new();
@@ -382,21 +382,14 @@ static int check_package(const char *nvr, const char *arch, map_string_t *osinfo
"X-Package-NVR: %s\r\n"
"X-Package-Arch: %s\r\n"
"X-OS-Release: %s\r\n",
- cfg.url, nvr, arch, releaseid);
-
- if (lang.encoding)
- strbuf_append_strf(http_request,
- "Accept-Charset: %s\r\n",
- lang.encoding);
- if (lang.locale)
- {
- strbuf_append_strf(http_request,
- "Accept-Language: %s\r\n",
- lang.locale);
- free(lang.locale);
- }
+ "%s"
+ "%s"
+ "\r\n",
+ cfg.url, nvr, arch, releaseid,
+ lang.accept_charset,
+ lang.accept_language
+ );

- strbuf_append_str(http_request, "\r\n");
PRInt32 written = PR_Send(tcp_sock, http_request->buf, http_request->len,
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
if (written == -1)
@@ -446,9 +439,6 @@ static int create(bool delete_temp_archive,
char **task_id,
char **task_password)
{
- struct language lang;
- get_language(&lang);
-
if (delay)
{
puts(_("Querying server settings"));
@@ -659,22 +649,14 @@ static int create(bool delete_temp_archive,
"Content-Type: application/x-xz-compressed-tar\r\n"
"Content-Length: %lld\r\n"
"Connection: close\r\n"
- "X-Task-Type: %d\r\n",
- cfg.url, (long long)file_stat.st_size, task_type);
-
- if (lang.encoding)
- strbuf_append_strf(http_request,
- "Accept-Charset: %s\r\n",
- lang.encoding);
- if (lang.locale)
- {
- strbuf_append_strf(http_request,
- "Accept-Language: %s\r\n",
- lang.locale);
- free(lang.locale);
- }
-
- strbuf_append_str(http_request, "\r\n");
+ "X-Task-Type: %d\r\n"
+ "%s"
+ "%s"
+ "\r\n",
+ cfg.url, (long long)file_stat.st_size, task_type,
+ lang.accept_charset,
+ lang.accept_language
+ );

PRInt32 written = PR_Send(tcp_sock, http_request->buf, http_request->len,
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
@@ -825,9 +807,6 @@ static void status(const char *task_id,
char **task_status,
char **status_message)
{
- struct language lang;
- get_language(&lang);
-
PRFileDesc *tcp_sock, *ssl_sock;
ssl_connect(&cfg, &tcp_sock, &ssl_sock);
struct strbuf *http_request = strbuf_new();
@@ -836,22 +815,14 @@ static void status(const char *task_id,
"Host: %s\r\n"
"X-Task-Password: %s\r\n"
"Content-Length: 0\r\n"
- "Connection: close\r\n",
- task_id, cfg.url, task_password);
-
- if (lang.encoding)
- strbuf_append_strf(http_request,
- "Accept-Charset: %s\r\n",
- lang.encoding);
- if (lang.locale)
- {
- strbuf_append_strf(http_request,
- "Accept-Language: %s\r\n",
- lang.locale);
- free(lang.locale);
- }
-
- strbuf_append_str(http_request, "\r\n");
+ "Connection: close\r\n"
+ "%s"
+ "%s"
+ "\r\n",
+ task_id, cfg.url, task_password,
+ lang.accept_charset,
+ lang.accept_language
+ );

PRInt32 written = PR_Send(tcp_sock, http_request->buf, http_request->len,
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
@@ -903,9 +874,6 @@ static void run_status(const char *task_id, const char *task_password)
static void backtrace(const char *task_id, const char *task_password,
char **backtrace)
{
- struct language lang;
- get_language(&lang);
-
PRFileDesc *tcp_sock, *ssl_sock;
ssl_connect(&cfg, &tcp_sock, &ssl_sock);
struct strbuf *http_request = strbuf_new();
@@ -914,22 +882,14 @@ static void backtrace(const char *task_id, const char *task_password,
"Host: %s\r\n"
"X-Task-Password: %s\r\n"
"Content-Length: 0\r\n"
- "Connection: close\r\n",
- task_id, cfg.url, task_password);
-
- if (lang.encoding)
- strbuf_append_strf(http_request,
- "Accept-Charset: %s\r\n",
- lang.encoding);
- if (lang.locale)
- {
- strbuf_append_strf(http_request,
- "Accept-Language: %s\r\n",
- lang.locale);
- free(lang.locale);
- }
-
- strbuf_append_str(http_request, "\r\n");
+ "Connection: close\r\n"
+ "%s"
+ "%s"
+ "\r\n",
+ task_id, cfg.url, task_password,
+ lang.accept_charset,
+ lang.accept_language
+ );

PRInt32 written = PR_Send(tcp_sock, http_request->buf, http_request->len,
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
@@ -971,9 +931,6 @@ static void run_backtrace(const char *task_id, const char *task_password)

static void run_log(const char *task_id, const char *task_password)
{
- struct language lang;
- get_language(&lang);
-
PRFileDesc *tcp_sock, *ssl_sock;
ssl_connect(&cfg, &tcp_sock, &ssl_sock);
struct strbuf *http_request = strbuf_new();
@@ -982,22 +939,14 @@ static void run_log(const char *task_id, const char *task_password)
"Host: %s\r\n"
"X-Task-Password: %s\r\n"
"Content-Length: 0\r\n"
- "Connection: close\r\n",
- task_id, cfg.url, task_password);
-
- if (lang.encoding)
- strbuf_append_strf(http_request,
- "Accept-Charset: %s\r\n",
- lang.encoding);
- if (lang.locale)
- {
- strbuf_append_strf(http_request,
- "Accept-Language: %s\r\n",
- lang.locale);
- free(lang.locale);
- }
-
- strbuf_append_str(http_request, "\r\n");
+ "Connection: close\r\n"
+ "%s"
+ "%s"
+ "\r\n",
+ task_id, cfg.url, task_password,
+ lang.accept_charset,
+ lang.accept_language
+ );

PRInt32 written = PR_Send(tcp_sock, http_request->buf, http_request->len,
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
@@ -1114,6 +1063,7 @@ int main(int argc, char **argv)
#endif

abrt_init(argv);
+ get_language(&lang);

const char *task_id = NULL;
const char *task_password = NULL;
diff --git a/src/plugins/https-utils.c b/src/plugins/https-utils.c
index 85d3f80..cb3c606 100644
--- a/src/plugins/https-utils.c
+++ b/src/plugins/https-utils.c
@@ -26,19 +26,34 @@ void get_language(struct language *lang)
{
lang->locale = NULL;
lang->encoding = NULL;
+ /*
+ * Note: ->accept_language and ->accept_charset will always be non-NULL:
+ * if we don't know them, they'll be ""; otherwise,
+ * they will be fully formed HTTP headers, with \r\n at the end.
+ * IOW: they are formatted for adding them to HTTP headers as-is.
+ */

char *locale = setlocale(LC_ALL, NULL);
if (!locale)
+ {
+ lang->accept_language = xzalloc(1);
+ lang->accept_charset = xzalloc(1);
return;
+ }

lang->locale = xstrdup(locale);
- lang->encoding = strchr(lang->locale, '.');
+ lang->accept_language = xasprintf("Accept-Language: %s\r\n", locale);

+ lang->encoding = strchr(lang->locale, '.');
if (!lang->encoding)
+ {
+ lang->accept_charset = xzalloc(1);
return;
+ }

*lang->encoding = '\0';
++lang->encoding;
+ lang->accept_charset = xasprintf("Accept-Charset: %s\r\n", lang->encoding);
}

void alert_server_error(const char *peer_name)
diff --git a/src/plugins/https-utils.h b/src/plugins/https-utils.h
index 7f73ab9..8ff9aed 100644
--- a/src/plugins/https-utils.h
+++ b/src/plugins/https-utils.h
@@ -39,7 +39,10 @@ struct language
{
char *locale;
char *encoding;
+ char *accept_charset;
+ char *accept_language;
};
+void get_language(struct language *lang);

struct https_cfg
{
@@ -48,7 +51,6 @@ struct https_cfg
bool ssl_allow_insecure;
};

-void get_language(struct language *lang);
void alert_server_error(const char *peer_name);
void alert_connection_error(const char *peer_name);
void ssl_connect(struct https_cfg *cfg, PRFileDesc **tcp_sock, PRFileDesc **ssl_sock);

[Jakub Filak]

I'd like to release updated ABRT packages but I'm not sure if I should
wait for this patch or not because I'm not sure what is the purpose of
this patch? Is this patch related to any bugzilla bug or github issue?

Post by Denys Vlasenko
---
src/plugins/abrt-dedup-client.c | 24 ++++---
src/plugins/abrt-retrace-client.c | 134 ++++++++++++--------------------------
src/plugins/https-utils.c | 17 ++++-
src/plugins/https-utils.h | 4 +-
4 files changed, 72 insertions(+), 107 deletions(-)
diff --git a/src/plugins/abrt-dedup-client.c b/src/plugins/abrt-dedup-client.c
index 835d551..830efda 100644
--- a/src/plugins/abrt-dedup-client.c
+++ b/src/plugins/abrt-dedup-client.c
@@ -171,19 +171,17 @@ int main(int argc, char **argv)
"Host: %s\r\n"
"Content-Type: application/x-www-form-urlencoded\r\n"
"Content-Length: %d\r\n"
- "Connection: close\r\n",
- cfg.url, request_body->len);
-
- if (lang.encoding)
- strbuf_append_strf(request, "Accept-Charset: %s\r\n", lang.encoding);
-
- if (lang.locale)
- {
- strbuf_append_strf(request, "Accept-Language: %s\r\n", lang.locale);
- free(lang.locale);
- }
-
- strbuf_append_strf(request, "\r\n%s", request_body->buf);
+ "Connection: close\r\n"
+ "%s"
+ "%s"
+ "\r\n",
+ cfg.url,
+ request_body->len,
+ lang.accept_charset,
+ lang.accept_language
+ );
+
+ strbuf_append_str(request, request_body->buf);
strbuf_free(request_body);
/* Initialize NSS */
diff --git a/src/plugins/abrt-retrace-client.c b/src/plugins/abrt-retrace-client.c
index 940939b..cc0b802 100644
--- a/src/plugins/abrt-retrace-client.c
+++ b/src/plugins/abrt-retrace-client.c
@@ -29,6 +29,8 @@ enum
TASK_VMCORE,
};
+static struct language lang;
+
struct retrace_settings
{
int running_tasks;
@@ -369,8 +371,6 @@ static int check_package(const char *nvr, const char *arch, map_string_t *osinfo
{
char *releaseid = get_release_id(osinfo, arch);
- struct language lang;
- get_language(&lang);
PRFileDesc *tcp_sock, *ssl_sock;
ssl_connect(&cfg, &tcp_sock, &ssl_sock);
struct strbuf *http_request = strbuf_new();
@@ -382,21 +382,14 @@ static int check_package(const char *nvr, const char *arch, map_string_t *osinfo
"X-Package-NVR: %s\r\n"
"X-Package-Arch: %s\r\n"
"X-OS-Release: %s\r\n",
- cfg.url, nvr, arch, releaseid);
-
- if (lang.encoding)
- strbuf_append_strf(http_request,
- "Accept-Charset: %s\r\n",
- lang.encoding);
- if (lang.locale)
- {
- strbuf_append_strf(http_request,
- "Accept-Language: %s\r\n",
- lang.locale);
- free(lang.locale);
- }
+ "%s"
+ "%s"
+ "\r\n",
+ cfg.url, nvr, arch, releaseid,
+ lang.accept_charset,
+ lang.accept_language
+ );
- strbuf_append_str(http_request, "\r\n");
PRInt32 written = PR_Send(tcp_sock, http_request->buf, http_request->len,
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
if (written == -1)
@@ -446,9 +439,6 @@ static int create(bool delete_temp_archive,
char **task_id,
char **task_password)
{
- struct language lang;
- get_language(&lang);
-
if (delay)
{
puts(_("Querying server settings"));
@@ -659,22 +649,14 @@ static int create(bool delete_temp_archive,
"Content-Type: application/x-xz-compressed-tar\r\n"
"Content-Length: %lld\r\n"
"Connection: close\r\n"
- "X-Task-Type: %d\r\n",
- cfg.url, (long long)file_stat.st_size, task_type);
-
- if (lang.encoding)
- strbuf_append_strf(http_request,
- "Accept-Charset: %s\r\n",
- lang.encoding);
- if (lang.locale)
- {
- strbuf_append_strf(http_request,
- "Accept-Language: %s\r\n",
- lang.locale);
- free(lang.locale);
- }
-
- strbuf_append_str(http_request, "\r\n");
+ "X-Task-Type: %d\r\n"
+ "%s"
+ "%s"
+ "\r\n",
+ cfg.url, (long long)file_stat.st_size, task_type,
+ lang.accept_charset,
+ lang.accept_language
+ );
PRInt32 written = PR_Send(tcp_sock, http_request->buf, http_request->len,
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
@@ -825,9 +807,6 @@ static void status(const char *task_id,
char **task_status,
char **status_message)
{
- struct language lang;
- get_language(&lang);
-
PRFileDesc *tcp_sock, *ssl_sock;
ssl_connect(&cfg, &tcp_sock, &ssl_sock);
struct strbuf *http_request = strbuf_new();
@@ -836,22 +815,14 @@ static void status(const char *task_id,
"Host: %s\r\n"
"X-Task-Password: %s\r\n"
"Content-Length: 0\r\n"
- "Connection: close\r\n",
- task_id, cfg.url, task_password);
-
- if (lang.encoding)
- strbuf_append_strf(http_request,
- "Accept-Charset: %s\r\n",
- lang.encoding);
- if (lang.locale)
- {
- strbuf_append_strf(http_request,
- "Accept-Language: %s\r\n",
- lang.locale);
- free(lang.locale);
- }
-
- strbuf_append_str(http_request, "\r\n");
+ "Connection: close\r\n"
+ "%s"
+ "%s"
+ "\r\n",
+ task_id, cfg.url, task_password,
+ lang.accept_charset,
+ lang.accept_language
+ );
PRInt32 written = PR_Send(tcp_sock, http_request->buf, http_request->len,
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
@@ -903,9 +874,6 @@ static void run_status(const char *task_id, const char *task_password)
static void backtrace(const char *task_id, const char *task_password,
char **backtrace)
{
- struct language lang;
- get_language(&lang);
-
PRFileDesc *tcp_sock, *ssl_sock;
ssl_connect(&cfg, &tcp_sock, &ssl_sock);
struct strbuf *http_request = strbuf_new();
@@ -914,22 +882,14 @@ static void backtrace(const char *task_id, const char *task_password,
"Host: %s\r\n"
"X-Task-Password: %s\r\n"
"Content-Length: 0\r\n"
- "Connection: close\r\n",
- task_id, cfg.url, task_password);
-
- if (lang.encoding)
- strbuf_append_strf(http_request,
- "Accept-Charset: %s\r\n",
- lang.encoding);
- if (lang.locale)
- {
- strbuf_append_strf(http_request,
- "Accept-Language: %s\r\n",
- lang.locale);
- free(lang.locale);
- }
-
- strbuf_append_str(http_request, "\r\n");
+ "Connection: close\r\n"
+ "%s"
+ "%s"
+ "\r\n",
+ task_id, cfg.url, task_password,
+ lang.accept_charset,
+ lang.accept_language
+ );
PRInt32 written = PR_Send(tcp_sock, http_request->buf, http_request->len,
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
@@ -971,9 +931,6 @@ static void run_backtrace(const char *task_id, const char *task_password)
static void run_log(const char *task_id, const char *task_password)
{
- struct language lang;
- get_language(&lang);
-
PRFileDesc *tcp_sock, *ssl_sock;
ssl_connect(&cfg, &tcp_sock, &ssl_sock);
struct strbuf *http_request = strbuf_new();
@@ -982,22 +939,14 @@ static void run_log(const char *task_id, const char *task_password)
"Host: %s\r\n"
"X-Task-Password: %s\r\n"
"Content-Length: 0\r\n"
- "Connection: close\r\n",
- task_id, cfg.url, task_password);
-
- if (lang.encoding)
- strbuf_append_strf(http_request,
- "Accept-Charset: %s\r\n",
- lang.encoding);
- if (lang.locale)
- {
- strbuf_append_strf(http_request,
- "Accept-Language: %s\r\n",
- lang.locale);
- free(lang.locale);
- }
-
- strbuf_append_str(http_request, "\r\n");
+ "Connection: close\r\n"
+ "%s"
+ "%s"
+ "\r\n",
+ task_id, cfg.url, task_password,
+ lang.accept_charset,
+ lang.accept_language
+ );
PRInt32 written = PR_Send(tcp_sock, http_request->buf, http_request->len,
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
@@ -1114,6 +1063,7 @@ int main(int argc, char **argv)
#endif
abrt_init(argv);
+ get_language(&lang);
const char *task_id = NULL;
const char *task_password = NULL;
diff --git a/src/plugins/https-utils.c b/src/plugins/https-utils.c
index 85d3f80..cb3c606 100644
--- a/src/plugins/https-utils.c
+++ b/src/plugins/https-utils.c
@@ -26,19 +26,34 @@ void get_language(struct language *lang)
{
lang->locale = NULL;
lang->encoding = NULL;
+ /*
+ * if we don't know them, they'll be ""; otherwise,
+ * they will be fully formed HTTP headers, with \r\n at the end.
+ * IOW: they are formatted for adding them to HTTP headers as-is.
+ */
char *locale = setlocale(LC_ALL, NULL);
if (!locale)
+ {
+ lang->accept_language = xzalloc(1);
+ lang->accept_charset = xzalloc(1);
return;
+ }
lang->locale = xstrdup(locale);
- lang->encoding = strchr(lang->locale, '.');
+ lang->accept_language = xasprintf("Accept-Language: %s\r\n", locale);
+ lang->encoding = strchr(lang->locale, '.');
if (!lang->encoding)
+ {
+ lang->accept_charset = xzalloc(1);
return;
+ }
*lang->encoding = '\0';
++lang->encoding;
+ lang->accept_charset = xasprintf("Accept-Charset: %s\r\n", lang->encoding);
}
void alert_server_error(const char *peer_name)
diff --git a/src/plugins/https-utils.h b/src/plugins/https-utils.h
index 7f73ab9..8ff9aed 100644
--- a/src/plugins/https-utils.h
+++ b/src/plugins/https-utils.h
@@ -39,7 +39,10 @@ struct language
{
char *locale;
char *encoding;
+ char *accept_charset;
+ char *accept_language;
};
+void get_language(struct language *lang);
struct https_cfg
{
@@ -48,7 +51,6 @@ struct https_cfg
bool ssl_allow_insecure;
};
-void get_language(struct language *lang);
void alert_server_error(const char *peer_name);
void alert_connection_error(const char *peer_name);
void ssl_connect(struct https_cfg *cfg, PRFileDesc **tcp_sock, PRFileDesc **ssl_sock);

[Denys Vlasenko]

Post by Jakub Filak
I'd like to release updated ABRT packages but I'm not sure if I should
wait for this patch or not because I'm not sure what is the purpose of
this patch? Is this patch related to any bugzilla bug or github issue?

It is a simplification. It does not fix any bugs.

Since Michal is the principal author of this code,
I think it's his right to decide whether it looks ok there.

[Jiri Moskovcak]

Post by Denys Vlasenko

Post by Jakub Filak
I'd like to release updated ABRT packages but I'm not sure if I should
wait for this patch or not because I'm not sure what is the purpose of
this patch? Is this patch related to any bugzilla bug or github issue?

It is a simplification. It does not fix any bugs.
Since Michal is the principal author of this code,
I think it's his right to decide whether it looks ok there.

- at least reference the ticket which is this patch supposed to help with

--Jirka

[Michal Toman]

Pushed with one minor change

Post by Denys Vlasenko
@@ -382,21 +382,14 @@ static int check_package(const char *nvr, const char *arch, map_string_t *osinfo
"X-Package-NVR: %s\r\n"
"X-Package-Arch: %s\r\n"
"X-OS-Release: %s\r\n",

removed the excessive comma ^

Post by Denys Vlasenko
- cfg.url, nvr, arch, releaseid);
-
- if (lang.encoding)
- strbuf_append_strf(http_request,
- "Accept-Charset: %s\r\n",
- lang.encoding);
- if (lang.locale)
- {
- strbuf_append_strf(http_request,
- "Accept-Language: %s\r\n",
- lang.locale);
- free(lang.locale);
- }
+ "%s"
+ "%s"
+ "\r\n",
+ cfg.url, nvr, arch, releaseid,
+ lang.accept_charset,
+ lang.accept_language
+ );