Opened 10 years ago

Closed 10 years ago

#8249 closed defect (fixed)

sagenb: notebook cookies

Reported by: was Owned by: was
Priority: major Milestone: sage-4.4.2
Component: notebook Keywords:
Cc: acleone, mpatel, mhampton Merged in: sagenb-0.8
Authors: Mitesh Patel Reviewers: Alex Leone, Tim Dumol
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by mpatel)

This is a followup to #6353. That ticket improved cookie naming a bit. However, it appears to be not enough.

On Thu, Feb 11, 2010 at 7:21 PM, Marshall Hampton <> wrote:
> Just for the record, this has happened to me quite a bit recently.
>
> I use a lot of different sage servers, often running different
> versions, so I don't usually report this kind of stuff since I think I
> am something of an extreme case.  But most of the servers I use are
> now running 4.3.2 and I am pretty sure I have seen the cookie message
> more than before.
>

Here's the relevant ticket I was remembering:

     http://trac.sagemath.org/sage_trac/ticket/6353

It is definitely in sage-4.3.2 (since it is merged into sagenb-0.7.4).   

Looking at that patch show that:

  (1) it addresses a related issue,

  (2) it might not solve the issue we're discussing, since it merely includes the *port* in the cookie name -- some unique id for the notebook (e.g., the URL or something else) is maybe also needed to fix the problem we're discussing.

So somebody should look at ticket 6353, see if a small modification of it would give a real fix, and make said modification.      Alex Leone: this would be a good project for you, if you're looking for something to do on the notebook. 

 -- William

See sage-support.

Attachments (3)

trac_8249-notebook_cookies.patch (1.5 KB) - added by mpatel 10 years ago.
Expire cookies on logout. sagenb repo.
trac_8249-notebook_cookies-selenium_errors.log (9.8 KB) - added by acleone 10 years ago.
Selenium errors with patch. Same errors when the patch for #6069 -missing_pub_ws.3 is applied.
trac_8249-notebook_cookies.2.patch (1.9 KB) - added by mpatel 10 years ago.
Adjust close_callback to make Se tests pass. Apply only this patch.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 10 years ago by acleone

I couldn't reproduce his by signing in/out of sagenb.org and demo.sagenb.org. Cookies are stored by domain (sagenb.org and demo.sagenb.org are two seperate sites), so differentiating by port should be all that's necessary.

I made sure all instances of 'cookie' in twist.py were updated from #6353 (they were), so I really have no idea what is causing this. It could be a subtle bug in the 'cookie_test' cookie.

comment:2 Changed 10 years ago by acleone

  • Cc acleone mpatel added

comment:3 Changed 10 years ago by mpatel

  • Description modified (diff)

comment:4 Changed 10 years ago by mpatel

  • Cc mhampton added

Has anyone been able to reproduce the problem reliably? It would help greatly to have specific instructions.

Changed 10 years ago by mpatel

Expire cookies on logout. sagenb repo.

comment:5 Changed 10 years ago by mpatel

I've attached a patch that should delete both the test and notebook session cookies, when a user logs out.

comment:6 Changed 10 years ago by mpatel

The patch may depend weakly on #6069.

comment:7 Changed 10 years ago by mpatel

  • Authors set to Mitesh Patel
  • Priority changed from minor to major
  • Status changed from new to needs_review

comment:8 Changed 10 years ago by acleone

  • Status changed from needs_review to needs_work

LGTM: I can't produce any cookie errors through normal use.

However, selenium errors when logging out - I'm working on a new patch to fix the tests. (See attached for log)

Changed 10 years ago by acleone

Selenium errors with patch. Same errors when the patch for #6069 -missing_pub_ws.3 is applied.

comment:9 Changed 10 years ago by mpatel

Thanks for catching the Se test errors. I should have checked.

The patch fixes for me the one cookie-related problem I could reproduce reliably: Logging out in Chrom* displays a browser error page:

This webpage has a redirect loop.

The webpage at http://localhost:8000/home/admin/ has resulted in too many redirects. Clearing your cookies for this site or allowing third-party cookies may fix the problem. If not, it is possibly a server configuration issue and not a problem with your computer.

But with the patch, clicking on "Sign Out" just returns me to the login page. I'm not sure if it helps with the reported problems, but making the cookies expire on logout seems logical.

comment:10 Changed 10 years ago by acleone

For some reason Selenium doesn't like HTTP redirect responses. I keep getting this 'Problem loading page' error (in firefox):

The page isn't redirecting properly
      
Firefox has detected that the server is redirecting the request for this address in a way that will never complete.

    *   This problem can sometimes be caused by disabling or refusing to accept
          cookies.

This always happens when the selenium gets the redirect HTTP response. Perhaps we should change it back to a dedicated logout page, but add a <meta http-equiv="Refresh" tag so the page redirects after a second.

Changed 10 years ago by mpatel

Adjust close_callback to make Se tests pass. Apply only this patch.

comment:11 Changed 10 years ago by mpatel

V2 replaces '/' with '/home/' + user_name in notebook_lib.js's close_callback. Strangely, this seems to work.

comment:12 Changed 10 years ago by mpatel

  • Status changed from needs_work to needs_review

comment:13 Changed 10 years ago by timdumol

I'm signing this off since it's a good idea, and may help. I'm unable to replicate the cookie issue though, with or without this patch, but it may be related to the performance issues of the sagenb server.

comment:14 Changed 10 years ago by timdumol

  • Reviewers set to Tim Dumol
  • Status changed from needs_review to positive_review

comment:15 Changed 10 years ago by timdumol

  • Reviewers changed from Tim Dumol to Alex Leone, Tim Dumol

Woops. Forgot to add Alex.

comment:16 Changed 10 years ago by timdumol

  • Merged in set to sagenb-0.8

comment:17 Changed 10 years ago by timdumol

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.