My favorites | Sign in
Project Home Wiki Issues
New issue   Search
for
  Advanced search   Search tips
Issue 1334: Fragment expansion brings up 404 error
3 people starred this issue and may be notified of changes. Back to list
Status:  Fixed
Owner:  chip...@gmail.com
Closed:  Oct 2009


Sign in to add a comment
 
Reported by thilo.al...@gmail.com, Sep 28, 2009
*NOTE: Do not post confidential information in this bug report.*

What version are you running?
1.0.3.1

What's the URL of the page containing the problem?
https://reviewboard.<domain>/r/<reviewid>/diff/#index_header

What steps will reproduce the problem?
1. Submit a review request through Post Review
2. In draft mode, click on "View Diff" once
3. Publish the review request
4. Open the request (either as submitter or reviewer)
5. Click on "View Diff"
6. Try to expand expand a hidden source code section by clicking on "Expand"

What is the expected output? What do you see instead?
Clicking on any of the "Expand" hyperlinks (to expand collapsed source code
sections) in the RB diff viewer brings up a 404 error. The URL to retrieve
the file fragment uses an incorrect diff index (0 instead of 1), such as:
  https://reviewboard.<domain>/r/<reviewid>/diff/0/fragment/2785/chunk/8/

Manually invoking the URL with a corrected diff # correctly retrieves the
fragment.

What operating system are you using? What browser?
Server: Windows Server 2003. Client: browser-independent.

Please provide any additional information below.

The issue only occurs when selecting "View Diff" at least once in draft
mode and does not occur when blindly publishing the review request.
Comment 1 by project member chip...@gmail.com, Sep 28, 2009
Interesting. Now, what happens if you repro this and then, on the diff where it
normally fails, you shift-reload to bypass the cache? I suspect this is either a
server-side or a client-side caching problem, so that'll be an important test.
Status: NeedInfo
Labels: Component-DiffViewer
Comment 2 by thilo.al...@gmail.com, Sep 28, 2009
The following test was performed with a web browser that did not previously access
the review request, so client-side caching should not be in place.

When initially accessing the request and clicking "Expand" the error happened again.
Just to be on the safe side I also shift-reloaded the page and hit "Expand" again -
with unchanged results.

One of the "Expand" links reads:

-- 8< --
87 lines hidden [<a href="#" onclick="javascript:expandChunk('file0', '2830', '0',
null, '0', this); return false;">Expand</a>]
-- 8< --

It apparently references the incorrect diff revision. In the same HTTP response
(towards the top of the request) there is also a variable that correlates to the diff
revision:

var gRevision = '1';

This one, however, seems to be correct and reference diff #1, so I'd rule out any
HTTP-related caching issues.

Next, I restarted the Apache httpd that hosts Review Board to rule out any in-memory
mod_python issues. The issue still persisted.

Restarting memcached, however, solved the issue, so cached content seems to play an
important role.
Comment 3 by project member chip...@gmail.com, Sep 28, 2009
Looks like we're not including the revision number, just the database ID of the
filediff. That would explain it. I'll get this fixed in a point release.
Status: Confirmed
Owner: chipx86
Labels: -Priority-Medium Priority-Critical Milestone-Release1.0.x EasyFix
Comment 4 by ciaom...@gmail.com, Oct 1, 2009
We're seeing this same issue. Looking forward to the fix!

Just as an added piece of info... turning off memcache on the server resolves the 404
error for us. Restarting memcache also helps in specific cases, but other folks were
continuing to run into 404 errors so I've had to turn it off completely for now. Strange.
Comment 5 by project member chip...@gmail.com, Oct 1, 2009
Should be fixed in our git repository now. You can try doing an install from there.

Fixed on release-1.0.x (rdbc7668) and master (r52dc35f).
Status: Fixed
Comment 6 by HooHaa8...@gmail.com, Feb 5, 2010
I see this same behavior in release 1.1 Alpha 1.  Was something not integrated?  
Comment 7 by project member chip...@gmail.com, Feb 5, 2010
Alpha 1 predates the 1.0.x fix for this. Please try a nightly.
Sign in to add a comment

Powered by Google Project Hosting