Notification (e.g. "Thank you for your bug report") appears in wrong window

Bug #131165 reported by Ian Jackson
80
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Launchpad itself
Triaged
High
Unassigned

Bug Description

Symptoms
========
I just submitted two bug reports against the same package in Ubuntu nearly simultaneously. I did this using firefox from a slightly out-of-date gutsy, with two windows open onto
 https://bugs.launchpad.net/ubuntu/+source/gfxboot/+filebug
into which I'd filled the respective bug details. When I was satisfied with my two reports I used the mouse to click "Submit Bug Report" on both pages, one quickly after the other.

Both bugs appears to have been filed correctly at:
 https://bugs.launchpad.net/ubuntu/+source/gfxboot/+bug/131161
 https://bugs.launchpad.net/ubuntu/+source/gfxboot/+bug/131160

But, the window for 131160 starts with two copies of the "Thank you for your bug report" (i)nformation item, and the window for 131161 has neither. Incidentally, I think I pressed submit first on the window that became 131161.

Cause
=====

The page notification subsystem in Launchpad uses a session key that is in a cookie shared amongst all browser windows. POSTs are followed up by a redirect and the notification added to the GET of the page the browser was redirected to. The server then generates the page and dumps all outstanding notifications into the page content.

What we want to achieve is to notify a subset of events to the *current* window / process that is taking the action. This is perhaps something that the notification service should assist with - when the notification is a global one (vs e.g. 'field XYZ is not an integer'). This might be done by having the notification service API return a hint for 'include in the current action results'. Separately, *if* LP had a consistent timeline widget we could look at always including all subscribed events in responses to actions, and they would render on the right hand side; possibly client side logic could take care of all the side effects from the requested action.

Possible fixes
==============

* Include a notification reference in the redirect from the POST allowing a unique key to pickup specific notifications. (Users would see a query parameter on the url right after taking mutating-pageload-actions). That parameter would be ignored if they copy-pasted it.

* Render the final result immediately rather than redirecting from the POST. Users taking mutating pageload actions would see the url that the action was POSTed too which (in our current system) might be a bit ugly. On the upside this would be significantly faster - less duplicate DB access, less roundtrips.

* On the server side include the page the notification is for. Then when we dump notifications we'd only show the notifications that are relevant to the page the user ends up on. This is the simplest approach known of to-date.

* Use form nonces where the POST takes a nonce, visible in the referrer of the follow-on GET and thus usable to key into the notifications set.

* Move all 'this worked' etc notifications into the client side using the data returned from the API call. This would essentially deprecate the existing system.

Revision history for this message
Ian Jackson (ijackson) wrote :
Revision history for this message
James Henstridge (jamesh) wrote :

There are a number of trade-offs at work here, so I don't really see this bug getting fixed. I am subscribing Stuart, since he worked on this notification code.

When you post a form on Launchpad, we reply with a redirect to another page. This is done because hitting the reload button on a page that results from a POST will prompt the user to repost the data, which could then result in duplicate comments, bugs, etc. Lots of people do hit the reload button and few read the dialog (developers and users alike), so the post-then-redirect behaviour is required.

So the page where you see the notification is not being served by the request that set the notification. The notifications are handled through the session cookie. As cookies are handled on a per-instance basis rather than per-window or tab, all the windows you have open share the same session. In this case, the POST requests for the two bugs filed would have been processed creating the notifications, then the first window that issued the subsequent GET picked up the two notifications.

We used to solve this problem by appending a query parameter, but changed to the current implementation due to complaints about the ugly URLs (see bug 5412).

Revision history for this message
James Henstridge (jamesh) wrote :

Closing as per bug 132747

Changed in malone:
status: New → Won't Fix
Revision history for this message
Matthew Paul Thomas (mpt) wrote :

Reproduced in bug 172688. Reopening because Robert Collins says there's a way to fix this without using URL parameters.

Changed in malone:
importance: Undecided → Low
status: Won't Fix → Confirmed
Revision history for this message
John A Meinel (jameinel) wrote :

Just to give a brief summary, I believe Robert mention you could use the referrer to get a clue as to what page they were coming from, and could certainly use the page you are referring them to as clues to what you want them to see.

Also, I mentioned in my bug report that some of the status messages could include a pointer to the page they were supposed to describe, which could be hidden if that page == current_page.

(For example, rather than saying "Attachment foobar added." say "Attachment foobar added to bug #13234". If this showed on the wrong page, it would be less confusing.)

Revision history for this message
James Henstridge (jamesh) wrote :

