Item8937: redirectcache breaks TOC links on trunk

pencil
Priority: Urgent
Current State: Closed
Released In:
Target Release: n/a
Applies To: Engine
Component: TemplateLogin
Branches:
Reported By: GeorgeClark
Waiting For:
Last Change By: CrawfordCurrie
  • Log out from trunk.foswiki.org if logged in.
  • Visit http://trunk.foswiki.org/System/PerlDoc?module=Foswiki::Func
  • Redirected to bin/login to enter userid / password
  • After login, redirected back to PerlDoc -
    • TOC links after redirect:
      http://trunk.foswiki.org/bin/login/System/PerlDoc?validation_key=fd15b1f3383a5852a8123f12d82dcd56;module=Foswiki::Func#getSkin_40_41_45_62_36skin
  • The TOC links all point to bin/login instead of using the shorter URL, or bin/view
  • After checking that the TOC links are all goofed, switch language: you end up on http://trunk.foswiki.org/System/PerlDoc so parameters are lost. -- OlivierRaginel - 26 Apr 2010
  • A shift-reload of the redirected page looses all the URL Parameters - the ?foswiki_redirect_cache is the only url param, but the cache is discarded after the redirect without actually being used to restore the url parameters. -- GeorgeClark - 26 Apr 2010

-- GeorgeClark - 21 Apr 2010

The problem seems to be related to browser behavior after a redirect. Firefox and Konqueror have the same behavior. TOC links are generated as relative to the current page, however the browser uses the redirecting URL instead of the displayed page.

(10:35:09 PM) gac410: pharvey: Definitely an issue with TOC - Fails for any TOC after a redirect.  Not just PerlDoc.  
(10:37:32 PM) gac410: Really strange.  urls in source are Relative -  <a href="?validation_key=232a159951cc017f0a3622a188ecad90#Fast_Start">Fast Start</a>  ... Firefox is providing the bogus bin/login for the url.
(10:40:21 PM) gac410: konqueror - exact same behavior.   relative URL's on the page are  relative to the original URL, bin/login,  not the final page bin/view.  
(10:43:04 PM) gac410: So if page is rendered as a redirect from login,  the TOC URL's need to be absolute, not relative URLs.  
(10:48:04 PM) pharvey: gac410: check in the html, the value of the <base> tag
(10:48:29 PM) gac410: That's it - base is set to bin/login
(10:48:35 PM) pharvey: lame :)
(10:53:12 PM) gac410: So issue in template?  foswiki.pattern.tmpl:<base href="%SCRIPTURL{%SCRIPTNAME%}%/%WEB%/%TOPIC%"></base> ... or rendering of %SCRIPTURL tag.

It appears that %SCRIPTNAME% is left set to login following a redirect from view -> login -> view.

See also: Item1157

-- PaulHarvey - 22 Apr 2010

Continuing some digging:
(04:08:56 PM) gac410: pharvey   I turned on cache trace - if I understand the log, the Cache is not used for the initial redirect from bin/view to bin/login    It uses the ?origurl parameter.  However after login complete, it uses the cache to return, but sets the script to login, not view.
(04:09:38 PM) gac410: Almost as if Cache is only partially implemented - for the 2nd redirect.
...
(05:41:45 PM) gac410: pharvey:  digging just a bit further, redirect_cache is not used for GET->GET,  so view->login is not handled through cache.   If I tweak redirect to always use the cache, the redirect GET view->login and POST login->login ...   are two separate cache events - no continuity.   So original script never retained.

Is there a mechanism where the 2nd redirect should use the previous cache? I'm guessing that to fix this:
  • Foswiki.pm needs to use cache for both POST->GET or *->login so that login has a cache entry available.
  • And when login.pm does it's redirect from POST, it needs to somehow reuse the previous cache entry.

