From 6c73d6a24590e50223e87a9bda2267120c85edff Mon Sep 17 00:00:00 2001
From: Slava Pestov <slava@factorcode.org>
Date: Thu, 13 Mar 2008 22:14:32 -0500
Subject: [PATCH] Improving session management and action link generation

---
 extra/http/client/client.factor               |  3 +-
 extra/http/http.factor                        | 82 ++++++++++++-------
 .../http/server/actions/actions-tests.factor  |  1 +
 extra/http/server/actions/actions.factor      | 24 ++----
 extra/http/server/auth/login/login.factor     |  3 +-
 extra/http/server/auth/login/login.fhtml      | 13 ++-
 extra/http/server/auth/login/recover-1.fhtml  |  5 +-
 extra/http/server/auth/login/recover-3.fhtml  |  7 +-
 extra/http/server/auth/login/recover-4.fhtml  |  6 +-
 extra/http/server/auth/login/register.fhtml   |  4 +-
 extra/http/server/auth/providers/db/db.factor |  4 +-
 extra/http/server/server.factor               | 34 ++++++--
 .../server/sessions/sessions-tests.factor     |  6 +-
 extra/http/server/sessions/sessions.factor    | 34 ++++----
 14 files changed, 136 insertions(+), 90 deletions(-)

diff --git a/extra/http/client/client.factor b/extra/http/client/client.factor
index ee0d5f7f3b..6d875ef560 100755
--- a/extra/http/client/client.factor
+++ b/extra/http/client/client.factor
@@ -95,5 +95,4 @@ PRIVATE>
     swap >>post-data-type ;
 
 : http-post ( content-type content url -- response string )
-    #! The content is URL encoded for you.
-    >r url-encode r> <post-request> http-request contents ;
+    <post-request> http-request contents ;
diff --git a/extra/http/http.factor b/extra/http/http.factor
index c72a631d16..4dd433f85d 100755
--- a/extra/http/http.factor
+++ b/extra/http/http.factor
@@ -4,7 +4,8 @@ USING: fry hashtables io io.streams.string kernel math
 namespaces math.parser assocs sequences strings splitting ascii
 io.encodings.utf8 io.encodings.string namespaces unicode.case
 combinators vectors sorting new-slots accessors calendar
-calendar.format quotations arrays ;
+calendar.format quotations arrays combinators.cleave
+combinators.lib byte-arrays ;
 IN: http
 
 : http-port 80 ; inline
@@ -12,18 +13,21 @@ IN: http
 : url-quotable? ( ch -- ? )
     #! In a URL, can this character be used without
     #! URL-encoding?
-    dup letter?
-    over LETTER? or
-    over digit? or
-    swap "/_-." member? or ; foldable
+    {
+        [ dup letter? ]
+        [ dup LETTER? ]
+        [ dup digit? ]
+        [ dup "/_-.:" member? ]
+    } || nip ; foldable
 
 : push-utf8 ( ch -- )
-    1string utf8 encode [ CHAR: % , >hex 2 CHAR: 0 pad-left % ] each ;
+    1string utf8 encode
+    [ CHAR: % , >hex 2 CHAR: 0 pad-left % ] each ;
 
 : url-encode ( str -- str )
-    [ [
-        dup url-quotable? [ , ] [ push-utf8 ] if
-    ] each ] "" make ;
+    [
+        [ dup url-quotable? [ , ] [ push-utf8 ] if ] each
+    ] "" make ;
 
 : url-decode-hex ( index str -- )
     2dup length 2 - >= [
@@ -108,7 +112,12 @@ IN: http
     ] when ;
 
 : assoc>query ( hash -- str )
-    [ [ url-encode ] 2apply "=" swap 3append ] { } assoc>map
+    [
+        [ url-encode ]
+        [ dup number? [ number>string ] when url-encode ]
+        bi*
+        "=" swap 3append
+    ] { } assoc>map
     "&" join ;
 
 TUPLE: cookie name value path domain expires http-only ;
@@ -169,10 +178,10 @@ cookies ;
 
 : <request>
     request construct-empty