I am not sure how much that'd help in Ian's case.

For the two bugs he submitted simultaneously, they both would have redirected through https://bugs.launchpad.net/ubuntu/+source/gfxboot/+filebug before reaching the page where the notification is displayed, so they'd also have identical "Referer" headers.

Revision history for this message
Stuart Bishop (stub) wrote :

Setting to Won't Fix until someone can come up with a way to identify if a notification should display in the current window that doesn't violate our other requirements (such as not passing a token in the URL, which is how we where originally doing this without the race condition)

Changed in launchpad:
status: Confirmed → Won't Fix
Revision history for this message
Martin Pool (mbp) wrote :

Correctness is more important than pretty URLs imo. Many other sites and applications use a query parameter to show the notification, and consistency with general practice is useful - advanced users can learn they can trim off the notification query string.

You should also consider that race conditions reduce user trust in the application, whereas savvy users will recognize extraneous parameters as sometimes being a necessary evil in http.

What you could also do is have a 'close' button or text link on the notification text that sends the user to the same page without the notification qp, and which will both remove the message and tidy up the url.

Revision history for this message
Andrew Bennetts (spiv) wrote :

Quick idea: you *can* use the Referrer header to disambiguate reliably. Every time you render a <form> include a "?nonce=XXX" in the form action. When the POST action generates a notification tie it to that nonce, and then the resulting GET can find the right notification to display.

Revision history for this message
Martin Pool (mbp) wrote :

To follow on from Robert's idea of the referrer, you could use a referrer page whose URL contains the eventual destination and the notification information. So the submission of the post request would redirect you to say /+notification/?message=thanks+for+the+bug&destination=/bugs/12345. This would then redirect you to /bugs/12345, which could look at the referer to see it should display a notification. If someone copy&pastes or bookmarks the page it will not be shown.

If they are using privacy software that hides the Referer (even for the same site and on ssl, which would be extreme) it will still work, but they won't get the notification. (This case is probably moot because such software would typically block cookies too.)

Revision history for this message
Martin Pool (mbp) wrote :

reopening as there are some plausible ways to fix it

Changed in launchpad:
importance: Low → Undecided
status: Won't Fix → Confirmed
Revision history for this message
John A Meinel (jameinel) wrote :

The only thing I would caution about using referrer and a message, is that I've seen sites that blindly post whatever is in the query. So savvy people can do:

/+notification/?message=Marry+Me+Susan&destination=/bugs/12345

(obviously there are more abusive things that could be sent as a message)

I think using pre-canned message ids, or maybe just a white list of allowed texts would be a reasonable way around it.

Revision history for this message
Kristopher Ives (krisives) wrote :

Why can't you use an HTTP header when you redirect with the token, since that would avoid a POST and not garble the URL. An easier solution would be to only display the message and remove the info from the session/cookie when the user goes to the page associated with that message. For example, the first bug redirect should only remove the session data associated with THAT bug post, not both.

Curtis Hovey (sinzui)
Changed in launchpad-foundations:
status: Confirmed → Triaged
importance: Undecided → Low
Changed in launchpad:
importance: Low → High
description: updated
tags: added: ui
Revision history for this message
Robert Collins (lifeless) wrote :

@Kristopher setting a http header requires javascript vs plain-old-forms, which we're still hesitant to depend on (and there are multiple other ways to do it).

description: updated
tags: added: notifications
description: updated
Revision history for this message
tsg1zzn (tsg1zzn) wrote :

You could do it with
- no redirect
- no extra cookies
- no ugly urls:
Every time a bug submit page is sent to the user, a unique id (counter) is added as a hidden form field. This will be sent back to the server in the POST request when the user submits the bug report. Because POST requests aren't sent in the URL, there is no ugly url involved.
When the server receives the request, it checks if the unique id is in the table of processed ids. If it is, just return an error "don't submit the page twice".
If the unique id is not in the table of processed ids, add it to the table of processed ids, and then process the bug report normally and return html on the same request.
The ids can be recycled periodically. We don't expect people to keep the same bug report page open for more than a few weeks without clicking submit.

Revision history for this message
Robert Collins (lifeless) wrote : Re: [Bug 131165] Re: Notification (e.g. "Thank you for your bug report") appears in wrong window

that is also a valid approach (I think its already covered above actually :)).

It isn't quite as trivial as you sketch though: we do millions of
pages a day with forms on them, and the problem is not in any way
limited to bug reporting forms; additionally you need to be secure
against spoofing/injection attacks.

That said, if you are interested in working on this, cool!

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.