RE: client_socket bogosity...

2002-01-27 Thread Ryan Bloom

  The create_connection hook has a fatal design flaw. create_conn is
run
 before
  ap_update_vhost_given_ip(), which means that it is impossible to
install
 input and
 output
  filters based on vhost info.
 
 More concisely, it is now impossible to install filters in place of
 CORE_IN and CORE_OUT
 based on vhost config.

It was always impossible to do that.  The patches you are talking about
actually make it quite a bit cleaner for you to insert your filters
above the CORE filters however, because now there is no way for somebody
else to use the socket that was a part conn_rec.  The other advantage to
that patch is that if you want to insert filters based on where the
connection came from, you can.  Those patches were meant to allow
different bottom-level filters based on the type of socket used, not
based on vhost config.

Ryan





RE: client_socket bogosity...

2002-01-25 Thread Ryan Bloom

 One Nov. 12, Ryan committed a patch creating the create_conn hook. The
 idea was to move
 the client_socket out of the conn_rec presumably to make available
only to
 the core_in and
 core_out filters.  However, I just found a backdoor...
 
 In core_create_conn() the socket is saved away thusly:
 ap_set_module_config(net-c-conn_config, core_module, csd);
 
 And whoever needs to access the socket does this:
 apr_socket_t *csd = ap_get_module_config(f-c-conn_config,
core_module);

That hack was added because the proxy does the completely wrong thing
with regard to handing sockets.  In order to finish the Nov. 12 patch, I
need to rip a lot of logic out of the proxy and re-implement, which I
haven't had time to do recently.  The only other module that should use
the get_module_config hack is the perchild module, which is also doing
to completely wrong thing with regard to sockets, but I haven't had time
to fix that one either.


 So the goal of hiding the socket is completely blown.  The Nov. 11
change
 added a lot of
 complexity to the server (hard to read/understand code) in pursuit of
a
 goal that is then
 immediately circumvented by the ap_get|set_module_config. So we made
the
 server more
 complex for no reason.

It actually isn't blown.  Try writing a module that implements a non TCP
socket, and it will work as long as you don't use the proxy or the
perchild module.  As proof, look at the fact that the Unix MPMs have
been using that mechanism to handle the pipe_of_death.  This allowed me
to remove the ugly hacks at the beginning of the accept loop, which
checked for the POD.

Also, a big portion of the Nov 12 patch was to consolidate the accept
functions for Unix and BeOS, which has meant far less duplicated code in
the server.

 I am on the verge of vetoing the Nov. 12 patch in favor of moving the
 client_socket back
 into the con_rec.
 
 Comments?

Please don't let two mis-behaved modules color your judgment on this.
Both proxy and perchild must be re-written if they are going to be
clean, and once that is done the stupid set_module_config can be
removed.  In fact, the server ran for over a day without the
set_module_config, but that broke the proxy, so I added the hack to
allow the proxy to continue to work, while I worked to solve the
underlying problem.  Unfortunately, work and some extracurricular
activities have stopped me from contributing much code recently.
Hopefully, I will have time to code again soon.

Ryan





RE: client_socket bogosity...

2002-01-25 Thread Allan Edwards

 Please don't let two mis-behaved modules color your judgment on this.
 Both proxy and perchild must be re-written if they are going to be
 clean, and once that is done the stupid set_module_config can be
 removed.  In fact, the server ran for over a day without the
 set_module_config, but that broke the proxy, so I added the hack to
 allow the proxy to continue to work, while I worked to solve the
 underlying problem.  

mmm... I'd be interested to know what the solution is for 
net_time_filter since it is using the ap_get_module_config 
hack also.

Allan



RE: client_socket bogosity...

2002-01-25 Thread Ryan Bloom

  Please don't let two mis-behaved modules color your judgment on
this.
  Both proxy and perchild must be re-written if they are going to be
  clean, and once that is done the stupid set_module_config can be
  removed.  In fact, the server ran for over a day without the
  set_module_config, but that broke the proxy, so I added the hack to
  allow the proxy to continue to work, while I worked to solve the
  underlying problem.
 
 mmm... I'd be interested to know what the solution is for
 net_time_filter since it is using the ap_get_module_config
 hack also.

The real solution is to pass the core_net_rec structure to the NET_TIME
filter as user-data for the filter, the same way we do for CORE_IN and
CORE_OUT.

