Test reading fail when use https

Hi,

I use the function ngx_http_test_reading in the module Nginx Push Stream
Module (https://github.com/wandenberg/nginx-push-stream-module) to know
when the user goes off, but when using https this function always
return a value on call
n = recv(c->fd, buf, 1, MSG_PEEK);

Checking what was happening and comparing with others calls on same file
ngx_http_request.c I saw that others calls are done with c->recv instead
of recv .

On the case of https this c->recv points to ngx_ssl_recv, which can sure
detect when user goes off.

I would like to suggest to change the line
n = recv(c->fd, buf, 1, MSG_PEEK);
for
n = c->recv(c, buf, 1);

and
} else if (n == -1) {
for
} else if ((n == -1) || (n == -2)) {

to other returns on https.

I’m not sure if it has any side effect, and don’t know if here is the
best place to do this suggestion, sorry if not.

Regards,
Wandenberg

Posted at Nginx Forum:
http://forum.nginx.org/read.php?2,209967,209967#msg-209967

Hello!

On Sat, Jun 25, 2011 at 10:49:32AM -0400, wandenberg wrote:

of recv .
} else if (n == -1) {
for
} else if ((n == -1) || (n == -2)) {

to other returns on https.

I’m not sure if it has any side effect, and don’t know if here is the
best place to do this suggestion, sorry if not.

There is a MSG_PEEK for reason.

Maxim D.

Sorry Maxim. I didn’t understood your answer.

Can you explain better? Why this work for detect when users goes off on
http, but not in https ?

Posted at Nginx Forum:
http://forum.nginx.org/read.php?2,209967,210038#msg-210038

Hello!

On Sat, Jun 25, 2011 at 04:06:30PM -0400, wandenberg wrote:

Sorry Maxim. I didn’t understood your answer.

You can’t use c->recv() as it doesn’t allow to peek. Using
c->recv() will result in possibly valid and required data to be
lost. And that’s why normal recv() is used instead there.

Can you explain better? Why this work for detect when users goes off on
http, but not in https ?

Quoting just 2 days old message from the same list[1]:

% Yes, the code in question won’t be able to detect SSL connection
% close with appropriate shutdown records sent by a client (as well
% as connection close with some other pending data, e.g. pipelined
% request). It is not possible to detect connection close with
% standard socket interface if there are pending data without
% reading that data first, which isn’t always desired/possible.
%
% In the particular case of SSL it should be possible to use
% SSL_peek() here, but better aproach is to use OS-provided hint,
% e.g. as EV_EOF provided by kqueue interface (and used by nginx).

[1] http://nginx.org/pipermail/nginx/2011-June/027672.html

Maxim D.

Hello!

Thanks for your explanation.
Now I understood, sorry for my mistakes.

Maxim D. Wrote:

required data to be
lost. And that’s why normal recv() is used
instead there.

I turned back to use normal recv() respecting msg_peek functionality.

a client (as well
possible to use
% SSL_peek() here, but better aproach is to use
OS-provided hint,
% e.g. as EV_EOF provided by kqueue interface (and
used by nginx).

[1]
http://nginx.org/pipermail/nginx/2011-June/027672.
html

To solve the problem of detect when users close connection on https I
did this implementation

#if (NGX_HTTP_SSL)
if (c->ssl == NULL) {
n = recv(c->fd, buf, 1, MSG_PEEK);
} else {
n = SSL_peek(c->ssl->connection, buf, 1);
}
#else
n = recv(c->fd, buf, 1, MSG_PEEK);
#endif

I would like to know if you see any other problem on that and if is
possible to this code be applied on nginx source?

Maxim D.


nginx mailing list
[email protected]
http://nginx.org/mailman/listinfo/nginx

Thanks for your quickly response.

Regards,
Wandenberg

Posted at Nginx Forum:
http://forum.nginx.org/read.php?2,209967,210178#msg-210178

Hello!

On Sun, Jun 26, 2011 at 09:28:29AM -0400, wandenberg wrote:

[…]

n = recv(c->fd, buf, 1, MSG_PEEK);

#endif

I would like to know if you see any other problem on that and if is
possible to this code be applied on nginx source?

SSL_peek may (and likely will in many cases) leave an error in
OpenSSL error queue, so I suspect this actually requires a bit
more code.

Maxim D.

Hi Maxim,

now I think the code is ok.
Could you check, please?

Maxim D. Wrote:

I would like to know if you see any other
Maxim D.


nginx mailing list
[email protected]
http://nginx.org/mailman/listinfo/nginx

#if (NGX_HTTP_SSL)
if (c->ssl != NULL) {
n = SSL_peek(c->ssl->connection, buf, 1);
} else {
n = recv(c->fd, buf, 1, MSG_PEEK);
}
#else
n = recv(c->fd, buf, 1, MSG_PEEK);
#endif

if (n == 0) {
    rev->eof = 1;
    c->error = 1;
    err = 0;

    goto closed;

} else if (n == -1) {

#if (NGX_HTTP_SSL)
if (c->ssl != NULL) {
err = SSL_get_error(c->ssl->connection, n);
} else {
err = ngx_socket_errno;
}

if ((err != NGX_EAGAIN) && (err != SSL_ERROR_WANT_READ) && (err !=

SSL_ERROR_WANT_WRITE)) {
#else
err = ngx_socket_errno;
if (err != NGX_EAGAIN) {
#endif
rev->eof = 1;
c->error = 1;

        goto closed;
    }
}

Posted at Nginx Forum:
http://forum.nginx.org/read.php?2,209967,211939#msg-211939