More from IRC:
(06:36:43 PM) pharvey: gac410: I am getting lots of separate redirect events in MetaCommentPlugin, MichaelDaum says it's due to TemplateLogin + ajax
(06:36:56 PM) pharvey: (infinite redirect loop where each redirect has a new cache id)
...
(06:38:36 PM) gac410: I think it's fundamental to cache and template.  There is nothing that I can see that would cause the login POST to redirect back to the original url.  I don't see how the script triggering the Login request is preserved.  
(06:39:14 PM) gac410: I tweaked Foswiki.pm to force ALL redirects to use the cache - so it is possible to trace what happens in each redirect.   There is no continuity.
(06:43:07 PM) gac410: TemplateLogin.pm does a simple redirect to (method=GET)  bin/login - Clicking login forces a cache type redirect (method = POST) Cache is built with current script (login) meaning that SCRIPTNAME is set to login, and <base is built using bin/login - breaking TOC.

-- GeorgeClark - 22 Apr 2010

I think I've fixed this. The redirect has always been going to the correct location, but the session "action" - externalized as %SCRIPTNAME% was being set to the cached login script instead of the destination script for the redirect. It doesn't appear that we really need to cache the action. The target script will set the correct action, and the cache is only needed for the URL parameters that are being passed forward.

I've deleted the action from the list of parameters added to the cache and cursory testing appears to be successful.

-- GeorgeClark - 25 Apr 2010

Interesting change: MetaCommentPlugin no longer does an infinite redirect loop, instead I get this:
ERROR: (404) Invalid REST invocation - /Sandbox/TestTopic2 does not refer to a known handlerContent-Length: 92 Status: 404 Content-Type: text/html; charset=ISO-8859-1 Set-Cookie: FOSWIKISID=deadbeef123456789; path=/; HttpOnly Set-Cookie: FOSWIKISID=deadbeef123456789; path=/; HttpOnly ERROR: (404) Invalid REST invocation - /Sandbox/TestTopic2 does not refer to a known handler

POST params:

cmt_action
save
cmt_author
WikiGuest
cmt_ref
 
cmt_text
some comment
cmt_title
some title
redirectto
http://wiki.org/svn/trunk/bin/view/Sandbox/TestTopic2#lastcomment
t
1272352333
topic
TestTopic2
validation_key
deadbeef123456789

Needs some study.

-- PaulHarvey - 27 Apr 2010

I have fixed the original problem reported here. I can't comment on MetaCommentPlugin as I failed to configure it based on the doc. You may want to pick up on MetaCommentPlugin in a new report.

Many thanks to George and Olivier for working with me on IRC to find the fixes.

-- CrawfordCurrie - 03 May 2010

Crawford, The last iteration is still restoring the login path instead of the original rest path. I've reopened this because even though the TOC is correct, the rest invocation gets the wrong path, so the fix to this issue still has problems.

I've enabled cache trace, and also added trace information to the TemplateLogin (lines start with LOGIN).

LOGIN forceAuthentication entered
[Mon May  3 19:35:44 2010] rest: Use of uninitialized value $method in concatenation (.) or string at /var/www/SVN/foswiki/core/lib/Foswiki/LoginManager/TemplateLogin.pm line 53.
[Mon May  3 19:35:44 2010] rest: Use of uninitialized value $action in concatenation (.) or string at /var/www/SVN/foswiki/core/lib/Foswiki/LoginManager/TemplateLogin.pm line 53.
LOGIN _packRequest incoming uri/method/action = Foswiki=HASH(0x848e328) /  /
LOGIN _packRequest returning uri/method/action = /bin/rest/MetaCommentPlugin/comment / POST / rest
CACHE save entered for 0dd2fb778b211d7c949fa303dc7c529b
Saving method =POST
 Saving path_info =/MetaCommentPlugin/comment
 Saving action =rest