Ryan





RE: client_socket bogosity...

2002-01-25 Thread Allan Edwards

 The real solution is to pass the core_net_rec structure to the NET_TIME
 filter as user-data for the filter, the same way we do for CORE_IN and
 CORE_OUT.

But you'll have to play some sort of trick to do that since the
NET_TIME filter is added from the create_request hook which does
not know about core_net_rec, only core_create_conn knows about that.
I suppose some core specific magic could be played but that would 
prevent any other module from running it's own net_time hook.

Allan  



RE: client_socket bogosity...

2002-01-25 Thread Allan Edwards

 prevent any other module from running it's own net_time hook.

I meant net_time filter...



RE: client_socket bogosity...

2002-01-25 Thread Ryan Bloom

 
  The real solution is to pass the core_net_rec structure to the
NET_TIME
  filter as user-data for the filter, the same way we do for CORE_IN
and
  CORE_OUT.
 
 But you'll have to play some sort of trick to do that since the
 NET_TIME filter is added from the create_request hook which does
 not know about core_net_rec, only core_create_conn knows about that.
 I suppose some core specific magic could be played but that would
 prevent any other module from running it's own net_time hook.

This is why I didn't do this originally, but it wouldn't be hard to add.
As long as it's just the core that knows about the core_net_rec, we are
okay.  I just ran out of time while doing the implementation.

Ryan





RE: client_socket bogosity...

2002-01-25 Thread Ryan Bloom

 On Fri, Jan 25, 2002 at 02:01:26PM -0800, Ryan Bloom wrote:
Please don't let two mis-behaved modules color your judgment on
  this.
Both proxy and perchild must be re-written if they are going to
be
clean, and once that is done the stupid set_module_config can be
removed.  In fact, the server ran for over a day without the
set_module_config, but that broke the proxy, so I added the hack
to
allow the proxy to continue to work, while I worked to solve the
underlying problem.
  
   mmm... I'd be interested to know what the solution is for
   net_time_filter since it is using the ap_get_module_config
   hack also.
 
  The real solution is to pass the core_net_rec structure to the
NET_TIME
  filter as user-data for the filter, the same way we do for CORE_IN
and
  CORE_OUT.
 
 You're kidding, right? The core_net_rec is private to the core. You
can't
 just start passing that around the server. That just breaks the
 abstraction
 that you've worked so hard to implement.
 
 No... something else needs to be figured out [for the
net_time_filter].

But the net_time_filter is implemented by the core.  If we really want
to abstract this more, we can create a function that, given a conn_rec
can set a timeout on the socket.  I think I see how that can be done.
Or, we can actually create a net_module, that has a spot in the
c-conf_vector that is the net_rec.  That module can implement the logic
to set a timeout on a connection.  Which is really what we want, to be
able to set a timeout on a connection, not on a socket.

Ryan





Re: client_socket bogosity...

2002-01-25 Thread Bill Stoddard

The create_connection hook has a fatal design flaw. create_conn is run before
ap_update_vhost_given_ip(), which means that it is impossible to install input and 
output
filters based on vhost info.

I want to install SSL_IN and SSL_OUT filters if the request is coming in to a 
vhost/port
enabled for SSL and that can't be done with the create_connection hook.

