[Libwebsockets] Issues with libwebsockets

Jaycocks, Mark (Mark) mjaycocks at avaya.com
Fri May 9 13:10:38 CEST 2014


Hello,

Whilst trialling out libwebsockets I have discovered the following three issues that you may want to consider for inclusion.

Issue 1
=====
The Microsoft sprint function can get confused when a 64-bit quantity is passed for a %lu. This then generates a crash.
The function lws_latency uses sprint to add in the latency_start yet is using an local variable unsigned long long u. The format should be using %llu rather than %lu otherwise the function can crash (it does not process the %s correctly).

This is the fix:
#ifdef LWS_LATENCY
void
lws_latency(struct libwebsocket_context *context, struct libwebsocket *wsi,
				     const char *action, int ret, int completed)
{
	unsigned long long u;
	char buf[256];

	u = time_in_microseconds();

	if (!action) {
		wsi->latency_start = u;
		if (!wsi->action_start)
			wsi->action_start = u;
		return;
	}
	if (completed) {
		if (wsi->action_start == wsi->latency_start)
			sprintf(buf,
//<=========================================>
// START OF CODE FIX (use %lluus instead of %luus).
//<=========================================>
			  "Completion first try lat %lluus: %p: ret %d: %s\n",
//<=========================================>
// END OF CODE FIX.
//<=========================================>
					u - wsi->latency_start,
						      (void *)wsi, ret, action);
		else
			sprintf(buf,
//<=========================================>
// START OF CODE FIX (use %lluus instead of %luus).
//<=========================================>
			  "Completion %lluus: lat %lluus: %p: ret %d: %s\n",
//<=========================================>
// END OF CODE FIX.
//<=========================================>
				u - wsi->action_start,
					u - wsi->latency_start,
						      (void *)wsi, ret, action);
		wsi->action_start = 0;
	} else
//<=========================================>
// START OF CODE FIX (use %lluus instead of %luus).
//<=========================================>
		sprintf(buf, "lat %lluus: %p: ret %d: %s\n",
//<=========================================>
// END OF CODE FIX.
//<=========================================>
			      u - wsi->latency_start, (void *)wsi, ret, action);

	if (u - wsi->latency_start > context->worst_latency) {
		context->worst_latency = u - wsi->latency_start;
		strcpy(context->worst_latency_info, buf);
	}
	lwsl_latency("%s", buf);
}
#endif



