Erlang/OTP Forums

Author Message

<  Yaws mailing list  ~  keepalive_timeout

Guest
Posted: Sun May 09, 2010 5:33 pm Reply with quote
Guest
erlyaws-list,

I was trying to set gconf.keepalive_timeout to make the http sockets
stay open for a longer-than-usual time. However,
gconf.keepalive_timeout appears to not be honored. I considered the
code and could not find anywhere it is being utilized, besides being
parsed out of the configuration file. So I made the following
modification to yaws.erl, around line 1776:

do_recv(Sock, Num, nossl) ->
gen_tcp:recv(Sock, Num, (get(gc))#gconf.keepalive_timeout);
do_recv(Sock, Num, ssl) ->
ssl:recv(Sock, Num, (get(gc))#gconf.keepalive_timeout).

%%do_recv(Sock, Num, nossl) ->
%% gen_tcp:recv(Sock, Num, ?READ_TIMEOUT);
%%do_recv(Sock, Num, ssl) ->
%% ssl:recv(Sock, Num, ?READ_TIMEOUT).

This causes the socket read timeout to be gconf.keepalive_timeout and
I get the behavior I desire. Does anyone know why this might be a
bad/wrong idea?

Brady

------------------------------------------------------------------------------

_______________________________________________
Erlyaws-list mailing list
Erlyaws-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/erlyaws-list
Post received from mailinglist
Guest
Posted: Sun May 09, 2010 9:00 pm Reply with quote
Guest
On 05/09/2010 07:32 PM, Brady McCary wrote:
> erlyaws-list,
>
> I was trying to set gconf.keepalive_timeout to make the http sockets
> stay open for a longer-than-usual time. However,
> gconf.keepalive_timeout appears to not be honored. I considered the
> code and could not find anywhere it is being utilized, besides being
> parsed out of the configuration file. So I made the following
> modification to yaws.erl, around line 1776:
>
> do_recv(Sock, Num, nossl) ->
> gen_tcp:recv(Sock, Num, (get(gc))#gconf.keepalive_timeout);
> do_recv(Sock, Num, ssl) ->
> ssl:recv(Sock, Num, (get(gc))#gconf.keepalive_timeout).
>
> %%do_recv(Sock, Num, nossl) ->
> %% gen_tcp:recv(Sock, Num, ?READ_TIMEOUT);
> %%do_recv(Sock, Num, ssl) ->
> %% ssl:recv(Sock, Num, ?READ_TIMEOUT).
>
> This causes the socket read timeout to be gconf.keepalive_timeout and
> I get the behavior I desire. Does anyone know why this might be a
> bad/wrong idea?
>


Hmmm, ... difficult. And, well looking at the code, the fix certainly
seems like the right thing to do.
And the fact that #gconf.keepalive_timeout is never used, and there is
neither an yaws.conf way to set it ... suspicious.

The fix seems right.

If you have the energy, please make a proper patch including
yaws_config.erl and yaws.conf.5 man page and post it.


/klacke








------------------------------------------------------------------------------

_______________________________________________
Erlyaws-list mailing list
Erlyaws-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/erlyaws-list
Post received from mailinglist
Guest
Posted: Mon May 10, 2010 1:58 am Reply with quote
Guest
klacke,

Yes, I will be happy to do so, with the guidance of the list. I will
try to describe the complication.

There are 3 versions of HTTP and 3 interpretations of Keep-Alive.
HTTP/0.9 and HTTP/1.0 have no notion of Keep-Alive in their formal
specs, but informally clients will often send one of the following
headers:

Keep-Alive: No-Meaningful-Args
Connection: Keep-Alive
Connection: Close

The Keep-Alive: header is a first-attempt at keeping the socket
connection open for multiple requests. It inspired the Connection:
header in the formal spec of HTTP/1.1. This is the current behavior of
YAWS.

1. HTTP/0.9 always close the connection.
2. HTTP/1.0 close the connection immediately unless the client has
supplied "Connection: Keep-Alive", in which case the connection will
remain open for ?READ_TIMEOUT milliseconds.
3. HTTP/1.1 always keep the connection open for ?READ_TIMEOUT
milliseconds unless the user has specified "Connection: close".

The ?READ_TIMEOUT is a constant defined in include/yaws.hrl which is
currently 30000 milliseconds. There is a field #headers.keep_alive,
and it is referred to in a few places, but it appears that it is only
being parsed and that its value does not affect the behavior of
keeping sockets alive any more.

Further, there is a read_timeout parameter references in
src/yaws_config.erl with the comment that it is deprecated and unused.
I imagine that the ?READ_TIMEOUT constant replaced the read_timeout
yaws.conf parameter when WebDAV support came in from the comment about
DAV support in yaws.conf.5:

"Turns on the DAV protocol for this server. The dav support
in yaws is highly limited. If dav is turned on, .yaws processing
of .yaws pages is turned off. Default is false.
Setting it to nolimit is potentially
dangerous. There is a hardcoded timeout on POST reads at 30 seconds.
If the read is not done within the timeout, the POST will fail."

So, why was I interested in fleshing out this issue? Because I want to
be able to specify the amount of time a socket will remain open if an
HTTP/1.0 or HTTP/1.1 client connection is in "Connection: Keep-Alive"
mode. This can be accomplished by allowing the user to specify the
read timeout in gen_tcp:recv/3 or ssl:recv/3, which is currently set
by ?READ_TIMEOUT. I have done this in my local YAWS source. However,
yaws_dav uses the same code path and I am not sure the implications
this might have. Most negative interactions with WebDAV can be avoided
(I think) by making the new parameter default to 30000 milliseconds,
which is the current default. I don't think there will be any other
negative interactions with WebDAV b/c WebDAV is a set of extensions to
HTTP/1.1, so presumably a working HTTP/1.1 implementation will not
interfere with WebDAV.

So my proposed fix is:

1. Remove all references to read_timeout which is not currently used.
2. Remove all references to READ_TIMEOUT which is currently used as
the socket read timeout.
3. Replace READ_TIMEOUT with the currently unused
#gconf.keepalive_timeout variable.
4. Change the default #gconf.keepalive_timeout to 30000.
5. Edit the man pages.

Comments before I proceed?

Brady

On Sun, May 9, 2010 at 3:59 PM, Claes Wikstrom <klacke@tail-f.com> wrote:
> On 05/09/2010 07:32 PM, Brady McCary wrote:
>> erlyaws-list,
>>
>> I was trying to set gconf.keepalive_timeout to make the http sockets
>> stay open for a longer-than-usual time. However,
>> gconf.keepalive_timeout appears to not be honored. I considered the
>> code and could not find anywhere it is being utilized, besides being
>> parsed out of the configuration file. So I made the following
>> modification to yaws.erl, around line 1776:
>>
>> do_recv(Sock, Num, nossl) ->
>>
Guest
Posted: Mon May 10, 2010 3:16 am Reply with quote
Guest
klacke,

I've attached the diffs.

scripts/yaws.conf.template.diff
man/yaws.conf.5.diff
src/yaws.erl.diff
include/yaws.hrl.diff

Brady

On Sun, May 9, 2010 at 8:57 PM, Brady McCary
<brady.mccary+YAWS@gmail.com> wrote:
> klacke,
>
> Yes, I will be happy to do so, with the guidance of the list. I will
> try to describe the complication.
>
> There are 3 versions of HTTP and 3 interpretations of Keep-Alive.
> HTTP/0.9 and HTTP/1.0 have no notion of Keep-Alive in their formal
> specs, but informally clients will often send one of the following
> headers:
>
>
Guest
Posted: Mon May 10, 2010 5:09 am Reply with quote
Guest
On Sun, May 9, 2010 at 11:15 PM, Brady McCary
<brady.mccary+YAWS@gmail.com> wrote:
> klacke,
>
> I've attached the diffs.
>
> scripts/yaws.conf.template.diff
> man/yaws.conf.5.diff
> src/yaws.erl.diff
> include/yaws.hrl.diff

Hi Brady,

I started to apply your patches but noticed what looks like a problem:
in yaws.erl you added a check for the Keep-Alive header, setting the
connection to stay open if that header is present. An issue with this
is that the Keep-Alive header is meaningful only under particular
circumstances. RFC 2068 says:

"If the Keep-Alive header is sent, the corresponding connection token
MUST be transmitted. The Keep-Alive header MUST be ignored if received
without the connection token."

The "connection token" they refer to here is the Connection header set
as follows:

Connection: Keep-Alive

I therefore think the current handling of Connection: Keep-Alive in
Yaws already does what's required, and taking this part of your patch
would allow the Keep-Alive header to work without a corresponding
Connection: Keep-Alive header being present, which I believe would be
incorrect.

The only reason I can think of to look at the Keep-Alive header at all
would be if you wanted to check its parameter value, but to the best
of my knowledge values for this header were never standardized,
meaning there's nothing Yaws can meaningfully do with any values it
might find for this header. Ignoring it is the right thing.

So, I propose leaving out this part of your patch. The rest of it looks OK.

--steve

------------------------------------------------------------------------------

_______________________________________________
Erlyaws-list mailing list
Erlyaws-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/erlyaws-list
Post received from mailinglist
Guest
Posted: Mon May 10, 2010 3:21 pm Reply with quote
Guest
Steve,

I agree that YAWS is currently doing the right thing with respect to
Connection and Keep-Alive headers. I have attached the updated patch
for src/yaws.erl.

Brady

On Mon, May 10, 2010 at 12:07 AM, Steve Vinoski <vinoski@ieee.org> wrote:
> On Sun, May 9, 2010 at 11:15 PM, Brady McCary
> <brady.mccary+YAWS@gmail.com> wrote:
>> klacke,
>>
>> I've attached the diffs.
>>
>> scripts/yaws.conf.template.diff
>> man/yaws.conf.5.diff
>> src/yaws.erl.diff
>> include/yaws.hrl.diff
>
> Hi Brady,
>
> I started to apply your patches but noticed what looks like a problem:
> in yaws.erl you added a check for the Keep-Alive header, setting the
> connection to stay open if that header is present. An issue with this
> is that the Keep-Alive header is meaningful only under particular
> circumstances. RFC 2068 says:
>
> "If the Keep-Alive header is sent, the corresponding connection token
> MUST be transmitted. The Keep-Alive header MUST be ignored if received
> without the connection token."
>
> The "connection token" they refer to here is the Connection header set
> as follows:
>
> Connection: Keep-Alive
>
> I therefore think the current handling of Connection: Keep-Alive in
> Yaws already does what's required, and taking this part of your patch
> would allow the Keep-Alive header to work without a corresponding
> Connection: Keep-Alive header being present, which I believe would be
> incorrect.
>
> The only reason I can think of to look at the Keep-Alive header at all
> would be if you wanted to check its parameter value, but to the best
> of my knowledge values for this header were never standardized,
> meaning there's nothing Yaws can meaningfully do with any values it
> might find for this header. Ignoring it is the right thing.
>
> So, I propose leaving out this part of your patch. The rest of it looks OK.
>
> --steve
>


Post received from mailinglist
Guest
Posted: Mon May 10, 2010 6:02 pm Reply with quote
Guest
On Mon, May 10, 2010 at 11:20 AM, Brady McCary
<brady.mccary+YAWS@gmail.com> wrote:
> Steve,
>
> I agree that YAWS is currently doing the right thing with respect to
> Connection and Keep-Alive headers. I have attached the updated patch
> for src/yaws.erl.

OK, thanks. I've incorporated and pushed your changes to github. I
hope you don't mind but I slightly edited some of your documentation
changes for consistency, and I also modified the Yaws PDF
documentation to reflect this change.

We appreciate your contribution.

thanks,
--steve

------------------------------------------------------------------------------

_______________________________________________
Erlyaws-list mailing list
Erlyaws-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/erlyaws-list
Post received from mailinglist

Display posts from previous:  

All times are GMT
Page 1 of 1
This forum is locked: you cannot post, reply to, or edit topics.

Jump to:  

You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
You can attach files in this forum
You can download files in this forum