Bill

  One Nov. 12, Ryan committed a patch creating the create_conn hook. The
  idea was to move
  the client_socket out of the conn_rec presumably to make available
 only to
  the core_in and
  core_out filters.  However, I just found a backdoor...
 
  In core_create_conn() the socket is saved away thusly:
  ap_set_module_config(net-c-conn_config, core_module, csd);
 
  And whoever needs to access the socket does this:
  apr_socket_t *csd = ap_get_module_config(f-c-conn_config,
 core_module);

 That hack was added because the proxy does the completely wrong thing
 with regard to handing sockets.  In order to finish the Nov. 12 patch, I
 need to rip a lot of logic out of the proxy and re-implement, which I
 haven't had time to do recently.  The only other module that should use
 the get_module_config hack is the perchild module, which is also doing
 to completely wrong thing with regard to sockets, but I haven't had time
 to fix that one either.


  So the goal of hiding the socket is completely blown.  The Nov. 11
 change
  added a lot of
  complexity to the server (hard to read/understand code) in pursuit of
 a
  goal that is then
  immediately circumvented by the ap_get|set_module_config. So we made
 the
  server more
  complex for no reason.

 It actually isn't blown.  Try writing a module that implements a non TCP
 socket, and it will work as long as you don't use the proxy or the
 perchild module.  As proof, look at the fact that the Unix MPMs have
 been using that mechanism to handle the pipe_of_death.  This allowed me
 to remove the ugly hacks at the beginning of the accept loop, which
 checked for the POD.

 Also, a big portion of the Nov 12 patch was to consolidate the accept
 functions for Unix and BeOS, which has meant far less duplicated code in
 the server.

  I am on the verge of vetoing the Nov. 12 patch in favor of moving the
  client_socket back
  into the con_rec.
 
  Comments?

 Please don't let two mis-behaved modules color your judgment on this.
 Both proxy and perchild must be re-written if they are going to be
 clean, and once that is done the stupid set_module_config can be
 removed.  In fact, the server ran for over a day without the
 set_module_config, but that broke the proxy, so I added the hack to
 allow the proxy to continue to work, while I worked to solve the
 underlying problem.  Unfortunately, work and some extracurricular
 activities have stopped me from contributing much code recently.
 Hopefully, I will have time to code again soon.

 Ryan







Re: client_socket bogosity...

2002-01-25 Thread Ian Holsman

Bill Stoddard wrote:
 The create_connection hook has a fatal design flaw. create_conn is run before
 ap_update_vhost_given_ip(), which means that it is impossible to install input and 
output
 filters based on vhost info.
 
 I want to install SSL_IN and SSL_OUT filters if the request is coming in to a 
vhost/port
 enabled for SSL and that can't be done with the create_connection hook.
 
 Bill
 

On that point.
I don't think there is any way of inserting a proxy specifc filter 
either, as their is now way for the hook to know what kind of request
is the connection is for.

 
One Nov. 12, Ryan committed a patch creating the create_conn hook. The
idea was to move
the client_socket out of the conn_rec presumably to make available

only to

the core_in and
core_out filters.  However, I just found a backdoor...

In core_create_conn() the socket is saved away thusly:
ap_set_module_config(net-c-conn_config, core_module, csd);

And whoever needs to access the socket does this:
apr_socket_t *csd = ap_get_module_config(f-c-conn_config,

core_module);

That hack was added because the proxy does the completely wrong thing
with regard to handing sockets.  In order to finish the Nov. 12 patch, I
need to rip a lot of logic out of the proxy and re-implement, which I
haven't had time to do recently.  The only other module that should use
the get_module_config hack is the perchild module, which is also doing
to completely wrong thing with regard to sockets, but I haven't had time
to fix that one either.



So the goal of hiding the socket is completely blown.  The Nov. 11

change

added a lot of
complexity to the server (hard to read/understand code) in pursuit of

a

goal that is then
immediately circumvented by the ap_get|set_module_config. So we made

the

server more
complex for no reason.

It actually isn't blown.  Try writing a module that implements a non TCP
socket, and it will work as long as you don't use the proxy or the
perchild module.  As proof, look at the fact that the Unix MPMs have
been using that mechanism to handle the pipe_of_death.  This allowed me
to remove the ugly hacks at the beginning of the accept loop, which
checked for the POD.

Also, a big portion of the Nov 12 patch was to consolidate the accept
functions for Unix and BeOS, which has meant far less duplicated code in
the server.


I am on the verge of vetoing the Nov. 12 patch in favor of moving the
client_socket back
into the con_rec.

Comments?

Please don't let two mis-behaved modules color your judgment on this.
Both proxy and perchild must be re-written if they are going to be
clean, and once that is done the stupid set_module_config can be
removed.  In fact, the server ran for over a day without the
set_module_config, but that broke the proxy, so I added the hack to
allow the proxy to continue to work, while I worked to solve the
underlying problem.  Unfortunately, work and some extracurricular
activities have stopped me from contributing much code recently.
Hopefully, I will have time to code again soon.

Ryan



 
 






Re: client_socket bogosity...

2002-01-25 Thread Bill Stoddard

 The create_connection hook has a fatal design flaw. create_conn is run before
 ap_update_vhost_given_ip(), which means that it is impossible to install input and
output
 filters based on vhost info.

More concisely, it is now impossible to install filters in place of CORE_IN and 
CORE_OUT
based on vhost config.

Bill