-    "1.1" >>version
-    http-port >>port
-    H{ } clone >>query
-    V{ } clone >>cookies ;
+        "1.1" >>version
+        http-port >>port
+        H{ } clone >>query
+        V{ } clone >>cookies ;
 
 : query-param ( request key -- value )
     swap query>> at ;
@@ -245,6 +254,10 @@ SYMBOL: max-post-request
 : extract-post-data-type ( request -- request )
     dup "content-type" header >>post-data-type ;
 
+: parse-post-data ( request -- request )
+    dup post-data-type>> "application/x-www-form-urlencoded" =
+    [ dup post-data>> query>assoc >>post-data ] when ;
+
 : extract-cookies ( request -- request )
     dup "cookie" header [ parse-cookies >>cookies ] when* ;
 
@@ -257,24 +270,31 @@ SYMBOL: max-post-request
     read-post-data
     extract-host
     extract-post-data-type
+    parse-post-data
     extract-cookies ;
 
 : write-method ( request -- request )
     dup method>> write bl ;
 
-: write-url ( request -- request )
-    dup path>> url-encode write
-    dup query>> dup assoc-empty? [ drop ] [
-        "?" write
-        assoc>query write
-    ] if ;
+: (link>string) ( url query -- url' )
+    [ url-encode ] [ assoc>query ] bi*
+    dup empty? [ drop ] [ "?" swap 3append ] if ;
+
+: write-url ( request -- )
+    [ path>> ] [ query>> ] bi (link>string) write ;
 
 : write-request-url ( request -- request )
-    write-url bl ;
+    dup write-url bl ;
 
 : write-version ( request -- request )
     "HTTP/" write dup request-version write crlf ;
 
+: unparse-post-data ( request -- request )
+    dup post-data>> dup sequence? [ drop ] [
+        assoc>query >>post-data
+        "application/x-www-form-urlencoded" >>post-data-type
+    ] if ;
+
 : write-request-header ( request -- request )
     dup header>> >hashtable
     over host>> [ "host" pick set-at ] when*
@@ -287,6 +307,7 @@ SYMBOL: max-post-request
     dup post-data>> [ write ] when* ;
 
 : write-request ( request -- )
+    unparse-post-data
     write-method
     write-request-url
     write-version
@@ -297,15 +318,16 @@ SYMBOL: max-post-request
 
 : request-url ( request -- url )
     [
-        dup host>> [
-            "http://" write
-            dup host>> url-encode write
-            ":" write
-            dup port>> number>string write
-        ] when
-        dup path>> "/" head? [ "/" write ] unless
-        write-url
-        drop
+        [
+            dup host>> [
+                [ "http://" write host>> url-encode write ]
+                [ ":" write port>> number>string write ]
+                bi
+            ] [ drop ] if
+        ]
+        [ path>> "/" head? [ "/" write ] unless ]
+        [ write-url ]
+        tri
     ] with-string-writer ;
 
 : set-header ( request/response value key -- request/response )
diff --git a/extra/http/server/actions/actions-tests.factor b/extra/http/server/actions/actions-tests.factor
index 98a92e083a..45f7ff385d 100755
--- a/extra/http/server/actions/actions-tests.factor
+++ b/extra/http/server/actions/actions-tests.factor
@@ -29,6 +29,7 @@ blah
 STRING: action-request-test-2
 POST http://foo/bar/baz HTTP/1.1
 content-length: 5
+content-type: application/x-www-form-urlencoded
 
 xxx=4
 ;
diff --git a/extra/http/server/actions/actions.factor b/extra/http/server/actions/actions.factor
index bab55eef0c..72c2d2df8e 100755
--- a/extra/http/server/actions/actions.factor
+++ b/extra/http/server/actions/actions.factor
@@ -17,14 +17,6 @@ TUPLE: action init display submit get-params post-params ;
         [ <400> ] >>display
         [ <400> ] >>submit ;
 
-: extract-params ( path -- assoc )
-    +path+ associate
-    request get dup method>> {
-        { "GET" [ query>> ] }
-        { "HEAD" [ query>> ] }
-        { "POST" [ post-data>> query>assoc ] }
-    } case union ;
-
 : with-validator ( string quot -- result error? )
     '[ , @ f ] [
         dup validation-error? [ t ] [ rethrow ] if
@@ -50,12 +42,10 @@ TUPLE: action init display submit get-params post-params ;
     action get display>> call exit-with ;
 
 M: action call-responder ( path action -- response )
-    [ extract-params params set ]
-    [
-        action set
-        request get method>> {
-            { "GET" [ handle-get ] }
-            { "HEAD" [ handle-get ] }
-            { "POST" [ handle-post ] }
-        } case
-    ] bi* ;
+    [ +path+ associate request-params union params set ]
+    [ action set ] bi*
+    request get method>> {
+        { "GET" [ handle-get ] }
+        { "HEAD" [ handle-get ] }
+        { "POST" [ handle-post ] }
+    } case ;
diff --git a/extra/http/server/auth/login/login.factor b/extra/http/server/auth/login/login.factor
index 7d92c727c6..9b2648158d 100755
--- a/extra/http/server/auth/login/login.factor
+++ b/extra/http/server/auth/login/login.factor
@@ -30,7 +30,8 @@ SYMBOL: login-failed?
 
 : successful-login ( user -- response )
     logged-in-user sset
-    post-login-url sget f <permanent-redirect> ;
+    post-login-url sget "" or f <permanent-redirect>
+    f post-login-url sset ;
 
 :: <login-action> ( -- action )
     [let | form [ <login-form> ] |
diff --git a/extra/http/server/auth/login/login.fhtml b/extra/http/server/auth/login/login.fhtml
index 8e879420a9..07201719e5 100755
--- a/extra/http/server/auth/login/login.fhtml
+++ b/extra/http/server/auth/login/login.fhtml
@@ -1,10 +1,13 @@
-<% USING: http.server.auth.login http.server.components kernel
-namespaces ; %>
+<% USING: http.server.auth.login http.server.components http.server
+kernel namespaces ; %>
 <html>
 <body>
 <h1>Login required</h1>
 
 <form method="POST" action="login">
+
+<% hidden-form-field %>
+
 <table>
 
 <tr>
@@ -30,10 +33,12 @@ login-failed? get
 
 <p>
 <% allow-registration? [ %>
-    <a href="register">Register</a>
+    <a href="<% "register" f write-link %>">Register</a>
 <% ] when %>
 <% allow-password-recovery? [ %>
-    <a href="recover-password">Recover Password</a>
+    <a href="<% "recover-password" f write-link %>">
+    	Recover Password
+    </a>
 <% ] when %>
 </p>
 
diff --git a/extra/http/server/auth/login/recover-1.fhtml b/extra/http/server/auth/login/recover-1.fhtml
index 3e8448f64b..8ec01f22e9 100755
--- a/extra/http/server/auth/login/recover-1.fhtml
+++ b/extra/http/server/auth/login/recover-1.fhtml
@@ -1,4 +1,4 @@
-<% USING: http.server.components ; %>
+<% USING: http.server.components http.server ; %>
 <html>
 <body>
 <h1>Recover lost password: step 1 of 4</h1>
@@ -6,6 +6,9 @@
 <p>Enter the username and e-mail address you used to register for this site, and you will receive a link for activating a new password.</p>
 
 <form method="POST" action="recover-password">
+
+<% hidden-form-field %>
+
 <table>
 
 <tr>
diff --git a/extra/http/server/auth/login/recover-3.fhtml b/extra/http/server/auth/login/recover-3.fhtml
index b220cc4f75..edd32fffe8 100755
--- a/extra/http/server/auth/login/recover-3.fhtml
+++ b/extra/http/server/auth/login/recover-3.fhtml
@@ -1,4 +1,4 @@
-<% USING: http.server.components http.server.auth.login
+<% USING: http.server.components http.server.auth.login http.server
 namespaces kernel combinators ; %>
 <html>
 <body>
@@ -7,6 +7,9 @@ namespaces kernel combinators ; %>
 <p>Choose a new password for your account.</p>
 
 <form method="POST" action="new-password">
+
+<% hidden-form-field %>
+
 <table>
 
 <% "username" component render-edit %>
@@ -32,7 +35,7 @@ namespaces kernel combinators ; %>
 <p><input type="submit" value="Set password" />
 
 <% password-mismatch? get [
-"passwords do not match" render-error
+    "passwords do not match" render-error
 ] when %>
 
 </p>
diff --git a/extra/http/server/auth/login/recover-4.fhtml b/extra/http/server/auth/login/recover-4.fhtml
index dec7a5404f..239d71d293 100755
--- a/extra/http/server/auth/login/recover-4.fhtml
+++ b/extra/http/server/auth/login/recover-4.fhtml
@@ -1,10 +1,10 @@
-<% USING: http.server.components http.server.auth.login
-namespaces kernel combinators ; %>
+<% USING: http.server ; %>
 <html>
 <body>
 <h1>Recover lost password: step 4 of 4</h1>
 
-<p>Your password has been reset. You may now <a href="login">log in</a>.</p>
+<p>Your password has been reset.
+You may now <a href="<% "login" f write-link %>">log in</a>.</p>
 
 </body>
 </html>
diff --git a/extra/http/server/auth/login/register.fhtml b/extra/http/server/auth/login/register.fhtml
index c7e274e626..99d1547d03 100755
--- a/extra/http/server/auth/login/register.fhtml
+++ b/extra/http/server/auth/login/register.fhtml
@@ -1,10 +1,12 @@
 <% USING: http.server.components http.server.auth.login
-namespaces kernel combinators ; %>
+http.server namespaces kernel combinators ; %>
 <html>
 <body>
 <h1>New user registration</h1>
 
 <form method="POST" action="register">
+<% hidden-form-field %>
+
 <table>
 
 <tr>
diff --git a/extra/http/server/auth/providers/db/db.factor b/extra/http/server/auth/providers/db/db.factor
index e9e79ff82f..c9e1328052 100755
--- a/extra/http/server/auth/providers/db/db.factor
+++ b/extra/http/server/auth/providers/db/db.factor
@@ -14,9 +14,7 @@ user "USERS"
     { "profile" "PROFILE" FACTOR-BLOB }
 } define-persistent
 
-: init-users-table ( -- )
-    [ user drop-table ] ignore-errors
-    user create-table ;
+: init-users-table user ensure-table ;
 
 TUPLE: from-db ;
 
diff --git a/extra/http/server/server.factor b/extra/http/server/server.factor
index b3fafc543f..60bb5d921d 100755
--- a/extra/http/server/server.factor
+++ b/extra/http/server/server.factor
@@ -9,6 +9,13 @@ IN: http.server
 
 GENERIC: call-responder ( path responder -- response )
 
+: request-params ( -- assoc )
+    request get dup method>> {
+        { "GET" [ query>> ] }
+        { "HEAD" [ query>> ] }
+        { "POST" [ post-data>> ] }
+    } case ;
+
 : <content> ( content-type -- response )
     <response>
         200 >>code
@@ -45,19 +52,27 @@ SYMBOL: 404-responder
 
 [ <404> ] <trivial-responder> 404-responder set-global
 
-: url-redirect ( to query -- url )
-    #! Different host.
-    dup assoc-empty? [
-        drop
-    ] [
-        assoc>query "?" swap 3append
-    ] if ;
+SYMBOL: link-hook
+
+: modify-query ( query -- query )
+    link-hook get [ ] or call ;
+
+: link>string ( url query -- url' )
+    modify-query (link>string) ;
+
+: write-link ( url query -- )
+    link>string write ;
+
+SYMBOL: form-hook
+
+: hidden-form-field ( -- )
+    form-hook get [ ] or call ;
 
 : absolute-redirect ( to query -- url )
     #! Same host.
     request get clone
         swap [ >>query ] when*
-        swap >>path
+        swap url-encode >>path
     request-url ;
 
 : replace-last-component ( path with -- path' )
@@ -67,11 +82,12 @@ SYMBOL: 404-responder
     request get clone
     swap [ >>query ] when*
     swap [ '[ , replace-last-component ] change-path ] when*
+    dup query>> modify-query >>query
     request-url ;
 
 : derive-url ( to query -- url )
     {
-        { [ over "http://" head? ] [ url-redirect ] }
+        { [ over "http://" head? ] [ link>string ] }
         { [ over "/" head? ] [ absolute-redirect ] }
         { [ t ] [ relative-redirect ] }
     } cond ;
diff --git a/extra/http/server/sessions/sessions-tests.factor b/extra/http/server/sessions/sessions-tests.factor
index 5c2d3a57cd..5530b04611 100755
--- a/extra/http/server/sessions/sessions-tests.factor
+++ b/extra/http/server/sessions/sessions-tests.factor
@@ -2,6 +2,8 @@ IN: http.server.sessions.tests
 USING: tools.test http.server.sessions math namespaces
 kernel accessors ;
 
+[ H{ } ] [ H{ } add-session-id ] unit-test
+
 : with-session \ session swap with-variable ; inline
 
 TUPLE: foo ;
@@ -10,7 +12,9 @@ C: <foo> foo
 
 M: foo init-session* drop 0 "x" sset ;
 
-f <session> [
+f <session> "123" >>id [
+    [ H{ { "factorsessid" "123" } } ] [ H{ } add-session-id ] unit-test
+
     [ ] [ 3 "x" sset ] unit-test
     
     [ 9 ] [ "x" sget sq ] unit-test
diff --git a/extra/http/server/sessions/sessions.factor b/extra/http/server/sessions/sessions.factor
index 1d90a32faf..260c80914e 100755
--- a/extra/http/server/sessions/sessions.factor
+++ b/extra/http/server/sessions/sessions.factor
@@ -2,7 +2,8 @@
 ! See http://factorcode.org/license.txt for BSD license.
 USING: assocs calendar kernel math.parser namespaces random
 boxes alarms new-slots accessors http http.server
-quotations hashtables sequences fry combinators.cleave ;
+quotations hashtables sequences fry combinators.cleave
+html.elements ;
 IN: http.server.sessions
 
 ! ! ! ! ! !
@@ -67,12 +68,6 @@ TUPLE: session manager id namespace alarm ;
 : sessions ( -- manager/f )
     \ session get dup [ manager>> ] when ;
 
-GENERIC: session-link* ( url query sessions -- string )
-
-M: object session-link* 2drop url-encode ;
-
-: session-link ( url query -- string ) sessions session-link* ;
-
 TUPLE: null-sessions ;
 
 : <null-sessions>
@@ -88,23 +83,30 @@ TUPLE: url-sessions ;
 
 : sess-id "factorsessid" ;
 
-: current-session ( responder request -- session )
-    sess-id query-param swap get-session ;
+: current-session ( responder -- session )
+    >r request-params sess-id swap at r> get-session ;
+
+: add-session-id ( query -- query' )
+    \ session get [ id>> sess-id associate union ] when* ;
+
+: session-form-field ( -- )
+    <input
+    "hidden" =type
+    sess-id =id
+    sess-id =name
+    \ session get id>> =value
+    input/> ;
 
 M: url-sessions call-responder ( path responder -- response )
-    dup request get current-session [
+    [ add-session-id ] link-hook set
+    [ session-form-field ] form-hook set
+    dup current-session [
         call-responder/session
     ] [
         nip
         f swap new-session sess-id associate <temporary-redirect>
     ] if* ;
 
-M: url-sessions session-link*
-    drop
-    url-encode
-    \ session get id>> sess-id associate union assoc>query
-    dup assoc-empty? [ drop ] [ "?" swap 3append ] if ;
-
 TUPLE: cookie-sessions ;
 
 : <cookie-sessions> ( responder -- responder' )