Issue 2
=====
With the test server, and navigating to the picture leaf.jpg test server using a browser (e.g. http://127.0.0.1:7681/leaf.jpg), the file does not always completely download.

[208821:1540] INFO: HTTP GET request for '/leaf.jpg'
    GET URI = /leaf.jpg
    Host = 127.0.0.1:7681
    Connection = keep-alive
    Accept: = text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
    Accept-Encoding: = gzip,deflate,sdch
    Accept-Language: = en,en-US;q=0.8,en-GB;q=0.6
    Pragma: = no-cache
    Cache-Control: = no-cache
    Cookie: = test=LWS_1399552159_803678_COOKIE
[208821:2030] INFO: ***** d26eb0 new partial sent 0 from 4096 total
[208821:2090] INFO: ***** d26eb0 partial send moved on by 4096 (vs 4096)
[208821:2130] INFO: ***** d26eb0 partial send completed
[208825:0152] INFO: TIMEDOUT WAITING on 3
[208825:0152] DEBUG: close: just_kill_connection

The fix is to modify int lws_issue_raw(struct libwebsocket *wsi, unsigned char *buf, size_t len)

int lws_issue_raw(struct libwebsocket *wsi, unsigned char *buf, size_t len)
{
	struct libwebsocket_context *context = wsi->protocol->owning_server;
	int n;
	size_t real_len = len;
	int m;
	
	if (!len)
		return 0;
	/* just ignore sends after we cleared the truncation buffer */
	if (wsi->state == WSI_STATE_FLUSHING_STORED_SEND_BEFORE_CLOSE &&
						!wsi->truncated_send_len)
		return len;

	if (wsi->truncated_send_len && (buf < wsi->truncated_send_malloc ||
			buf > (wsi->truncated_send_malloc +
				wsi->truncated_send_len +
				wsi->truncated_send_offset))) {
		lwsl_err("****** %x Sending new, pending truncated ...\n", wsi);
		assert(0);
	}

	m = lws_ext_callback_for_each_active(wsi,
			LWS_EXT_CALLBACK_PACKET_TX_DO_SEND, &buf, len);
	if (m < 0)
		return -1;
	if (m) /* handled */ {
		n = m;
		goto handle_truncated_send;
	}
	if (wsi->sock < 0)
		lwsl_warn("** error invalid sock but expected to send\n");

	/*
	 * nope, send it on the socket directly
	 */
	lws_latency_pre(context, wsi);
	n = lws_ssl_capable_write(wsi, buf, len);
	lws_latency(context, wsi, "send lws_issue_raw", n, n == len);

	switch (n) {
	case LWS_SSL_CAPABLE_ERROR:
		return -1;
	case LWS_SSL_CAPABLE_MORE_SERVICE:
		/* nothing got sent, not fatal, retry the whole thing later */
		n = 0;
		break;
	}

handle_truncated_send:
	/*
	 * we were already handling a truncated send?
	 */
	if (wsi->truncated_send_len) {
		lwsl_info("***** %x partial send moved on by %d (vs %d)\n",
							     wsi, n, real_len);
		wsi->truncated_send_offset += n;
		wsi->truncated_send_len -= n;

		if (!wsi->truncated_send_len) {
			lwsl_info("***** %x partial send completed\n", wsi);
			/* done with it, but don't free it */
			n = real_len;
			if (wsi->state == WSI_STATE_FLUSHING_STORED_SEND_BEFORE_CLOSE) {
				lwsl_info("***** %x signalling to close now\n", wsi);
				return -1; /* retry closing now */
			}

//<=========================================>
// START OF CODE FIX.
//<=========================================>
	/*
	 *  - Finished with the truncated send.
	 *  - Free the storage and mark as NULL so a new truncated send can occur.
	 */
		    if (wsi->truncated_send_malloc) {
			    free(wsi->truncated_send_malloc);
			    wsi->truncated_send_malloc = NULL;
		    }
//<=========================================>
// END OF CODE FIX.
//<=========================================>
		}
		/* always callback on writeable */
		libwebsocket_callback_on_writable(
					     wsi->protocol->owning_server, wsi);

//<=========================================>
// OPTIONAL CODE CHANGE (RESET TIMEOUT).
//<=========================================>
        libwebsocket_set_timeout(wsi, PENDING_TIMEOUT_HTTP_CONTENT, 10);
//<=========================================>
// END OF OPTIONAL CODE CHANGE.
//<=========================================>
        return n;
	}

	if (n == real_len)
		/* what we just sent went out cleanly */
		return n;

	if (n && wsi->u.ws.clean_buffer)
		/*
		 * This buffer unaffected by extension rewriting.
		 * It means the user code is expected to deal with
		 * partial sends.  (lws knows the header was already
		 * sent, so on next send will just resume sending
		 * payload)
		 */
		 return n;

	/*
	 * Newly truncated send.  Buffer the remainder (it will get
	 * first priority next time the socket is writable)
	 */
	lwsl_info("***** %x new partial sent %d from %d total\n",
						      wsi, n, real_len);

	/*
	 *  - if we still have a suitable malloc lying around, use it
	 *  - or, if too small, reallocate it
	 *  - or, if no buffer, create it
	 */
	if (!wsi->truncated_send_malloc ||
			real_len - n > wsi->truncated_send_allocation) {
		if (wsi->truncated_send_malloc)
			free(wsi->truncated_send_malloc);

		wsi->truncated_send_allocation = real_len - n;
		wsi->truncated_send_malloc = malloc(real_len - n);
		if (!wsi->truncated_send_malloc) {
			lwsl_err("truncated send: unable to malloc %d\n",
							  real_len - n);
			return -1;
		}
	}
	wsi->truncated_send_offset = 0;
	wsi->truncated_send_len = real_len - n;
	memcpy(wsi->truncated_send_malloc, buf + n, real_len - n);

	/* since something buffered, force it to get another chance to send */
	libwebsocket_callback_on_writable(wsi->protocol->owning_server, wsi);

	return real_len;
}

Then the following gets produced:
[208874:6860] INFO: HTTP GET request for '/leaf.jpg'
    GET URI = /leaf.jpg
    Host = 127.0.0.1:7681
    Connection = keep-alive
    Accept: = text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
    Accept-Encoding: = gzip,deflate,sdch
    Accept-Language: = en,en-US;q=0.8,en-GB;q=0.6
    Pragma: = no-cache
    Cache-Control: = no-cache
    Cookie: = test=LWS_1399552159_803678_COOKIE
[208874:7240] INFO: ***** 1e66eb0 new partial sent 0 from 4096 total
[208874:7250] INFO: ***** 1e66eb0 partial send moved on by 4096 (vs 4096)
[208874:7280] INFO: ***** 1e66eb0 partial send completed
[208874:7390] INFO: ***** 1e66eb0 new partial sent 0 from 4096 total
[208874:7410] INFO: ***** 1e66eb0 partial send moved on by 4096 (vs 4096)
[208874:7440] INFO: ***** 1e66eb0 partial send completed
       :
       :
[208876:6452] INFO: ***** 1e66eb0 new partial sent 0 from 4096 total
[208876:6472] INFO: ***** 1e66eb0 partial send moved on by 4096 (vs 4096)
[208876:6512] INFO: ***** 1e66eb0 partial send completed
[208876:6542] INFO: ***** 1e66eb0 new partial sent 0 from 3534 total
[208876:6572] INFO: ***** 1e66eb0 partial send moved on by 3534 (vs 3534)
[208876:6592] INFO: ***** 1e66eb0 partial send completed
[208876:6622] DEBUG: close: just_kill_connection
[208876:6652] INFO: remove_wsi_socket_from_fds: wsi=01E66EB0, sock=404, fds pos=1
[208876:6752] DEBUG: calling back CLOSED_HTTP


Issue 3
The server can crash if using SSL with non-SSL connections (and vice-versa).

In the function lws_server_socket_service_ssl, there are conditions when new_ssl is NULL. This can lead to a crash if wsi->mode is LWS_CONNMODE_SERVER_LISTENER.

The following code will fix this:

LWS_VISIBLE int
lws_server_socket_service_ssl(struct libwebsocket_context *context,
		struct libwebsocket **pwsi, struct libwebsocket *new_wsi,
			int accept_fd, struct libwebsocket_pollfd *pollfd)
{
	int n, m;
	struct libwebsocket *wsi = *pwsi;
#ifndef USE_CYASSL
	BIO *bio;
#endif

	if (!LWS_SSL_ENABLED(context))
		return 0;

	switch (wsi->mode) {
	case LWS_CONNMODE_SERVER_LISTENER:
//<=========================================>
// START OF CODE FIX.
//<=========================================>
	/*
	 * - Validate that new_wsi is not NULL.
	 */
       if (new_wsi == NULL)
       {
       break;
       }
//<=========================================>
// END OF CODE FIX.
//<=========================================>
		new_wsi->ssl = SSL_new(context->ssl_ctx);
		if (new_wsi->ssl == NULL) {
			lwsl_err("SSL_new failed: %s\n",
			    ERR_error_string(SSL_get_error(
			    new_wsi->ssl, 0), NULL));
			    libwebsockets_decode_ssl_error();
			free(new_wsi);
			compatible_close(accept_fd);
			break;
		}

		SSL_set_ex_data(new_wsi->ssl,
			openssl_websocket_private_data_index, context);

		SSL_set_fd(new_wsi->ssl, accept_fd);

#ifdef USE_CYASSL
		CyaSSL_set_using_nonblock(new_wsi->ssl, 1);
#else
		SSL_set_mode(new_wsi->ssl, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
		bio = SSL_get_rbio(new_wsi->ssl);
		if (bio)
			BIO_set_nbio(bio, 1); /* nonblocking */
		else
			lwsl_notice("NULL rbio\n");
		bio = SSL_get_wbio(new_wsi->ssl);
		if (bio)
			BIO_set_nbio(bio, 1); /* nonblocking */
		else
			lwsl_notice("NULL rbio\n");
#endif

		/*
		 * we are not accepted yet, but we need to enter ourselves
		 * as a live connection.  That way we can retry when more
		 * pieces come if we're not sorted yet
		 */

		*pwsi = new_wsi;
		wsi = *pwsi;
		wsi->mode = LWS_CONNMODE_SSL_ACK_PENDING;
		insert_wsi_socket_into_fds(context, wsi);

		libwebsocket_set_timeout(wsi, PENDING_TIMEOUT_SSL_ACCEPT,
							AWAITING_TIMEOUT);

		lwsl_info("inserted SSL accept into fds, trying SSL_accept\n");

		/* fallthru */

	case LWS_CONNMODE_SSL_ACK_PENDING:

		if (lws_change_pollfd(wsi, LWS_POLLOUT, 0))
			goto fail;

		lws_libev_io(context, wsi, LWS_EV_STOP | LWS_EV_WRITE);

		lws_latency_pre(context, wsi);

		n = recv(wsi->sock, context->service_buffer,
			sizeof(context->service_buffer), MSG_PEEK);

		/*
		 * optionally allow non-SSL connect on SSL listening socket
		 * This is disabled by default, if enabled it goes around any
		 * SSL-level access control (eg, client-side certs) so leave
		 * it disabled unless you know it's not a problem for you
		 */

		if (context->allow_non_ssl_on_ssl_port && n >= 1 &&
					context->service_buffer[0] >= ' ') {
			/*
			 * TLS content-type for Handshake is 0x16
			 * TLS content-type for ChangeCipherSpec Record is 0x14
			 *
			 * A non-ssl session will start with the HTTP method in
			 * ASCII.  If we see it's not a legit SSL handshake
			 * kill the SSL for this connection and try to handle
			 * as a HTTP connection upgrade directly.
			 */
			wsi->use_ssl = 0;
			SSL_shutdown(wsi->ssl);
			SSL_free(wsi->ssl);
			wsi->ssl = NULL;
			goto accepted;
		}

		/* normal SSL connection processing path */

		n = SSL_accept(wsi->ssl);
		lws_latency(context, wsi,
			"SSL_accept LWS_CONNMODE_SSL_ACK_PENDING\n", n, n == 1);

		if (n == 1)
			goto accepted;

		m = SSL_get_error(wsi->ssl, n);
		lwsl_debug("SSL_accept failed %d / %s\n",
						  m, ERR_error_string(m, NULL));

		if (m == SSL_ERROR_WANT_READ) {
			if (lws_change_pollfd(wsi, 0, LWS_POLLIN))
				goto fail;

			lws_libev_io(context, wsi, LWS_EV_START | LWS_EV_READ);

			lwsl_info("SSL_ERROR_WANT_READ\n");
			break;
		}
		if (m == SSL_ERROR_WANT_WRITE) {
			if (lws_change_pollfd(wsi, 0, LWS_POLLOUT))
				goto fail;

			lws_libev_io(context, wsi, LWS_EV_START | LWS_EV_WRITE);
			break;
		}
		lwsl_debug("SSL_accept failed skt %u: %s\n",
					 pollfd->fd, ERR_error_string(m, NULL));
		libwebsocket_close_and_free_session(context, wsi,
						     LWS_CLOSE_STATUS_NOSTATUS);
		break;

accepted:
		/* OK, we are accepted... give him some time to negotiate */
		libwebsocket_set_timeout(wsi,
			PENDING_TIMEOUT_ESTABLISH_WITH_SERVER,
							AWAITING_TIMEOUT);

		wsi->mode = LWS_CONNMODE_HTTP_SERVING;

		lwsl_debug("accepted new SSL conn\n");
		break;
	}

	return 0;
	
fail:
	return 1;
}


Cheers
Mark



More information about the Libwebsockets mailing list