Re: [1.3 PATCH] rework suppress-error-charset feature
On 5/2/05, Jeff Trawick [EMAIL PROTECTED] wrote: On 5/2/05, Rici Lake [EMAIL PROTECTED] wrote: If the action is required to compensate for a browser bug, wouldn't it be better to leave it as an environment variable and set it with BrowserMatch? You're absolutely right. I put the blinders on to environment variables when I saw that mod_env didn't do anything until the fixup hook, which is too late for processing of Redirect directives. But mod_setenvif does its work sooner and is the proper solution. Funny. I just heard from a Japanese user with an issue with this path too, but this time with 2.0. A third-party module is generating a custom error response and specifies a Japanese codepage in the generated HTML. But it goes through this error response code and picks up the charset=iso-8859-1 attribute. This particular situation isn't tied to a particular browser, but I'll still use the suppress-error-charset solution so that it matches 1.3. The user can use SetEnvIf with Request_URI to match all requests.
Re: [1.3 PATCH] rework suppress-error-charset feature
On 5/11/05, Jeff Trawick [EMAIL PROTECTED] wrote: Funny. I just heard from a Japanese user with an issue with this path too, but this time with 2.0. A third-party module is generating a custom error response and specifies a Japanese codepage in the generated HTML. But it goes through this error response code and picks up the charset=iso-8859-1 attribute. This particular situation isn't tied to a particular browser, but I'll still use the suppress-error-charset solution so that it matches 1.3. The user can use SetEnvIf with Request_URI to match all requests. Not funny: 1.3.x and 2.x docs say that Apache = 2.0.40 already has this feature. I can't find that the trivial code was ever committed though. Related PR: 31274.
Re: [1.3 PATCH] rework suppress-error-charset feature
On 1-May-05, at 7:04 PM, Jeff Trawick wrote: I found a description of the problem: When Apache issues a redirect in response to a client request, the response includes some actual text to be displayed in case the client can't (or doesn't) automatically follow the redirection. Apache ordinarily labels this text according to the character set which it uses, which is ISO-8859-1. However, if the redirection is to a page that uses a different character set, some broken browser versions will try to use the character set from the redirection text rather than the actual page. This can result in Greek, for instance, being incorrectly rendered. If the action is required to compensate for a browser bug, wouldn't it be better to leave it as an environment variable and set it with BrowserMatch?
Re: [1.3 PATCH] rework suppress-error-charset feature
On 4/29/05, Jeff Trawick [EMAIL PROTECTED] wrote: On 4/29/05, Jim Jagielski [EMAIL PROTECTED] wrote: +1 from me :) How appropriate would a 2.0/2.1 patch be as well? You tell me. I'm certainly willing to work up a 2.1 patch. I have to confess to ignorance about the scope of the problem. I found a description of the problem: When Apache issues a redirect in response to a client request, the response includes some actual text to be displayed in case the client can't (or doesn't) automatically follow the redirection. Apache ordinarily labels this text according to the character set which it uses, which is ISO-8859-1. However, if the redirection is to a page that uses a different character set, some broken browser versions will try to use the character set from the redirection text rather than the actual page. This can result in Greek, for instance, being incorrectly rendered.
Re: [1.3 PATCH] rework suppress-error-charset feature
On 4/29/05, Jim Jagielski [EMAIL PROTECTED] wrote:
+1 from me :)
How appropriate would a 2.0/2.1 patch be as well?
Attached is an updated 1.3 patch which avoids specifying the directive
within limit. Also attached is a 2.1 patch.
The coding styles of handling the directive differ quite a bit. Each
seems to match the style of other similar directives in the respective
Apache releases.
Index: src/include/http_core.h
===
--- src/include/http_core.h (revision 122969)
+++ src/include/http_core.h (working copy)
@@ -318,6 +318,8 @@
/* Digest auth. */
char *ap_auth_nonce;
+/* if set, don't include charset= on error responses */
+ap_flag_e suppress_error_charset;
} core_dir_config;
/* Per-server core configuration */
Index: src/main/http_protocol.c
===
--- src/main/http_protocol.c(revision 122969)
+++ src/main/http_protocol.c(working copy)
@@ -2751,6 +2751,7 @@
int idx = ap_index_of_response(status);
char *custom_response;
const char *location = ap_table_get(r-headers_out, Location);
+core_dir_config *conf;
#ifdef CHARSET_EBCDIC
/* Error Responses (builtin / string literal / redirection) are TEXT! */
ap_bsetflag(r-connection-client, B_EBCDIC2ASCII, r-ebcdic.conv_out = 1);
@@ -2828,7 +2829,10 @@
r-content_languages = NULL;
r-content_encoding = NULL;
r-clength = 0;
-if (ap_table_get(r-subprocess_env,
+conf = (core_dir_config *)ap_get_module_config(r-per_dir_config,
+ core_module);
+if (conf-suppress_error_charset == AP_FLAG_ON ||
+ap_table_get(r-subprocess_env,
suppress-error-charset) != NULL) {
r-content_type = text/html;
}
Index: src/main/http_core.c
===
--- src/main/http_core.c(revision 122969)
+++ src/main/http_core.c(working copy)
@@ -143,6 +143,8 @@
conf-etag_add = ETAG_UNSET;
conf-etag_remove = ETAG_UNSET;
+conf-suppress_error_charset = AP_FLAG_UNSET;
+
return (void *)conf;
}
@@ -319,6 +321,10 @@
conf-cgi_command_args = new-cgi_command_args;
}
+if (new-suppress_error_charset != AP_FLAG_UNSET) {
+conf-suppress_error_charset = new-suppress_error_charset;
+}
+
return (void*)conf;
}
@@ -1214,6 +1220,16 @@
}
return NULL;
}
+static const char *set_suppress_error_charset(cmd_parms *cmd,
+ core_dir_config *d, int arg)
+{
+const char *err = ap_check_cmd_context(cmd, NOT_IN_LIMIT);
+if (err != NULL) {
+return err;
+}
+d-suppress_error_charset = arg != 0 ? AP_FLAG_ON : AP_FLAG_OFF;
+return NULL;
+}
static const char *set_accept_mutex(cmd_parms *cmd, void *dummy, char *arg)
{
return ap_init_mutex_method(arg);
@@ -3508,7 +3524,8 @@
#endif
{ AddDefaultCharset, set_add_default_charset, NULL, OR_FILEINFO,
TAKE1, The name of the default charset to add to any Content-Type without
one or 'Off' to disable },
-
+{ SuppressErrorCharset, set_suppress_error_charset, NULL, OR_FILEINFO,
+ FLAG, Whether or not to suppress addition of charset= on error responses },
/* Old resource config file commands */
{ AccessFileName, set_access_name, NULL, RSRC_CONF, RAW_ARGS,
Index: server/core.c
===
--- server/core.c (revision 165404)
+++ server/core.c (working copy)
@@ -159,6 +159,7 @@
conf-enable_mmap = ENABLE_MMAP_UNSET;
conf-enable_sendfile = ENABLE_SENDFILE_UNSET;
conf-allow_encoded_slashes = 0;
+conf-suppress_error_charset = SUPPRESS_ERROR_CHARSET_UNSET;
return (void *)conf;
}
@@ -436,7 +437,11 @@
}
conf-allow_encoded_slashes = new-allow_encoded_slashes;
-
+
+if (new-suppress_error_charset != SUPPRESS_ERROR_CHARSET_UNSET) {
+conf-suppress_error_charset = new-suppress_error_charset;
+}
+
return (void*)conf;
}
@@ -1607,6 +1612,29 @@
return NULL;
}
+static const char *set_suppress_error_charset(cmd_parms *cmd, void *d_,
+ const char *arg)
+{
+core_dir_config *d = d_;
+const char *err = ap_check_cmd_context(cmd, NOT_IN_LIMIT);
+
+if (err != NULL) {
+return err;
+}
+
+if (strcasecmp(arg, on) == 0) {
+d-suppress_error_charset = SUPPRESS_ERROR_CHARSET_ON;
+}
+else if (strcasecmp(arg, off) == 0) {
+d-suppress_error_charset = SUPPRESS_ERROR_CHARSET_OFF;
+}
+else {
+return parameter must be 'on' or 'off';
+}
+
+return NULL;
+}
+
static const char *satisfy(cmd_parms *cmd, void *c_, const char *arg)
{
core_dir_config *c = c_;
@@ -3117,6 +3145,8 @@
Controls
Re: [1.3 PATCH] rework suppress-error-charset feature
Jeff Trawick wrote: On 4/29/05, Jim Jagielski [EMAIL PROTECTED] wrote: +1 from me :) How appropriate would a 2.0/2.1 patch be as well? Attached is an updated 1.3 patch which avoids specifying the directive within limit. Also attached is a 2.1 patch. +1 Bill
[1.3 PATCH] rework suppress-error-charset feature
That feature avoids the addition of charset=iso-8859-1 on certain
error responses. But its dependence on SetEnv means it only works if
the request reached fixup hook before failing. That means it can't
be used like this:
Redirect /Contacts http://www.example.com/contacts
Location /Contacts
SetEnv suppress-error-charset 1
/Location
This patch turns it into a directive:
Location /Contacts
SuppressErrorCharset On
/Location
Does anybody even care?
Index: src/include/http_core.h
===
--- src/include/http_core.h (revision 165318)
+++ src/include/http_core.h (working copy)
@@ -318,6 +318,8 @@
/* Digest auth. */
char *ap_auth_nonce;
+/* if set, don't include charset= on error responses */
+ap_flag_e suppress_error_charset;
} core_dir_config;
/* Per-server core configuration */
Index: src/main/http_protocol.c
===
--- src/main/http_protocol.c(revision 165318)
+++ src/main/http_protocol.c(working copy)
@@ -2751,6 +2751,7 @@
int idx = ap_index_of_response(status);
char *custom_response;
const char *location = ap_table_get(r-headers_out, Location);
+core_dir_config *conf;
#ifdef CHARSET_EBCDIC
/* Error Responses (builtin / string literal / redirection) are TEXT! */
ap_bsetflag(r-connection-client, B_EBCDIC2ASCII, r-ebcdic.conv_out = 1);
@@ -2828,7 +2829,10 @@
r-content_languages = NULL;
r-content_encoding = NULL;
r-clength = 0;
-if (ap_table_get(r-subprocess_env,
+conf = (core_dir_config *)ap_get_module_config(r-per_dir_config,
+ core_module);
+if (conf-suppress_error_charset == AP_FLAG_ON ||
+ap_table_get(r-subprocess_env,
suppress-error-charset) != NULL) {
r-content_type = text/html;
}
Index: src/main/http_core.c
===
--- src/main/http_core.c(revision 165318)
+++ src/main/http_core.c(working copy)
@@ -143,6 +143,8 @@
conf-etag_add = ETAG_UNSET;
conf-etag_remove = ETAG_UNSET;
+conf-suppress_error_charset = AP_FLAG_UNSET;
+
return (void *)conf;
}
@@ -319,6 +321,10 @@
conf-cgi_command_args = new-cgi_command_args;
}
+if (new-suppress_error_charset != AP_FLAG_UNSET) {
+conf-suppress_error_charset = new-suppress_error_charset;
+}
+
return (void*)conf;
}
@@ -1214,6 +1220,12 @@
}
return NULL;
}
+static const char *set_suppress_error_charset(cmd_parms *cmd,
+ core_dir_config *d, int arg)
+{
+d-suppress_error_charset = arg != 0 ? AP_FLAG_ON : AP_FLAG_OFF;
+return NULL;
+}
static const char *set_accept_mutex(cmd_parms *cmd, void *dummy, char *arg)
{
return ap_init_mutex_method(arg);
@@ -3508,7 +3520,8 @@
#endif
{ AddDefaultCharset, set_add_default_charset, NULL, OR_FILEINFO,
TAKE1, The name of the default charset to add to any Content-Type without
one or 'Off' to disable },
-
+{ SuppressErrorCharset, set_suppress_error_charset, NULL, OR_FILEINFO,
+ FLAG, Whether or not to suppress addition of charset= on error responses },
/* Old resource config file commands */
{ AccessFileName, set_access_name, NULL, RSRC_CONF, RAW_ARGS,
Re: [1.3 PATCH] rework suppress-error-charset feature
+1 from me :) How appropriate would a 2.0/2.1 patch be as well? On Apr 29, 2005, at 1:19 PM, Jeff Trawick wrote: That feature avoids the addition of charset=iso-8859-1 on certain error responses. But its dependence on SetEnv means it only works if the request reached fixup hook before failing. That means it can't be used like this: Redirect /Contacts http://www.example.com/contacts Location /Contacts SetEnv suppress-error-charset 1 /Location This patch turns it into a directive: Location /Contacts SuppressErrorCharset On /Location Does anybody even care? patch.txt -- === Jim Jagielski [|] [EMAIL PROTECTED] [|] http://www.jaguNET.com/ There 10 types of people: those who read binary and everyone else.
Re: [1.3 PATCH] rework suppress-error-charset feature
On 4/29/05, Jim Jagielski [EMAIL PROTECTED] wrote: +1 from me :) How appropriate would a 2.0/2.1 patch be as well? You tell me. I'm certainly willing to work up a 2.1 patch. I have to confess to ignorance about the scope of the problem. Somebody tells me this suppress-error-charset feature doesn't work [you idiot], and [unspecified] ancient browsers then misbehave on a redirect, and I trust them (at least partially) and go find why it doesn't work, but I don't know how ancient the browser has to be or exactly how the browser misbehavior manifests itself.
