Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor to use standard SocketServer RequestHandler design (new pull) #111

Merged
merged 47 commits into from
Dec 19, 2013

Conversation

astrand
Copy link

@astrand astrand commented Nov 28, 2013

The original pull request #72 was closed by mistake; this one is a replacement for that one, since it seems impossible to reopen a pull request.

Also see pull #110; my suggestion is to merge that one first.

Peter Åstrand (astrand) added 30 commits March 14, 2013 15:23
Rename self.client to self.request, since this is what standard
SocketServer request handlers are using.
Move around functions and methods, so that connection-related and
server-related stuff are close together.

This patch just moves things around - it does not change anything at
all. This can be verified with:

git diff websocket.py | grep ^- | cut -c 2- | sort > removed
git diff websocket.py | grep ^+ | cut -c 2- | sort > added
diff -u removed added
* Move traffic_legend.

* Since websocket.WebSocketServer.socket is static, don't call it with
  self.socket.
refactoring. Basically, we are dividing WebSocketServer into two
classes: One request handler following the SocketServer Requesthandler
API, and one optional server engine. The standard Python SocketServer
engine can also be used.

websocketproxy.py has been updated to match the API change. I've also
added a new option --libserver in order to use the Python built in
server instead.

I've done a lot of testing with the new code. This includes: verbose,
daemon, run-once, timeout, idle-timeout, ssl, web, libserver. I've
tested both Python 2 and 3. I've also tested websocket.py in another
external service.

Code details follows:

* The new request handler class is called WebSocketRequestHandler,
  inheriting SimpleHTTPRequestHandler.

* The service engine is called WebSocketServer, just like before.

* do_websocket_handshake: Using send_header() etc, instead of manually
  sending HTTP response.

* A new method called handle_websocket() upgrades the connection to
  WebSocket, if requested. Otherwise, it returns False. A typical
  application use is:

    def do_GET(self):
        if not self.handle_websocket():
	   # handle normal requests

* new_client has been renamed to new_websocket_client, in order to
  have a better name in the SocketServer/HTTPServer request handler
  hierarchy.

* Note that in the request handler, configuration variables must be
  provided by the "server" object, ie self.server.target_host.
TypeError: exceptions must be old-style classes or derived from
BaseException, not str

Thus, we are not allowed to raise a string. Raise Exception instead.
Also, call the server instance "server", not "httpd", even when using
LibProxyServer.
>commit b9e1295
>    Prepare for solving novnc#71:
>
>    Rename self.client to self.request, since this is what standard
>    SocketServer request handlers are using.
websocket.py:

* With echo.py, which doesn't need any server configuration, we can
  just switch over our application class to inherit from
  WebSocketRequestHandler instead of WebSocketServer. Also, need to
  use the new method name new_websocket_client.

* With load.py, since we have the "delay" configuration, we need both
  a server class and a request handler.

Note that for both tests, I've removed the raising of
self.EClose(closed). This is incorrect. First of all, it's described
as "An exception before the WebSocket connection was established", so
not suitable for our case. Second, it will cause send_close to be
called twice. Finally, self.EClose is now in the WebSocketServer
class, and not a member of the request handler.
requesthandler. Removed comments about above/below which does not make
sense any longer. No functional changes.
from WebSocketProxy without having to specify handler class.
* commit '2d078e8cd83735dad7d98fadcece652d93e2a037':
  correctly include include directory in egg.
* commit '33e9a21ce4e38d52f43fe775fb154fc71c09c827':
  Add gimite/web-socket-js submodule for DFSG compliance.
* commit '477947ba96a00032ae35ac55fd02a4a5f485497e':
  Remove wsproxy references. Sync launch.sh from noVNC.
* commit 'b7f255ce0b21dc42189205b1f0e46b4f1d9854f9':
  Clarify messages when optional modules are not found.
* commit '6d27b5d321978586ea1601f757ead73dfba03da7':
  Add 2 arguments to websockify.WSRequestHandler

As of now, only implemented the first command; see #83 for details.
* commit 'd3ba23fa64d79eeb602ff1015ec31014fd8e9b35':
  Update to c0855c6cae of web-socket-js
* commit '36fcd5784fa0825eedbf31d91bc42c970605ddb4':
  Add package file for websockify.js
* commit '36cb8f4676c7b5ff34bd22ad729e00e77efb6f00':
  Move javascript websockify files to other/js
* commit '264f8fdd7f12bd5b9f6813fb8de81c55b6328d9b':
  Update to version 0.5.1
* commit 'ab389d4e7114d7ddbfd085591d336ea5cc06c00d':
  Collect zombie child processes within server loop
* commit 'f30ad05c70ab2a43c9078e2f79da40f1dc0c60ec':
  Fix novnc#97: rebind.so not found when installed
* commit 'edde5cd0ff6059bddae10c9b9faf0b3e8f388a9e':
  Fixed wiki reference
Peter Åstrand (astrand) added 13 commits November 27, 2013 13:41
* commit 'a7fa97f0e14926cc4433483fcb7581e0b3782140':
  WebSocketProxy: support non path target_cfg
* commit '4459824cc8196ad78fe9258b6c560ad46fe4cd52':
  websocket: do not exit at the middle of process
  websocket: restore signals after processing
  websocket: support SIGTERM as exit signal
* commit '477dce6cf86d61b20a394f3cbf3170e60d199658':
  websocket: use python logging module
  websocket: fix exception statement introduced by comment 903e3f0

Adapted to new standard SocketServer RequestHandler design. For
example, this means that self.i_am_client is not needed.
* commit 'a61ae52610642ae58e914dda705df8bb9c8213ec':
  fixed 1.8 compatibility bug for OpenSSL::SSL::SSLSocket#read_nonblock vs #readpartial tested in 1.8 and 2.0
  adding SSL support and Ruby1.9 support
* commit '0e5c3ecfda3b1506b41412052db75d84df2b4ae7':
  Handle SIGCHLD properly for multiprocessing
* commit '32b0567343aee7753b2b6be1bc1ee9a69657ba26':
  Fix syntax errors in other/websockify.rb
* commit 'b4e0b534d5d04d57265045b4baf49dd81555064b':
  Fix crash when an import is missing
* commit '13c99bcf053f7f3af8ba84c0d963a9591e020f49':
  Fix search path construction in tests.
* commit 'c3acdc2e38f871e28ffda1847b4338c4b02296b8':
  Adds optional TCP_KEEPALIVE to WebSocketServer
* commit 'a47be21f9fa69ddf8d888ff9e3c75cdfc9e31c00':
  Added unit tests for websocket and websocketproxy
* commit 'a04edfe80f54b44df5a3579f71710560c6b7b4fc':
  Added temp dir for unit test data and cleanup
* commit '34a1b68d79a13c03aa63b5c4194796341c9383fe':
  Clarify ssl module build for old python versions.
in request handler class.
astrand pushed a commit that referenced this pull request Dec 16, 2013
Rename self.client and new_client - prepare for #72 / #111
@astrand
Copy link
Author

astrand commented Dec 17, 2013

I've committed the simple code moves and renames now, and tweaked this pull request so that it is minimal. If there are no objections, I will merge this pull request later this week.

astrand pushed a commit that referenced this pull request Dec 19, 2013
Move around functions and methods, so that connection-related and
server-related stuff are close together.

This patch just moves things around - it does not change anything at
all. This can be verified with:

git diff | grep ^- | cut -c 2- | sort > removed
git diff | grep ^+ | cut -c 2- | sort > added
diff -u removed added
astrand pushed a commit that referenced this pull request Dec 19, 2013
Refactor to use standard SocketServer RequestHandler design.
@astrand astrand merged commit 38db12c into novnc:master Dec 19, 2013
@astrand astrand mentioned this pull request Feb 12, 2014
DirectXMan12 added a commit that referenced this pull request Feb 18, 2014
*** NOTE ***

This version of websockify will break existing code which sub-classes
`WebsocketProxy` -- see pull requests #110 and #111
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant