Re: RFC: replace r-subprocess_env was Re: Patch for implementing ap_document_root as a hook

[Jakob Goldbach]

Thoughts?


I like it.

I prefer this generel env. API rather than making
ap_add_{common,cgi}_vars hook'able.

/Jakob

Patch for implementing ap_document_root as a hook

[Jakob Goldbach]

Hi,

Attached is a patch which implements ap_document_root(r) as a hook.
This way modules can set document_root on the fly. (think vhost_alias)
AND get the right DOCUMENT_ROOT env. variable (as set by
ap_add_common_vars(r)).

The patch also changes ap_core_translate to use ap_document_root(r)
instead of conf-ap_document_root.  This way, modules that just need
to point to a different docroot won't have to implement a translate
hook by appending r-uri til r-filename, but just rely on
ap_document_root.

Comments ?

/Jakob
Index: server/core.c
===
--- server/core.c	(revision 530676)
+++ server/core.c	(working copy)
@@ -69,12 +69,16 @@
 
 APR_HOOK_STRUCT(
 APR_HOOK_LINK(get_mgmt_items)
+APR_HOOK_LINK(document_root)
 )
 
 AP_IMPLEMENT_HOOK_RUN_ALL(int, get_mgmt_items,
   (apr_pool_t *p, const char *val, apr_hash_t *ht),
   (p, val, ht), OK, DECLINED)
 
+AP_IMPLEMENT_HOOK_RUN_FIRST(const char *,document_root,
+			(request_rec *r), (r), NULL)
+
 /* Server core module... This module provides support for really basic
  * server operations, including options and commands which control the
  * operation of other modules.  Consider this the bureaucracy module.
@@ -682,7 +686,7 @@
: DEFAULT_CONTENT_TYPE;
 }
 
-AP_DECLARE(const char *) ap_document_root(request_rec *r) /* Don't use this! */
+AP_DECLARE(const char *) ap_core_document_root(request_rec *r)
 {
 core_server_config *conf;
 
@@ -3390,7 +3394,7 @@
 while (*path == '/') {
 ++path;
 }
-if ((rv = apr_filepath_merge(r-filename, conf-ap_document_root, path,
+if ((rv = apr_filepath_merge(r-filename, ap_document_root(r), path,
  APR_FILEPATH_TRUENAME
| APR_FILEPATH_SECUREROOT, r-pool))
 != APR_SUCCESS) {
@@ -3413,7 +3417,7 @@
 while (*path == '/') {
 ++path;
 }
-if ((rv = apr_filepath_merge(r-filename, conf-ap_document_root, path,
+if ((rv = apr_filepath_merge(r-filename, ap_document_root(r), path,
  APR_FILEPATH_TRUENAME
| APR_FILEPATH_SECUREROOT, r-pool))
 != APR_SUCCESS) {
@@ -3863,6 +3867,7 @@
 APR_OPTIONAL_HOOK(proxy, create_req, core_create_proxy_req, NULL, NULL,
   APR_HOOK_MIDDLE);
 ap_hook_pre_mpm(ap_create_scoreboard, NULL, NULL, APR_HOOK_MIDDLE);
+ap_hook_document_root(ap_core_document_root, NULL, NULL, APR_HOOK_REALLY_LAST);
 
 /* register the core's insert_filter hook and register core-provided
  * filters
Index: include/http_core.h
===
--- include/http_core.h	(revision 530676)
+++ include/http_core.h	(working copy)
@@ -148,11 +148,10 @@
 /**
  * Retrieve the document root for this server
  * @param r The current request
- * @warning Don't use this!  If your request went through a Userdir, or 
- * something like that, it'll screw you.  But it's back-compatible...
  * @return The document root
  */
-AP_DECLARE(const char *) ap_document_root(request_rec *r);
+AP_DECLARE(const char *) ap_core_document_root(request_rec *r);
+#define ap_document_root(r) ap_run_document_root(r)
 
 /**
  * Lookup the remote client's DNS name or IP address
@@ -629,6 +628,14 @@
 AP_DECLARE_HOOK(int, get_mgmt_items,
 (apr_pool_t *p, const char * val, apr_hash_t *ht))
 
+/**
+ * This hook allows modules to return customized documentroot
+ * @param r the current request
+ * @ingroup hooks
+ */
+AP_DECLARE_HOOK(const char*,document_root,(request_rec *r))
+
+
 /* -- */
 
 /* --

Re: Patch for implementing ap_document_root as a hook

[Jakob Goldbach]

It is now in bugzilla as  #42192

/Jakob

Re: Patch for implementing ap_document_root as a hook

[Jakob Goldbach]

-1 on the face of things.  The map_to_storage hook was added to accomplish
what you desire.


I thought map_to_storage was made to do per-dir configuration. Not
path-translation.

The problem is not really doing the translation. I can always provide
my own translate handler in my module.

But in the current API I cannot to set my env. variables at will. They
will be overwritten by ap_add_common_vars which returns
carved-in-stone docroot from ap_document_root.

My only other option is to patch every single module which calls
add_common_vars, that is,  cgi,cgid, fastcgi, includes,...

I thought a document_root hook was more elegant. Or maybe a
add_common_vars hook? [I would be happy to supply it]


Unfortunately it is not coupled to the DOCUMENT_ROOT
variable, but I'd look at remedying this over building on ap_document_root,
which should simply go away, IMHO.


What's so bad about ap_document_root?  I know the source says 'dont
use' because it won't be right with mod_userdir etc. But with a hook
it would be possible to get right.

/Jakob

Stopping bogus clients in Apache 1.3

[Jakob Goldbach]

Hi,

Right now many p2p clients thinks my apache is part of their network,
dcplusplus.

All apache childs are occupied and I see many loglines like this in my
error.log

[Fri Oct  6 10:06:36 2006] [error] [client 85.166.201.160] request
failed: erroneous characters after protocol string: $MyNick
generalbassist|$Lock EXTENDEDPROTOCOLABCABCABCABCABCABC
Pk=DCPLUSPLUS0.689ABCABC|

Apparently the p2p client connects, startes with a $MyNick... line and
then wait for response.

I've decreased Timeout to 10 seconds - below is a strace output. Is
there anyway for me to hook in and drop these bogus request ??

Any help is appreciated
/Jakob



09:58:46 accept(15, {sa_family=AF_INET, sin_port=htons(4329),
sin_addr=inet_addr(clientip)}, [16]) = 10

09:58:46 rt_sigaction(SIGUSR1, {SIG_IGN}, {0x805f290, [], SA_INTERRUPT},
8) = 0

09:58:46 fcntl64(10, F_SETFD, FD_CLOEXEC) = 0

09:58:46 getsockname(10, {sa_family=AF_INET, sin_port=htons(80),
sin_addr=inet_addr(myip)}, [16]) = 0

09:58:46 setsockopt(10, SOL_TCP, TCP_NODELAY, [1], 4) = 0

09:58:46 read(10, $MyNick ElBarto|$Lock EXTENDEDPR..., 4096) = 82

09:58:46 read(10, 0x809dbc4, 4096)  = ? ERESTARTSYS (To berestarted)

09:58:58 --- SIGALRM (Alarm clock) @ 0 (0) ---

09:58:58 time(NULL) = 1160121538