LOGIN forceAuthentication redirect to /bin/login/Sandbox/MetaCommentTest
ReqCache: Loading 0dd2fb778b211d7c949fa303dc7c529b
CACHE LOAD set method ( POST )
CACHE LOAD set path_info ( /MetaCommentPlugin/comment )
CACHE LOAD set action ( rest )
ReqCache: Loaded /var/www/SVN/foswiki/core/working/tmp/passthru_0dd2fb778b211d7c949fa303dc7c529b, URL now http://foswiki.blah.com/bin/rest
LOGIN _unpackRequest returning uri/method/action = /bin/rest/MetaCommentPlugin/comment / POST / rest
LOGIN _packRequest incoming uri/method/action = /bin/rest/MetaCommentPlugin/comment / POST / rest
LOGIN _packRequest returning uri/method/action = /bin/rest/MetaCommentPlugin/comment / POST / rest

The first block is the post of a comment to the rest handler and the GET for the bin/login page. This next block begins with the post of the userid/password and the results of the 302 redirect from the post. The path_info was taken from the login, and not from the initial post. Note that the actual URL generated by the redirect is correct. http://foswiki.blah.com/bin/rest/MetaCommentPlugin/comment?foswiki_redirect_cache=37164957bdf62d383fd5fff6ed5eb413, but the cache restores the path back to the incorrect Sandbox/MetaCommentTest

LOGIN _unpackRequest returning uri/method/action = /bin/rest/MetaCommentPlugin/comment / POST / rest
CACHE save entered for 37164957bdf62d383fd5fff6ed5eb413
 Saving method =POST
 Saving path_info =/Sandbox/MetaCommentTest
 Saving action =rest
LOGIN login - redirecting method POST, url /bin/rest/MetaCommentPlugin/comment
ReqCache: Loading 37164957bdf62d383fd5fff6ed5eb413
CACHE LOAD set method ( POST )
CACHE LOAD set path_info ( /Sandbox/MetaCommentTest )
CACHE LOAD set action ( rest )
ReqCache: Loaded /var/www/SVN/foswiki/core/working/tmp/passthru_37164957bdf62d383fd5fff6ed5eb413, URL now http://foswiki.blah.com/bin/rest

-- GeorgeClark - 03 May 2010

CDot, I've worked out a fix that seems to be successful. When saving the cache, I pass along the redirect url as well. I extract and use the path from the url instead of the session path. The redirects work for protected topic, Comment and MetaComment.

svn diff lib/Foswiki.pm lib/Foswiki/Request/Cache.pm 
Index: lib/Foswiki.pm                                                                                                                  
===================================================================                                                                    
--- lib/Foswiki.pm      (revision 7322)                                                                                                
+++ lib/Foswiki.pm      (working copy)                                                                                                 
@@ -1082,6 +1082,8 @@                                                                                                                  
                                                                                                                                       
     ( $url, my $anchor ) = splitAnchorFromUrl($url);                                                                                  
                                                                                                                                       
