Forcing additional headers with Location: redirect
I'm currently working on modifying a php based application into an apache
module.
The short of it is that I want to send a Location header with a 0 content
length message and headers for cache-control and pragma.
Setting the headers, and returning HTTP_MOVED_TEMPORARILY the additional
headers are stripped and the default message is sent.
Using ErrorDocument in my httpd.conf to use a 0 length file, the content
length is correctly set to 0, but even the Location header gets stripped.
I am pretty sure the cause of my issue is unrelated to the additional
headers, just needing me to send no content which is confusing the problem
handsets. How can I cause this behaviour?
For anyone interested, the background of the situation, some source after
the background:
In addition to logging an access to an image, this module will redirect the
request based on some parameters to a different server. The php version
would send Location, pragma: no-cache, cache-control: no-cache,
must-revalidate and 0 content length, which worked perfectly for all
clients.
Within the module I have set the location header as well as the pragma and
cache-control, however only the location will go through, and the default
302 message is sent. This is not generally a problem, except for with one of
the local mobile phone handset manufacturers which fails to redirect the
image correctly claiming a bad image.
Thus I am attempting to emulate the behaviour of the php, being unsure of
which part is causing the problem. Most likely it would be the content being
sent confusing the handset. Honestly I'd like to just ignore the particular
manufacturer, but this is not an option.
Some source(stripped down to the essentials... In the actual version there's
error handling and whatnot):
curl_easy_setopt(hCurl, CURLOPT_URL, adUrl); // url to get the redirect
location from
curl_easy_setopt(hCurl, CURLOPT_WRITEDATA, outUrl);
curl_easy_setopt(hCurl, CURLOPT_WRITEFUNCTION, outUrlWrite);
if(curl_easy_perform(hCurl) == 0)
{
apr_table_setn(r-headers_out, Location, outUrl);
}
r-content_type = CONTENT_TYPE_MOBILE;
apr_table_setn(r-headers_out, Cache-Control, no-cache,
must-revalidate);
apr_table_setn(r-headers_out, Pragma, no-cache);
curl_easy_cleanup(hCurl);
return HTTP_MOVED_TEMPORARILY;
Re: Forcing additional headers with Location: redirect
On Wed, Jan 7, 2009 at 07:14, Juhani Connolly juh...@ninja.co.jp wrote:
I'm currently working on modifying a php based application into an apache
module.
The short of it is that I want to send a Location header with a 0 content
length message and headers for cache-control and pragma.
Setting the headers, and returning HTTP_MOVED_TEMPORARILY the additional
headers are stripped and the default message is sent.
Using ErrorDocument in my httpd.conf to use a 0 length file, the content
length is correctly set to 0, but even the Location header gets stripped.
I am pretty sure the cause of my issue is unrelated to the additional
headers, just needing me to send no content which is confusing the problem
handsets. How can I cause this behaviour?
For anyone interested, the background of the situation, some source after
the background:
In addition to logging an access to an image, this module will redirect the
request based on some parameters to a different server. The php version
would send Location, pragma: no-cache, cache-control: no-cache,
must-revalidate and 0 content length, which worked perfectly for all
clients.
Within the module I have set the location header as well as the pragma and
cache-control, however only the location will go through, and the default
302 message is sent. This is not generally a problem, except for with one of
the local mobile phone handset manufacturers which fails to redirect the
image correctly claiming a bad image.
Thus I am attempting to emulate the behaviour of the php, being unsure of
which part is causing the problem. Most likely it would be the content being
sent confusing the handset. Honestly I'd like to just ignore the particular
manufacturer, but this is not an option.
Some source(stripped down to the essentials... In the actual version there's
error handling and whatnot):
curl_easy_setopt(hCurl, CURLOPT_URL, adUrl); // url to get the redirect
location from
curl_easy_setopt(hCurl, CURLOPT_WRITEDATA, outUrl);
curl_easy_setopt(hCurl, CURLOPT_WRITEFUNCTION, outUrlWrite);
if(curl_easy_perform(hCurl) == 0)
{
apr_table_setn(r-headers_out, Location, outUrl);
}
r-content_type = CONTENT_TYPE_MOBILE;
apr_table_setn(r-headers_out, Cache-Control, no-cache,
must-revalidate);
apr_table_setn(r-headers_out, Pragma, no-cache);
curl_easy_cleanup(hCurl);
return HTTP_MOVED_TEMPORARILY;
Use r-err_headers_out instead of r-headers_out. If it does not work,
there's a longer and more complicated solution based on
insert_error_filter. Come back for details if err_headers_out does not
solve the problem.
--
Sorin
--
A: Because it reverses the logical flow of conversation.
Q: Why is top-posting frowned upon?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
Re: svn commit: r729579 - in /httpd/httpd/trunk: CHANGES modules/generators/mod_cgid.c
On Sat, Dec 27, 2008 at 8:12 AM, Nick Kew n...@webthing.com wrote: On 27 Dec 2008, at 09:52, Ruediger Pluem wrote: On 12/27/2008 03:13 AM, n...@apache.org wrote: Author: niq Date: Fri Dec 26 18:13:47 2008 New Revision: 729579 URL: http://svn.apache.org/viewvc?rev=729579view=rev Log: PR#39332: fix for segfault problem with mod_cgid on Solaris Patch by Masaoki Kobayashi Modified: httpd/httpd/trunk/CHANGES httpd/httpd/trunk/modules/generators/mod_cgid.c Just for my understanding. This patch works around a bug in Solaris with write and UNIX domain sockets, right? Otherwise I wouldn't get why the new code using writev would be better thatn the old one using write. It's a bug with a number of reports, though I haven't actually been able to reproduce it myself on opensolaris. AFAIK it is resolved by 120664-01 or later (or x86-equiv patch), from several years ago...
Re: svn commit: r731965 - /httpd/httpd/trunk/modules/generators/mod_cgid.c
On Tue, Jan 6, 2009 at 10:05 AM, traw...@apache.org wrote:
Author: trawick
Date: Tue Jan 6 07:05:33 2009
New Revision: 731965
URL: http://svn.apache.org/viewvc?rev=731965view=rev
Log:
the length arguments are apr_size_t, not int
Modified:
httpd/httpd/trunk/modules/generators/mod_cgid.c
Modified: httpd/httpd/trunk/modules/generators/mod_cgid.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/generators/mod_cgid.c?rev=731965r1=731964r2=731965view=diff
==
--- httpd/httpd/trunk/modules/generators/mod_cgid.c (original)
+++ httpd/httpd/trunk/modules/generators/mod_cgid.c Tue Jan 6 07:05:33
2009
@@ -354,7 +354,7 @@
va_start(ap, count);
for (i = 0; i count; i++) {
vec[i].iov_base = va_arg(ap, caddr_t);
-vec[i].iov_len = va_arg(ap, int);
+vec[i].iov_len = va_arg(ap, apr_size_t);
IIRC, size_t is 64-bit with a 64-bit build on AIX; someone with access to
the right bits (and bored out of their skull) could look for some fun
behavior with the original code
Re: svn commit: r731965 - /httpd/httpd/trunk/modules/generators/mod_cgid.c
[crossposting because this httpd issue raises an APR issue]
Jeff Trawick wrote:
On Tue, Jan 6, 2009 at 10:05 AM, traw...@apache.org
mailto:traw...@apache.org wrote:
Author: trawick
Date: Tue Jan 6 07:05:33 2009
New Revision: 731965
URL: http://svn.apache.org/viewvc?rev=731965view=rev
http://svn.apache.org/viewvc?rev=731965view=rev
Log:
the length arguments are apr_size_t, not int
How is it apr_size_t?
I see the following in apr_want.h. Neither definition uses
apr_size_t. man iovec shows int on Solaris, size_t on
FreeBSD and Linux. Never AFAICS an apr_size_t.
apr_want.h contains the following, which appears to leave open
the possibility of not having it at all. Do we have a deeper bug
here that calls for an apr_iovec datatype matching the platform?
#if APR_HAVE_IOVEC
#if APR_HAVE_SYS_UIO_H
#include sys/uio.h
#endif
#else
struct iovec
{
char *iov_base;
size_t iov_len;
};
#endif
--
Nick Kew
Re: svn commit: r731965 - /httpd/httpd/trunk/modules/generators/mod_cgid.c
traw...@apache.org wrote:
Author: trawick
Date: Tue Jan 6 07:05:33 2009
New Revision: 731965
URL: http://svn.apache.org/viewvc?rev=731965view=rev
Log:
the length arguments are apr_size_t, not int
Modified:
httpd/httpd/trunk/modules/generators/mod_cgid.c
Modified: httpd/httpd/trunk/modules/generators/mod_cgid.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/generators/mod_cgid.c?rev=731965r1=731964r2=731965view=diff
==
--- httpd/httpd/trunk/modules/generators/mod_cgid.c (original)
+++ httpd/httpd/trunk/modules/generators/mod_cgid.c Tue Jan 6 07:05:33 2009
@@ -354,7 +354,7 @@
va_start(ap, count);
for (i = 0; i count; i++) {
vec[i].iov_base = va_arg(ap, caddr_t);
-vec[i].iov_len = va_arg(ap, int);
+vec[i].iov_len = va_arg(ap, apr_size_t);
More correctly, aren't these size_t native as opposed to apr_'s mapping?
This is a system structure, not an apr structure.
Re: svn commit: r731965 - /httpd/httpd/trunk/modules/generators/mod_cgid.c
-Ursprüngliche Nachricht- Von: Nick Kew [mailto:n...@webthing.com] Gesendet: Dienstag, 6. Januar 2009 16:48 An: dev@httpd.apache.org; d...@apr.apache.org Betreff: Re: svn commit: r731965 - /httpd/httpd/trunk/modules/generators/mod_cgid.c [crossposting because this httpd issue raises an APR issue] Jeff Trawick wrote: On Tue, Jan 6, 2009 at 10:05 AM, traw...@apache.org mailto:traw...@apache.org wrote: Author: trawick Date: Tue Jan 6 07:05:33 2009 New Revision: 731965 URL: http://svn.apache.org/viewvc?rev=731965view=rev http://svn.apache.org/viewvc?rev=731965view=rev Log: the length arguments are apr_size_t, not int How is it apr_size_t? I guess what is important here is that all length parameters passed to sock_writev as variable parameters are of size_t. So the second argument given to va_arg should reflect this. IMHO it doesn't matter what type we have in the iovec struct for this purpose. Regards Rüdiger
Re: svn commit: r731965 - /httpd/httpd/trunk/modules/generators/mod_cgid.c
Plüm, Rüdiger, VF-Group wrote: I guess what is important here is that all length parameters passed to sock_writev as variable parameters are of size_t. So the second argument given to va_arg should reflect this. IMHO it doesn't matter what type we have in the iovec struct for this purpose. Sorry, yes, Jeff was right. Looking at what gets passed to the vararg-consuming function, that's apr_size_t. Jeff, you have my +1 to add r731965 to my backport proposal in STATUS. -- Nick Kew
Re: svn commit: r729579 - in /httpd/httpd/trunk: CHANGES modules/generators/mod_cgid.c
On Tue, Jan 6, 2009 at 7:11 AM, Jeff Trawick traw...@gmail.com wrote: AFAIK it is resolved by 120664-01 or later (or x86-equiv patch), from several years ago... Yah, we already have configure checks for this. I don't like adding unnecessary complexity to work around an OS bug that we've detected and flagged for years. -- justin
Re: svn commit: r731965 - /httpd/httpd/trunk/modules/generators/mod_cgid.c
Nick Kew wrote: [crossposting because this httpd issue raises an APR issue] Jeff Trawick wrote: On Tue, Jan 6, 2009 at 10:05 AM, traw...@apache.org mailto:traw...@apache.org wrote: Author: trawick Date: Tue Jan 6 07:05:33 2009 New Revision: 731965 URL: http://svn.apache.org/viewvc?rev=731965view=rev http://svn.apache.org/viewvc?rev=731965view=rev Log: the length arguments are apr_size_t, not int How is it apr_size_t? I see the following in apr_want.h. Neither definition uses apr_size_t. man iovec shows int on Solaris, size_t on FreeBSD and Linux. Never AFAICS an apr_size_t. apr_want.h contains the following, which appears to leave open the possibility of not having it at all. Do we have a deeper bug here that calls for an apr_iovec datatype matching the platform? Yes, I think we need an apr_iov_len_t type. Bill
Graceful restart not so graceful?
Would folks comment on Nathan's, Joe's and Stefan's work on https://issues.apache.org/bugzilla/show_bug.cgi?id=42829 and offer any comments on why this patch; https://issues.apache.org/bugzilla/attachment.cgi?id=22822 should not be applied to trunk/ and branches/2.2.x/ in time for the next releases? If I hear no objections by noon Jan 8 I'll apply. According to Stefan this current patch is improving the situation. Bill
Re: svn commit: r729579 - in /httpd/httpd/trunk: CHANGES modules/generators/mod_cgid.c
On Tue, Jan 6, 2009 at 11:28 AM, Justin Erenkrantz jus...@erenkrantz.comwrote: On Tue, Jan 6, 2009 at 7:11 AM, Jeff Trawick traw...@gmail.com wrote: AFAIK it is resolved by 120664-01 or later (or x86-equiv patch), from several years ago... Yah, we already have configure checks for this. I don't like adding unnecessary complexity to work around an OS bug that we've detected and flagged for years. -- justin I'm happy to see some syscalls disappear, but this patch only goes so far. -- Born in Roswell... married an alien...
Re: PR42829: graceful restart with multiple listeners using prefork MPM can result in hung processes
On Tue, Feb 5, 2008 at 7:53 AM, Joe Orton jor...@redhat.com wrote:
On Fri, Feb 01, 2008 at 10:41:39AM +0100, Stefan Fritsch wrote:
Joe Orton wrote:
I mentioned in the bug that the signal handler could cause undefined
behaviour, but I'm not sure now whether that is true. On Linux I can
reproduce some cases where this will happen, which are all due to
well-defined behaviour:
1) with some (default on Linux) accept mutex types,
apr_proc_mutex_lock() will loop on EINTR. Hence, children blocked
waiting for the mutex do hang until the mutex is released. Fixing
this would need some APR work, new interfaces, blah
This is not a problem. On graceful-stop or reload the processes will get
the lock one by one and die (or hang somewhere else). I have never seen a
left over process hanging in this function.
Well, normally all children will be woken up and take the accept mutex
because of the dummy connections. But if you have one child blocked
because of issue (3) - whilst holding the accept mutex - all the other
children will also be blocked. If the EINTR could be processed at MPM
level, this wouldn't happen. So I think it is a problem, though you
could argue that solving (3) also sort of solves (1).
I can also reproduce a third case, but I'm not sure about the cause:
3) apr_pollset_poll() is blocking despite the fact that the listening
fds are supposedly already closed before entering the syscall.
This is the main problem in my experience.
...
On Linux with epoll, the hanging processes just blocks in
apr_pollset_poll(), so checking the return value won't do any good.
Maybe the problem is that (AIUI) poll() returns POLLNVAL if a fd is not
open, while epoll() does not have something similar. In epoll.c, a
comment
says APR_POLLNVAL is not handled by epoll. Or should epoll return
EPOLLHUP in this case?
I did some more research on this: the case is covered in the epoll(7)
man page - fds are removed from any containing epoll sets on closure.
So it is well-defined behaviour, and the hang is expected; when all
the listeners are closed, the poll set becomes empty, so the
apr_pollset_poll() call will sleep forever, or until interrupted by
signal!
select() and poll() will indeed return POLLNVAL for the closed-fds case,
and prefork needs to check for that.
From some brief googling, FreeBSD kqueue appears to have the same
guarantee. This PR has some investigation of what happens with Solaris
ports: http://issues.apache.org/bugzilla/show_bug.cgi?id=42580
For the graceful-stop case, it would be simple enough to just signal any
dozy children again to wake them up in the wait-for-exit loop, but
graceful-restart doesn't have that opportunity, so I'm not sure about a
general solution. Reducing the poll timeout to some non-infinite time
would work.
This holds up to some very light graceful-restart testing on OpenSolaris
(the same light testing that triggered a hang):
Index: server/mpm/prefork/prefork.c
===
--- server/mpm/prefork/prefork.c(revision 731724)
+++ server/mpm/prefork/prefork.c(working copy)
@@ -540,10 +540,12 @@
apr_int32_t numdesc;
const apr_pollfd_t *pdesc;
-/* timeout == -1 == wait forever */
-status = apr_pollset_poll(pollset, -1, numdesc, pdesc);
+/* timeout == 10 seconds to avoid a hang at graceful
restart/stop
+ * caused by the closing of sockets by the signal handler
+ */
+status = apr_pollset_poll(pollset, apr_time_from_sec(10),
numdesc, pdesc);
if (status != APR_SUCCESS) {
-if (APR_STATUS_IS_EINTR(status)) {
+if (APR_STATUS_IS_TIMEUP(status) ||
APR_STATUS_IS_EINTR(status)) {
if (one_process shutdown_pending) {
return;
}
Re: Graceful restart not so graceful?
On Tue, Jan 6, 2009 at 1:10 PM, William A. Rowe, Jr. wr...@rowe-clan.netwrote: Would folks comment on Nathan's, Joe's and Stefan's work on https://issues.apache.org/bugzilla/show_bug.cgi?id=42829 and offer any comments on why this patch; https://issues.apache.org/bugzilla/attachment.cgi?id=22822 should not be applied to trunk/ and branches/2.2.x/ in time for the next releases? If I hear no objections by noon Jan 8 I'll apply. According to Stefan this current patch is improving the situation. Bill See also http://marc.info/?l=apache-httpd-devm=119945416529706w=2
Re: Graceful restart not so graceful?
On 01/06/2009 07:10 PM, William A. Rowe, Jr. wrote:
Would folks comment on Nathan's, Joe's and Stefan's work on
https://issues.apache.org/bugzilla/show_bug.cgi?id=42829
and offer any comments on why this patch;
https://issues.apache.org/bugzilla/attachment.cgi?id=22822
should not be applied to trunk/ and branches/2.2.x/ in time for
the next releases?
@@ -573,6 +588,11 @@ static void child_main(int child_num_arg
apr_int32_t numdesc;
const apr_pollfd_t *pdesc;
+if (die_now) {
+status = !APR_SUCCESS;
+goto unlock;
+}
+
Hm, what happens if we get the signal exactly here?
Then stop_listening only set die_now to 1. So we reenter the poll and stay in
until we get a new request to OUR process on one of the listeners.
Is this the desired behaviour?
/* timeout == -1 == wait forever */
status = apr_pollset_poll(pollset, -1, numdesc, pdesc);
if (status != APR_SUCCESS) {
How about this one instead:
Index: server/mpm/prefork/prefork.c
===
--- server/mpm/prefork/prefork.c(Revision 731601)
+++ server/mpm/prefork/prefork.c(Arbeitskopie)
@@ -99,6 +99,8 @@
static int mpm_state = AP_MPMQ_STARTING;
static ap_pod_t *pod;
+static int dummy_listener;
+
/*
* The max child slot ever assigned, preserved across restarts. Necessary
* to deal with MaxClients changes across AP_SIG_GRACEFUL restarts. We
@@ -136,6 +138,8 @@
#endif /* TPF */
static volatile int die_now = 0;
+static volatile int listeners_closed = 0;
+static int close_connection = 1;
#ifdef GPROF
/*
@@ -304,8 +308,19 @@
static void stop_listening(int sig)
{
-ap_close_listeners();
+if (close_connection !listeners_closed) {
+ap_listen_rec *lr;
+for (lr = ap_listeners; lr; lr = lr-next) {
+apr_os_sock_t fd;
+
+apr_os_sock_get(fd, lr-sd);
+
+dup2(dummy_listener, fd);
+}
+listeners_closed = 1;
+}
+
/* For a graceful stop, we want the child to exit when done */
die_now = 1;
}
@@ -533,6 +548,7 @@
if (num_listensocks == 1) {
/* There is only one listener record, so refer to that one. */
lr = ap_listeners;
+close_connection = 0;
}
else {
/* multiple listening sockets - need to poll */
@@ -540,8 +556,14 @@
apr_int32_t numdesc;
const apr_pollfd_t *pdesc;
+if (die_now) {
+status = !APR_SUCCESS;
+goto unlock;
+}
+
/* timeout == -1 == wait forever */
status = apr_pollset_poll(pollset, -1, numdesc, pdesc);
+close_connection = 0;
if (status != APR_SUCCESS) {
if (APR_STATUS_IS_EINTR(status)) {
if (one_process shutdown_pending) {
@@ -595,8 +617,15 @@
/* if we accept() something we don't want to die, so we have to
* defer the exit
*/
-status = lr-accept_func(csd, lr, ptrans);
+if (!die_now) {
+status = lr-accept_func(csd, lr, ptrans);
+}
+else {
+status = !APR_SUCCESS;
+}
+unlock:
+close_connection = 1;
SAFE_ACCEPT(accept_mutex_off()); /* unlock after accept */
if (status == APR_EGENERAL) {
@@ -612,6 +641,11 @@
* socket options, file descriptors, and read/write buffers.
*/
+if (die_now !listeners_closed) {
+ap_close_listeners();
+listeners_closed = 1;
+}
+
current_conn = ap_run_create_connection(ptrans, ap_server_conf, csd,
my_child_num, sbh, bucket_alloc);
if (current_conn) {
ap_process_connection(current_conn, csd);
@@ -890,6 +924,17 @@
mpm_state = AP_MPMQ_STOPPING;
return 1;
}
+
+if (dummy_listener == 0) {
+dummy_listener = socket(AF_INET, SOCK_STREAM, 0);
+if (dummy_listener 0) {
+rv = errno;
+ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s,
+ Couldn't create dummy listener socket);
+mpm_state = AP_MPMQ_STOPPING;
+return 1;
+}
+}
#if APR_USE_SYSVSEM_SERIALIZE
if (ap_accept_lock_mech == APR_LOCK_DEFAULT ||
Regards
Rüdiger
Index: server/mpm/prefork/prefork.c
===
--- server/mpm/prefork/prefork.c (Revision 731601)
+++ server/mpm/prefork/prefork.c (Arbeitskopie)
@@ -99,6 +99,8 @@
static int mpm_state = AP_MPMQ_STARTING;
static ap_pod_t *pod;
+static int dummy_listener;
+
/*
* The max child slot ever assigned, preserved across restarts. Necessary
* to deal with MaxClients changes across AP_SIG_GRACEFUL
