branch merge proposal pages should limit how much diff they show

Bug #401554 reported by Michael Hudson-Doyle
32
This bug affects 4 people
Affects Status Importance Assigned to Milestone
Launchpad itself
Fix Released
High
Tim Penhey

Bug Description

Because diffs can be arbitrarily large, and trying to render a diff that's several tens of megabytes in size doesn't do anyone any favours.

Probably the fix is to restrict ourselves to the first 2000 lines or 1 megabyte or so, whichever is smaller.

Tags: lp-code

Related branches

Changed in launchpad-code:
status: New → Triaged
importance: Undecided → High
Changed in launchpad-code:
milestone: none → 2.2.8
Revision history for this message
Eledran (eledran-deactivatedaccount) wrote :

I think that rendering the page to x first lines of the diff is not the wiser solution. Neither to render it to x megabytes of text.

Instead, give a link to the diff could be given, with the mask (Download diff {Size in MB}), as even the diff lines counter for the raw text file can run out of time. Besides the script can be very optimized, a attacker could down the site by making it run many times...

If you want, you can try to give a look around this set of diffs:

http://bazaar.launchpad.net/~icy-phoenix-admins/icy-phoenix/1.2-stable/revision/1?start_revid=110

Warning! Even my browser (firefox) has said me the script used for the diff retrive was blocking the computer.

So, there can be a server-side script that makes the diff file, and the lp template files could strip out the diff from the page, showing only the link, and its size in megabytes.

Just tring to give ideas as the fix is not commited yet... but as you are holding the fix development, I guess it is up to you and the Launchpad groups up to determine the appropiate fix for this bug.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

The diff is precomputed and stored in the librarian, and we store how large it is when we generate it. So that shouldn't be an issue, I hope.

Thanks for the thoughts though!

Tim Penhey (thumper)
Changed in launchpad-code:
assignee: nobody → Tim Penhey (thumper)
status: Triaged → In Progress
Revision history for this message
Tim Penhey (thumper) wrote :

Fixed in devel 9042.

Changed in launchpad-code:
status: In Progress → Fix Committed
Tim Penhey (thumper)
Changed in launchpad-code:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

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