Patch for fdsrv support

[Roberto Saccon]

I'd like to use MochiWeb also for ports below 1024 without reverse-
proxying. Unfortunately I haven't found a way to extend Mochiweb with
the fdsrv stuff without hacking mochiweb_socket_server.erl. If there
exists a way to do so, please let me know, otherwise the patch below
might be useful for others and of course I would love to see it
applied to trunk !

regards
Roberto


Index: /Users/rsaccon/opensource/mochiweb-read-only/src/
mochiweb_socket_server.erl
===================================================================
--- /Users/rsaccon/opensource/mochiweb-read-only/src/
mochiweb_socket_server.erl (revision 20)
+++ /Users/rsaccon/opensource/mochiweb-read-only/src/
mochiweb_socket_server.erl (working copy)
@@ -21,7 +21,8 @@
max=2048,
ip=any,
listen=null,
- acceptor=null}).
+ acceptor=null,
+ fdsrv=false}).

start(State=#mochiweb_socket_server{}) ->
start_server(State);
@@ -63,6 +64,8 @@
parse_options(Rest, State#mochiweb_socket_server{port=Port});
parse_options([{port, Port} | Rest], State) ->
parse_options(Rest, State#mochiweb_socket_server{port=Port});
+parse_options([{fdsrv, Fdsrv} | Rest], State) ->
+ parse_options(Rest, State#mochiweb_socket_server{fdsrv=Fdsrv});
parse_options([{ip, Ip} | Rest], State) ->
ParsedIp = case Ip of
any ->
@@ -93,7 +96,7 @@
gen_server:start_link(Name, ?MODULE, State, [])
end.

-init(State=#mochiweb_socket_server{ip=Ip, port=Port}) ->
+init(State=#mochiweb_socket_server{ip=Ip, port=Port, fdsrv=Fdrsv}) ->
process_flag(trap_exit, true),
BaseOpts = [binary,
{reuseaddr, true},
@@ -107,13 +110,31 @@
Ip ->
[{ip, Ip} | BaseOpts]
end,
+ case Fdrsv of
+ true ->
+ case fdsrv:start() of
+ {ok, _} ->
+ case fdsrv:bind_socket(tcp, Port) of
+ {ok, Fd} ->
+ gen_tcp_listen(Port, [{fd, Fd} | Opts],
State);
+ _ ->
+ {stop, fdsrv_bind_failed}
+ end;
+ _ ->
+ {stop, fdsrv_start_failed}
+ end;
+ _ ->
+ gen_tcp_listen(Port, Opts, State)
+ end.
+
+gen_tcp_listen(Port, Opts, State) ->
case gen_tcp:listen(Port, Opts) of
- {ok, Listen} ->
- {ok, ListenPort} = inet:port(Listen),
- {ok, new_acceptor(State#mochiweb_socket_server{listen=Listen,
- port=ListenPort})};
- {error, Reason} ->
- {stop, Reason}
+ {ok, Listen} ->
+ {ok, ListenPort} = inet:port(Listen),
+ {ok,
new_acceptor(State#mochiweb_socket_server{listen=Listen,
+
port=ListenPort})};
+ {error, Reason} ->
+ {stop, Reason}
end.

new_acceptor(State=#mochiweb_socket_server{max=0}) ->
@@ -163,9 +184,15 @@
handle_cast(stop, State) ->
{stop, normal, State}.

-terminate(_Reason, #mochiweb_socket_server{listen=Listen}) ->
+terminate(_Reason, #mochiweb_socket_server{listen=Listen,
fdsrv=Fdsrv}) ->
gen_tcp:close(Listen),
- ok.
+ case Fdsrv of
+ true ->
+ fdsrv:stop(),
+ ok;
+ _ ->
+ ok
+ end.

code_change(_OldVsn, State, _Extra) ->
State.

[Bob Ippolito]

It'd probably be better to just assume that if port =< 1024 then it
should try fdsrv, which wouldn't change the record layout. The only
times you'd be allowed to open those ports without fdsrv are windows
(maybe?) and running as root and neither of those should be done (in
production at least).

I'm not really very convinced that fdsrv is a great idea. Using a
reverse proxy or pf/iptables seems like a much better solution to me.
We need something like nginx anyway to serve static content, load
balance, and to send some URLs to other services (e.g. php over
fastcgi, python http servers, etc.). Why is fdsrv important to you?

-bob

[Roberto Saccon]

On Nov 25, 9:55 am, "Bob Ippolito" <b...@redivi.com> wrote:
> It'd probably be better to just assume that if port =< 1024 then it
> should try fdsrv, which wouldn't change the record layout. The only
> times you'd be allowed to open those ports without fdsrv are windows
> (maybe?) and running as root and neither of those should be done (in
> production at least).

that makes perfect sense. At my app I actually set the fsdrv property
based on that criteria. The only reason I suggested to modify the
record, is hat I heard that there exist patches for running Linux and
FreeBSD with port < 1024 as non-root.

>
> I'm not really very convinced that fdsrv is a great idea. Using a
> reverse proxy or pf/iptables seems like a much better solution to me.
> We need something like nginx anyway to serve static content, load
> balance, and to send some URLs to other services (e.g. php over
> fastcgi, python http servers, etc.). Why is fdsrv important to you?

For two reasons, and I really hope I am wrong on both ! First, because
it is so easy to set up. Second, the main reason, I do chunked
responses, were the HTPP connection stays open (for erlycomet, and
also for RTMPT flash video) and long time ago when I was messing with
yaws, I investigated about nginx and I was told at that time that
reverse proxying was not suited for streaming type of applications. I
would love to hear that this has changed in the meantime !

regards
Roberto

[Bob Ippolito]

On 11/25/07, Roberto Saccon <rsa...@gmail.com> wrote:
>
>
>
> On Nov 25, 9:55 am, "Bob Ippolito" <b...@redivi.com> wrote:
> > It'd probably be better to just assume that if port =< 1024 then it
> > should try fdsrv, which wouldn't change the record layout. The only
> > times you'd be allowed to open those ports without fdsrv are windows
> > (maybe?) and running as root and neither of those should be done (in
> > production at least).
>
> that makes perfect sense. At my app I actually set the fsdrv property
> based on that criteria. The only reason I suggested to modify the
> record, is hat I heard that there exist patches for running Linux and
> FreeBSD with port < 1024 as non-root.

Well if someone needs to do that sans fdsrv then I'm sure they'll
speak up. Maybe the patch could try gen_tcp first, and if it fails,
then try fdsrv (or vice versa)? On closing the socket you could just
close it both ways and catch the badarg, there's no meaningful result
or error code from closing a socket anyway.

> >
> > I'm not really very convinced that fdsrv is a great idea. Using a
> > reverse proxy or pf/iptables seems like a much better solution to me.
> > We need something like nginx anyway to serve static content, load
> > balance, and to send some URLs to other services (e.g. php over
> > fastcgi, python http servers, etc.). Why is fdsrv important to you?
>
> For two reasons, and I really hope I am wrong on both ! First, because
> it is so easy to set up. Second, the main reason, I do chunked
> responses, were the HTPP connection stays open (for erlycomet, and
> also for RTMPT flash video) and long time ago when I was messing with
> yaws, I investigated about nginx and I was told at that time that
> reverse proxying was not suited for streaming type of applications. I
> would love to hear that this has changed in the meantime !
>

Okay, that sounds reasonable. If you put together a new patch that
uses fdsrv for Port < 1024 instead of changing the record then I'll
apply it.

-bob

[Roberto Saccon]

Here is the updated patch with fdsrv for Port < 1024 and not touching
the reocrd:

Index: /Users/rsaccon/opensource/mochiweb-read-only/src/
mochiweb_socket_server.erl
===================================================================
--- /Users/rsaccon/opensource/mochiweb-read-only/src/
mochiweb_socket_server.erl (revision 20)
+++ /Users/rsaccon/opensource/mochiweb-read-only/src/
mochiweb_socket_server.erl (working copy)

@@ -107,13 +107,36 @@

Ip ->
[{ip, Ip} | BaseOpts]
end,

+ case gen_tcp_listen(Port, Opts, State) of
+ {error, Reason} ->
+ if
+ Port < 1024 ->

+ case fdsrv:start() of
+ {ok, _} ->
+ case fdsrv:bind_socket(tcp, Port) of
+ {ok, Fd} ->
+ gen_tcp_listen(Port, [{fd, Fd} |
Opts], State);
+ _ ->
+ {stop, fdsrv_bind_failed}
+ end;
+ _ ->
+ {stop, fdsrv_start_failed}
+ end;

+ true ->
+ {error, Reason}
+ end;
+ Ok ->
+ Ok

+ end.
+
+gen_tcp_listen(Port, Opts, State) ->
case gen_tcp:listen(Port, Opts) of
- {ok, Listen} ->
- {ok, ListenPort} = inet:port(Listen),
- {ok, new_acceptor(State#mochiweb_socket_server{listen=Listen,
- port=ListenPort})};
- {error, Reason} ->
- {stop, Reason}
+ {ok, Listen} ->
+ {ok, ListenPort} = inet:port(Listen),
+ {ok,
new_acceptor(State#mochiweb_socket_server{listen=Listen,
+
port=ListenPort})};
+ {error, Reason} ->
+ {stop, Reason}
end.

new_acceptor(State=#mochiweb_socket_server{max=0}) ->

@@ -163,9 +186,15 @@

handle_cast(stop, State) ->
{stop, normal, State}.

-terminate(_Reason, #mochiweb_socket_server{listen=Listen}) ->

+terminate(_Reason, #mochiweb_socket_server{listen=Listen, port=Port})

->
gen_tcp:close(Listen),
- ok.

+ if
+ Port < 1024 ->

+ fdsrv:stop(),
+ ok;

+ true ->

[Bob Ippolito]

Can you send this as an attachment? Gmail and/or google groups mangled
the patch.

On 11/25/07, Roberto Saccon <rsa...@gmail.com> wrote:
>

[Roberto Saccon]

I uploaded mochiweb_socket_server.diff to the file section of the
google groups web interface

regards
Roberto

On Nov 26, 10:46 pm, "Bob Ippolito" <b...@redivi.com> wrote:
> Can you send this as an attachment? Gmail and/or google groups mangled
> the patch.
>

[Bob Ippolito]

Awesome, applied cleanly in r25. As a matter of style I changed the
"if" expressions to "case" (we don't otherwise use "if" anywhere), but
otherwise it's unmodified. Let me know if it still works, we don't
have fdsrv installed anywhere right now.

[Roberto Saccon]

I tested it and discovered a problem:

we need to check for {stop, eacces} and not for {error, Reason} before
doing the fdsrv stuff

I have prepared a new patch "mochiweb_socket_server-2.diff" in the
filesection

regards
Roberto

On Nov 27, 3:14 am, "Bob Ippolito" <b...@redivi.com> wrote:
> Awesome, applied cleanly in r25. As a matter of style I changed the
> "if" expressions to "case" (we don't otherwise use "if" anywhere), but
> otherwise it's unmodified. Let me know if it still works, we don't
> have fdsrv installed anywhere right now.
>

[Bob Ippolito]

Applied in r26.

[Roberto Saccon]

great, thanks very much

On Nov 27, 12:58 pm, "Bob Ippolito" <b...@redivi.com> wrote:
> Applied in r26.
>