Re: [PATCH 1/1] http: don't send C or POSIX in Accept-Language

Justin Tobler [off-list ref] writes:

Looking at `get_preferred_languages()` when NO_GETTEXT is defined, we
already filter out "C" and "POSIX". So doing this for the LANGUAGE
environment variable when writing the header also makes sense.

True.  I wonder if it makes sense to do the check in that helper
function, though.  I.e. something like

diff --git c/gettext.c w/gettext.c
index 8d08a61f84..e2e0fe339d 100644
--- c/gettext.c
+++ w/gettext.c

@@ -41,6 +41,16 @@ static const char *locale_charset(void)
 
 static const char *charset;
 
+static const char *filter_out_non_languages(const char *candidate)
+{
+	if (candidate && *candidate &&
+	    strcmp(candidate, "C") &&
+	    strcmp(candidate, "POSIX"))
+		return candidate;
+	else
+		return NULL;
+}
+
 /*
  * Guess the user's preferred languages from the value in LANGUAGE environment
  * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.

@@ -51,15 +61,13 @@ const char *get_preferred_languages(void)
 {
 	const char *retval;
 
-	retval = getenv("LANGUAGE");
-	if (retval && *retval)
+	retval = filter_out_non_languages(getenv("LANGUAGE"));
+	if (retval)
 		return retval;
 
 #ifndef NO_GETTEXT
-	retval = setlocale(LC_MESSAGES, NULL);
-	if (retval && *retval &&
-		strcmp(retval, "C") &&
-		strcmp(retval, "POSIX"))
+	retval = filter_out_non_languages(setlocale(LC_MESSAGES, NULL));
+	if (retval)
 		return retval;
 #endif
 

In the production code, we should have a comment before that new
helper function that explains why we exclude C and POSIX, if we were
to go that route.

Not sure if being more strict adds much more value here in practice
though. So it may be fine to keep it as-is. :)

Yup.  I care more about having a single place that checks using the
same logic, than what that logic exactly is ;-).

Thanks.

On Fri, Jul 11, 2025 at 11:32:19AM -0800, Junio C Hamano wrote:

Justin Tobler [off-list ref] writes:

quoted

Looking at `get_preferred_languages()` when NO_GETTEXT is defined, we
already filter out "C" and "POSIX". So doing this for the LANGUAGE
environment variable when writing the header also makes sense.

True.  I wonder if it makes sense to do the check in that helper
function, though.  I.e. something like

Definitely, and might also fix another bug, as IMHO the current logic have
a couple of issues:

* LANGUAGE is not meant to be relevant unless LANG is set to a valid locale
  as per the SPEC[1], allthough for our use case it might be better to still
  do, specially if there are users in the wild setting C and POSIX there.
* it might make more sense to use the union of LANGUAGE and LC_MESSAGES
  instead.

Carlo