+    #SMELL:  Is there a better way to recover the path from the redirect url?
+    my ($urlpath) = $url =~ m/^$Foswiki::cfg{ScriptUrlPath}\/[^\/]+(.*)/;                                                             
+                                                                                                                                      
     if ( $passthru && defined $this->{request}->method() ) {                                                                          
         my $existing = '';                                                                                                            
         if ( $url =~ s/\?(.*)$// ) {                                                                                                  
@@ -1090,7 +1092,7 @@                                                                                                                  
         if ( uc( $this->{request}->method() ) eq 'POST' ) {                                                                           
                                                                                                                                       
             # Redirecting from a post to a get                                                                                        
-            my $cache = $this->cacheQuery();                                                                                          
+            my $cache = $this->cacheQuery($urlpath);                                                                                  
             if ($cache) {                                                                                                             
                 if ( $url eq '/' ) {                                                                                                  
                     $url = $this->getScriptUrl( 1, 'view' );                                                                          
@@ -1168,6 +1170,7 @@                                                                                                                  
                                                                                                                                       
 sub cacheQuery {                                                                                                                      
     my $this  = shift;                                                                                                                
+    my $url = shift;                                                                                                                  
     my $query = $this->{request};                                                                                                     
                                                                                                                                       
     return '' unless ( scalar( $query->param() ) );                                                                                   
@@ -1176,7 +1179,7 @@                                                                                                                  
     return '' if ( $query->param('foswiki_redirect_cache') );                                                                         
                                                                                                                                       
     require Foswiki::Request::Cache;                                                                                                  
-    my $uid = Foswiki::Request::Cache->new()->save($query);                                                                           
+    my $uid = Foswiki::Request::Cache->new()->save($query, $url);                                                                     
     if ( $Foswiki::cfg{UsePathForRedirectCache} ) {                                                                                   
         return '/foswiki_redirect_cache/' . $uid;                                                                                     
     }                                                                                                                                 
Index: lib/Foswiki/Request/Cache.pm                                                                                                    
===================================================================                                                                    
--- lib/Foswiki/Request/Cache.pm        (revision 7322)                                                                                
+++ lib/Foswiki/Request/Cache.pm        (working copy)                                                                                 
@@ -29,7 +29,7 @@                                                                                                                      
 use Foswiki::Request::Upload ();                                                                                                      
 use Foswiki::Sandbox ();                                                                                                              
                                                                                                                                       
-sub TRACE_CACHE { 0 };                                                                                                                
+sub TRACE_CACHE { 1 };                                                                                                                
                                                                                                                                       
 =begin TML                                                                                                                            
                                                                                                                                       
@@ -60,13 +60,16 @@                                                                                                                    
 =cut                                                                                                                                  
                                                                                                                                       
 sub save {                                                                                                                            
-    my ($this, $req) = @_;                                                                                                            
+    my ($this, $req, $url) = @_;                                                                                                      
                                                                                                                                       
     # get a hex-encoded session-specific unguessable key for the cache                                                                
     my $digester = new Digest::MD5();                                                                                                 
     $digester->add( $$, rand(time) );                                                                                                 
     my $uid = $digester->hexdigest();                                                                                                 
                                                                                                                                       
+    print STDERR "CACHE save entered for $uid \n" if (TRACE_CACHE);                                                                   
+    print STDERR " passed URL $url \n" if (TRACE_CACHE && $url);                                                                      
+                                                                                                                                      
     # passthrough file is only written to once, so if it already exists,                                                              
     # suspect a security hack (O_EXCL)                                                                                                
     my $F;                                                                                                                            
@@ -78,11 +81,13 @@                                                                                                                    
       . $!;                                                                                                                           
                                                                                                                                       
     # Serialize some key info from the request                                                                                        
-    foreach my $field qw(method path_info action) {                                                                                   
+    foreach my $field qw(method action) {                                                                                             
         print $F $field,'=', ($req->$field() || ''), "\n";                                                                            
-        print STDERR "Saving $field =" . $req->$field() || '' . "\n"                                                                  
+        print STDERR "Saving $field =" . ($req->$field() || '') . "\n"                                                                
           if (TRACE_CACHE);                                                                                                           
     }                                                                                                                                 
+                                                                                                                                      
+    #SMELL  -  Need to pick up passed url, or path_info if not supplied.
+    print $F 'path_info=', ($url || ''), "\n";                                                                                        
     print $F "=\n";                                                                                                                   
                                                                                                                                       
     # Serialize the request parameters                                                      

-- GeorgeClark - 04 May 2010

After some experimentation I think I finally understand the problem:
  1. rest has to be added to $Foswiki::cfg{AuthScripts}
  2. The original request to the rest handler has to be a POST (GET works fine)

The problem is actually in the login template. In that template, it sets the <form action= using %WEB and %TOPIC; but of course by the time the login template is expanded, the web and topic have been set from the topic url parameter. This action should actually use the path_info; or, the path_info should be cached as part of the origin.

OK, this time I understand the MetaCommentPlugin problem. Should be OK now (r7325). If not, please open a new task, as this one has been abused to fix three different bugs so far frown, sad smile

-- CrawfordCurrie - 04 May 2010

 
Topic revision: r32 - 04 May 2010, CrawfordCurrie
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy