Getting more comments, packaging, and ASD correct

buffer.lisp

@@ -1,10 +1,10 @@
-(in-package :http2)
+(in-package :cl-http2-protocol)
 
 ; Port notes: We cannot subclass STRING in CL to accomplish what we
 ; want. We have less utility methods out-of-the-box, so we'll define a
-; bunch of methods similar to the Ruby String ones.
-
-; We start with some vector primitives, and then define the buffer class
+; bunch of methods similar to the Ruby String ones. We start with some
+; vector primitives, and then define the BUFFER class, which will have
+; a slot carrying the actual binary data.
 
 (defun vector-concat (src dest)
   "Modifies vector DEST by concatenating the elements of vector SRC to the end, and returns DEST.

cl-http2.asd → cl-http2-protocol.asd

@@ -1,11 +1,11 @@
 (in-package :cl-user)
 
-(defpackage :cl-http2-asd
+(defpackage :cl-http2-protocol-asd
   (:use :cl :asdf))
 
-(in-package :cl-http2-asd)
+(in-package :cl-http2-protocol-asd)
 
-(defsystem :cl-http2
+(defsystem :cl-http2-protocol
   :description "HTTP/2.0 draft-06 implementation with client/server examples.
 Originally a port of Ruby code by Ilya Grigorik, see: https://github.com/igrigorik/http-2
 For HTTP/2.0 draft-06, see: http://tools.ietf.org/html/draft-ietf-httpbis-http2-06
@@ -19,7 +19,7 @@ For other implementations, see: https://github.com/http2/http2-spec/wiki/Impleme
 	       :usocket
 	       :cl+ssl)
   :components ((:file "packages")
-	       (:file "util" :depends-on ("packages" (:require :alexandria)))
+	       (:file "util" :depends-on ("packages" :alexandria))
 	       (:file "buffer" :depends-on ("util"))
 	       (:file "flow-buffer" :depends-on ("util" "buffer"))
 	       (:file "emitter" :depends-on ("util"))
@@ -28,7 +28,8 @@ For other implementations, see: https://github.com/http2/http2-spec/wiki/Impleme
 	       (:file "framer" :depends-on ("util" "buffer"))
 	       (:file "compressor" :depends-on ("util" "error" "buffer"))
 	       (:file "stream" :depends-on ("util" "flow-buffer" "emitter" "error" "buffer"))
-	       (:file "client" :depends-on ("util" "connection" "compressor" "stream" (:require :puri)))
+	       (:file "client" :depends-on ("util" "connection" "compressor" "stream"))
 	       (:file "server" :depends-on ("util" "connection" "compressor" "stream"))
-	       (:file "ssl" :depends-on ("util" (:require :cl+ssl)))
-	       (:file "example" :depends-on ("util" "ssl" "client" "server"))))
+	       (:file "ssl" :depends-on ("util" :cl+ssl))
+	       (:file "net" :depends-on ("util" "ssl" :cl+ssl :usocket))
+	       (:file "example" :depends-on ("util" "ssl" "net" "client" "server" :puri))))

client.lisp

@@ -1,4 +1,11 @@
-(in-package :http2)
+(in-package :cl-http2-protocol)
+
+; HTTP 2.0 client connection class that implements appropriate header
+; compression / decompression algorithms and stream management logic.
+;
+; Your code is responsible for driving the client object, which in turn
+; performs all of the necessary HTTP 2.0 encoding / decoding, state
+; management, and the rest. See README.md for an example.
 
 (defclass client (connection)
   ((stream-id :initform 1)
@@ -10,6 +17,7 @@
   (:documentation "HTTP 2.0 client object"))
 
 (defmethod send :before ((client client) frame)
+  "Send an outgoing frame. Connection and stream flow control is managed by CONNECTION class."
   (with-slots (state stream-limit window-limit) client
     (when (eq state :connection-header)
       (emit client :frame *connection-header*)

compressor.lisp

@@ -1,4 +1,9 @@
-(in-package :http2)
+(in-package :cl-http2-protocol)
+
+; Implementation of header compression for HTTP 2.0 (HPACK) format adapted
+; to efficiently represent HTTP headers in the context of HTTP 2.0.
+;
+; - http://tools.ietf.org/html/draft-ietf-httpbis-header-compression
 
 (defparameter *req-defaults*
   '((":scheme"             . "http")
@@ -63,19 +68,27 @@
     ("retry-after"                 . "")
     ("strict-transport-security"   . "")
     ("transfer-encoding"           . "")
-    ("www-authenticate"            . "")))
+    ("www-authenticate"            . ""))
+  "Default response working set as defined by the spec.")
+
+; The set of components used to encode or decode a header set form an
+; encoding context: an encoding context contains a header table and a
+; reference set - there is one encoding context for each direction.
 
 (defclass encoding-context (error-include)
   ((type :initarg :type)
    (table :reader table)
    (limit :initarg :limit :initform 4096)
-   (refset :reader refset :initform (make-array 128 :element-type t :adjustable t :fill-pointer 0))))
+   (refset :reader refset :initform (make-array 128 :element-type t :adjustable t :fill-pointer 0)))
+  (:documentation "Encoding context: a header table and reference set for one direction"))
 
 (defmethod initialize-instance :after ((encoding-context encoding-context) &key)
+  "Initializes compression context with appropriate client/server
+defaults and maximum size of the header table."
   (with-slots (table type) encoding-context
     (setf table (if (eq type :request)
-		    (copy-tree *req-defaults*)
-		    (copy-tree *resp-defaults*)))))
+		    (copy-list *req-defaults*)
+		    (copy-list *resp-defaults*)))))
 
 (defmethod process ((encoding-context encoding-context) cmd)
   "Performs differential coding based on provided command type.
@@ -222,13 +235,16 @@ entry of the header table is always associated to the index 0."
   (with-slots (type) encoding-context
     (< idx (length (if (eq type :request) *req-defaults* *resp-defaults*)))))
 
-; Header representation as defined by the spec.
 (defparameter *headrep*
   '(:indexed      (:prefix 7 :pattern #x80)
     :noindex      (:prefix 5 :pattern #x60)
     :incremental  (:prefix 5 :pattern #x40)
-    :substitution (:prefix 6 :pattern #x00)))
+    :substitution (:prefix 6 :pattern #x00))
+  "Header representation as defined by the spec.")
 
+; Responsible for encoding header key-value pairs using HPACK algorithm.
+; Compressor must be initialized with appropriate starting context based
+; on local role: client or server.
 (defclass compressor ()
   ((cc-type :initarg :type)
    (cc)))

connection.lisp

@@ -1,4 +1,4 @@
-(in-package :http2)
+(in-package :cl-http2-protocol)
 
 (defparameter *default-flow-window* 65535
   "Default connection and stream flow control window (64KB)")
@@ -11,26 +11,35 @@
     (buffer-simple (concatenate 'string "PRI * HTTP/2.0" #1='(#\Return #\Linefeed) #1# "SM" #1# #1#))
     "Default connection \"fast-fail\" preamble string as defined by the spec"))
 
+; Connection encapsulates all of the connection, stream, flow-control,
+; error management, and other processing logic required for a well-behaved
+; HTTP 2.0 endpoint.
+;
+; Note that this class should not be used directly. Instead, you want to
+; use either Client or Server class to drive the HTTP 2.0 exchange.
+;
 (defclass connection (flowbuffer-include emitter-include error-include)
-  ((state :reader conn-state)
+  ((state :reader conn-state :type (member :new :connection-header :connected :closed))
    (error :reader conn-error :initform nil)
-   (window :reader conn-window :initarg :window :initform *default-flow-window*)
-   (stream-limit :reader conn-stream-limit :initarg :streams :initform 100)
-   (active-stream-count :reader conn-active-stream-count :initform 0)
-   (streams :initform (make-hash-table))
-   (framer :initform (make-instance 'framer))
-   (window-limit :initarg :window-limit)
-   (recv-buffer :initform (make-instance 'buffer))
+   (window :reader conn-window :initarg :window :initform *default-flow-window* :type (or integer float))
+   (stream-limit :reader conn-stream-limit :initarg :streams :initform 100 :type integer)
+   (active-stream-count :reader conn-active-stream-count :initform 0 :type integer)
+   (streams :initform (make-hash-table) :type hash-table)
+   (framer :initform (make-instance 'framer) :type framer)
+   (window-limit :initarg :window-limit :type (or integer float))
+   (recv-buffer :initform (make-instance 'buffer) :type buffer)
    (send-buffer :initform nil)
-   (continuation :initform nil)
-   (stream-id :initform nil)))
+   (continuation :initform nil :type list)
+   (stream-id :initform nil))
+  (:documentation "Encapsulate connection, stream, flow-control, error management for an endpoint"))
 
 (defmethod initialize-instance :after ((connection connection) &key)
   (setf (slot-value connection 'window-limit) (slot-value connection 'window)))
 
 (defgeneric send (obj frame))
 
 (defmethod new-stream ((connection connection) &optional (priority *default-priority*) (parent nil))
+  "Allocates new stream for current connection."
   (with-slots (state active-stream-count stream-limit stream-id) connection
     (cond ((eq state :closed)                   (raise 'http2-connection-closed))
 	  ((= active-stream-count stream-limit) (raise 'http2-stream-limit-exceeded))
@@ -39,10 +48,18 @@
 	       (incf stream-id 2))))))
 
 (defmethod ping ((connection connection) payload blk)
+  "Sends PING frame to the peer."
   (send connection (list :type :ping :stream 0 :payload payload))
   (if blk (once connection :pong blk)))
 
+; Endpoints MAY append opaque data to the payload of any GOAWAY frame.
+; Additional debug data is intended for diagnostic purposes only and
+; carries no semantic value. Debug data MUST NOT be persistently stored,
+; since it could contain sensitive information.
+;
 (defmethod goaway ((connection connection) &optional (error :no-error) (payload nil))
+  "Sends a GOAWAY frame indicating that the peer should stop creating
+new streams for current connection."
   (with-slots (streams) connection
     (let ((last-stream (or (loop for k being the hash-keys of streams maximize k) 0)))
       (send connection (list :type :goaway :last-stream last-stream
@@ -51,19 +68,14 @@
 (defmethod settings ((connection connection) &optional
 		     (stream-limit (slot-value connection 'stream-limit))
 		     (window-limit (slot-value connection 'window-limit)))
+  "Sends a connection SETTINGS frame to the peer. Setting window size
+to +INFINITY disables flow control."
   (with-slots (window) connection
     (let ((payload (list :settings-max-concurrent-streams stream-limit)))
       (if (eql window +infinity)
-	  (appendf payload (list :settings-flow-control-options 1))
-	  (appendf payload (list :settings-initial-window-size window-limit))
-	  ; semantically the following pair makes a bit more sense but it
-	  ; results in the order of the settings (and thus the bytes)
-	  ; coming out in reverse order to the Ruby code which made testing
-	  ; for correctness more difficult
-	  ;   (setf (getf payload :settings-flow-control-options) 1)
-	  ;   (setf (getf payload :settings-initial-window-size) window-limit)
-	  )
-      (send connection (list :type :settings :stream 0 :payload payload)))))
+	  (setf (getf payload :settings-flow-control-options) 1)
+	  (setf (getf payload :settings-initial-window-size) window-limit))
+      (send connection (list :type :settings :stream 0 :payload (reverse-plist payload))))))
 
 ; these have to appear here to compile (receive connection ...) properly
 (defgeneric receive (obj data))
@@ -370,6 +382,4 @@ aborted, and once sent, raise a local exception."
 		       (simple-condition-format-control msg)
 		       (simple-condition-format-arguments msg))))
 
-    ; in the Ruby code introspection is used to raise an Error subclass
-    ; similarly we convert the keyword symbol to a defined condition
     (raise (find-symbol (symbol-name type)) msg)))

emitter.lisp

@@ -1,20 +1,23 @@
-(in-package :http2)
+(in-package :cl-http2-protocol)
 
 (defclass emitter-include ()
-  ((listeners :accessor listeners :initarg :listeners :initform (make-hash-table))))
+  ((listeners :accessor listeners :initarg :listeners :initform (make-hash-table)))
+  (:documentation "Basic event emitter with support for persistent and one-time event callbacks."))
 
 (defmethod add-listener ((emitter emitter-include) event block)
-  (unless block
-    (error "must provide callback"))
+  "Subscribe to all future events for specified type."
+  (assert (and block (functionp block)) (block) "Must provide callback")
   (push block (gethash (to-sym event) (listeners emitter) nil)))
 
 (defalias on add-listener)
 
 (defmethod once ((emitter emitter-include) event block)
+  "Subscribe to next event (at most once) for specified type."
   (add-listener emitter event
 		(lambda (&rest args) (apply block args) :delete)))
 
 (defmethod emit ((emitter emitter-include) event &rest args)
+  "Emit event with provided arguments."
   (deletef-if (gethash event (listeners emitter))
 	      (lambda (cb) (eq (apply cb args) :delete))))
 

error.lisp

@@ -1,6 +1,7 @@
-(in-package :http2)
+(in-package :cl-http2-protocol)
 
-(defclass error-include () ())
+(defclass error-include () ()
+  (:documentation "Stream, connection, and compressor exceptions."))
 
 (define-condition http2-error (simple-error) ())
 
@@ -33,6 +34,8 @@ client and server contexts are out of sync."))
   (:documentation "Raised on invalid stream processing: invalid frame type received or
 sent, or invalid command issued."))
 
+; recoverable errors
+
 (define-condition http2-stream-closed (http2-error) ()
   (:documentation "Raised if stream has been closed and new frames cannot be sent."))
 

example.lisp

@@ -1,4 +1,6 @@
-(in-package :http2)
+(in-package :cl-http2-protocol)
+
+; see also net.lisp which is part of the example and not strictly necessary
 
 (defun example-client (uri &key (net nil net-arg-p) (secure nil secure-arg-p))
   (assert (or (not net-arg-p) (not secure-arg-p)) (net secure) "Provide either :NET or :SECURE")

flow-buffer.lisp

@@ -1,17 +1,27 @@
-(in-package :http2)
+(in-package :cl-http2-protocol)
 
-(defparameter *max-frame-size* (1- (expt 2 14)))
+(defparameter *max-frame-size* (1- (expt 2 14))
+  "Maximum size of a DATA payload (16383 bytes, ~16K).")
 
 (defclass flowbuffer-include ()
-  ((send-buffer :accessor send-buffer :initarg :send-buffer)))
+  ((send-buffer :accessor send-buffer :initarg :send-buffer))
+  (:documentation "Implementation of stream and connection DATA flow control: frames may
+be split and / or may be buffered based on current flow control window."))
 
 (defmethod buffered-amount ((obj flowbuffer-include))
+  "Amount of buffered data. Only DATA payloads are subject to flow stream
+and connection flow control."
   (reduce #'+ (mapcar (lambda (f) (getf f :length)) (send-buffer obj))))
 
 (defgeneric encode (obj frame))
 (defgeneric emit (obj obj &rest args))
 
 (defmethod send-data ((obj flowbuffer-include) &optional frame encode)
+  "Buffers outgoing DATA frames and applies flow control logic to split
+and emit DATA frames based on current flow control window. If the
+window is large enough, the data is sent immediately. Otherwise, the
+data is buffered until the flow control window is updated.
+Buffered DATA frames are emitted in FIFO order."
   (with-slots (send-buffer window) obj
     (when frame
       (push frame send-buffer))

framer.lisp

@@ -1,6 +1,4 @@
-(in-package :http2)
-
-(declaim (optimize (debug 3) (safety 3) (speed 0) (space 0) (compilation-speed 0)))
+(in-package :cl-http2-protocol)
 
 (defparameter *max-payload-size* (1- (expt 2 16))
   "Maximum frame size (65535 bytes)")
@@ -20,7 +18,8 @@
 			      :ping          #x6
 			      :goaway        #x7
 			      :window-update #x9
-			      :continuation  #xA))
+			      :continuation  #xA)
+  "HTTP 2.0 frame type mapping as defined by the spec")
 
 (defparameter *frame-flags* '(:data          (:end-stream 0 :reserved 1)
 			      :headers       (:end-stream 0 :reserved 1
@@ -32,11 +31,13 @@
 			      :ping          (:pong 0)
 			      :goaway        ()
 			      :window-update ()
-			      :continuation  (:end-stream 0 :end-headers 1)))
+			      :continuation  (:end-stream 0 :end-headers 1))
+  "Per frame flags as defined by the spec")
 
 (defparameter *defined-settings* '(:settings-max-concurrent-streams 4
 				   :settings-initial-window-size    7
-				   :settings-flow-control-options   10))
+				   :settings-flow-control-options   10)
+  "Default settings as defined by the spec")
 
 (defparameter *defined-errors* '(:no-error           0
 				 :protocol-error     1
@@ -46,7 +47,8 @@
 				 :frame-too-large    6
 				 :refused-stream     7
 				 :cancel             8
-				 :compression-error  9))
+				 :compression-error  9)
+  "Default error types as defined by the spec")
 
 (defparameter *rbit*  #x7FFFFFFF)
 (defparameter *rbyte* #x0FFFFFFF)
@@ -55,7 +57,8 @@
   (defparameter *headerpack* "nCCN")
   (defparameter *uint32* "N"))
 
-(defclass framer () ())
+(defclass framer () ()
+  (:documentation "Performs encoding, decoding, and validation of binary HTTP 2.0 frames."))
 
 (defmethod common-header ((framer framer) frame)
   "Generates common 8-byte frame header.
@@ -89,6 +92,7 @@
     (pack *headerpack* (nreverse header))))
 
 (defmethod read-common-header ((framer framer) (buf buffer))
+  "Decodes common 8-byte header."
   (let (frame)
     (destructuring-bind (flength type flags stream)
 	(unpack *headerpack* (buffer-data (buffer-slice buf 0 8)))