Permission denied: "Cannot create '+filediff' viewing diffs on +branch urls

Bug #1055751 reported by James Westby on 2012-09-24
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Launchpad itself
Critical
j.c.sackett

Bug Description

OOPS-0625726f0e60d9ad5e7084aa6dad9bd0

When trying to see the diff content on https://bazaar.launchpad.net/+branch/launchpad/revision/16015 (and any other branch)

  Traceback (most recent call last):
  Module oops_wsgi.middleware, line 208, in oops_middleware
    app(environ, oops_start_response))
  Module launchpad_loggerhead.debug, line 106, in wrapped_application
    result = application(environ, response_hook)
  Module __main__, line 166, in wrapped
  Module launchpad_loggerhead.debug, line 45, in wrapped
    return app(environ, start_response)
  Module paste.translogger, line 68, in __call__
    return self.application(environ, replacement_start_response)
  Module paste.deploy.config, line 285, in __call__
    return self.app(environ, start_response)
  Module __main__, line 150, in wrapped
  Module paste.wsgilib, line 179, in catch_errors
    app_iter = application(environ, start_response)
  Module launchpad_loggerhead.session, line 56, in __call__
    return self.cookie_handler(environ, start_response)
  Module paste.auth.cookie, line 305, in __call__
    return self.application(environ, response_hook)
  Module launchpad_loggerhead.session, line 83, in _process
    return self.application(environ, response_hook)
  Module paste.httpexceptions, line 636, in __call__
    return self.application(environ, start_response)
  Module launchpad_loggerhead.app, line 277, in __call__
    lp_server.get_url().strip(':/'), branch_url)
  Module lp.codehosting.safe_open, line 351, in safe_open
    ignore_fallbacks=ignore_fallbacks)
  Module lp.codehosting.safe_open, line 335, in open
    url = self.checkAndFollowBranchReference(url)
  Module lp.codehosting.safe_open, line 252, in checkAndFollowBranchReference
    next_url = self.followReference(url)
  Module lp.codehosting.safe_open, line 295, in followReference
    bzrdir = self._open_dir(url)
  Module lp.codehosting.safe_open, line 326, in _open_dir
    redirected)
  Module bzrlib.transport, line 1718, in do_catching_redirections
    return action(transport)
  Module lp.codehosting.safe_open, line 319, in find_format
    return transport, prober.probe_transport(transport)
  Module bzrlib.bzrdir, line 1250, in probe_transport
    format_string = transport.get_bytes(".bzr/branch-format")
  Module lp.codehosting.vfs.transport, line 305, in get_bytes
    return extract_result(self._async_transport.get_bytes(relpath))
  Module lp.services.twistedsupport, line 102, in extract_result
    failures[0].raiseException()
  Module twisted.python.failure, line 370, in raiseException
    raise self.type, self.value, self.tb
PermissionDenied: Permission denied: "Cannot create '+filediff'. Only Bazaar branches are allowed."

Related branches

James Westby (james-w) on 2012-09-24
tags: added: oops
Changed in launchpad:
importance: Undecided → Critical
status: New → Confirmed
Changed in launchpad:
status: Confirmed → Triaged
Changed in loggerhead:
status: New → Triaged
importance: Undecided → Critical
summary: - Permission denied: "Cannot create '+filediff' trying to get diff for a
- file
+ Permission denied: "Cannot create '+filediff' viewing diffs on +branch
+ urls on
summary: Permission denied: "Cannot create '+filediff' viewing diffs on +branch
- urls on
+ urls
Curtis Hovey (sinzui) on 2012-09-24
tags: added: 403
Curtis Hovey (sinzui) on 2012-09-24
description: updated
William Grant (wgrant) on 2012-09-25
no longer affects: loggerhead
j.c.sackett (jcsackett) on 2012-10-04
Changed in launchpad:
assignee: nobody → j.c.sackett (jcsackett)
j.c.sackett (jcsackett) on 2012-10-09
Changed in launchpad:
assignee: j.c.sackett (jcsackett) → nobody
Curtis Hovey (sinzui) wrote :

I see another round of these errors in the oops reports. They relate to Aaron, Abel, Steve, Stuart, and Ian. I wonder if this relates to cursed scans and deleted revisions. Maybe the loggerhead page/app's information is out of sync with Lp, and when a rev does not exist, but the error is "Only Bazaar branches are allowed" which adapts to a 403.

Curtis Hovey (sinzui) wrote :

This recent oops OOPS-d9ab95c6985913b6b3a231f960144322 has the same trace back as above. but when you look the same url today, you get OOPS-2d07699043120927cb214dce8a792337 which is

    Traceback (most recent call last):
  Module oops_wsgi.middleware, line 208, in oops_middleware
    app(environ, oops_start_response))
  Module launchpad_loggerhead.debug, line 106, in wrapped_application
    result = application(environ, response_hook)
  Module __main__, line 166, in wrapped
  Module launchpad_loggerhead.debug, line 45, in wrapped
    return app(environ, start_response)
  Module paste.translogger, line 68, in __call__
    return self.application(environ, replacement_start_response)
  Module paste.deploy.config, line 285, in __call__
    return self.app(environ, start_response)
  Module __main__, line 150, in wrapped
  Module paste.wsgilib, line 179, in catch_errors
    app_iter = application(environ, start_response)
  Module launchpad_loggerhead.session, line 56, in __call__
    return self.cookie_handler(environ, start_response)
  Module paste.auth.cookie, line 305, in __call__
    return self.application(environ, response_hook)
  Module launchpad_loggerhead.session, line 83, in _process
    return self.application(environ, response_hook)
  Module paste.httpexceptions, line 636, in __call__
    return self.application(environ, start_response)
  Module launchpad_loggerhead.app, line 287, in __call__
    return view.app(environ, start_response)
  Module loggerhead.apps.branch, line 197, in app
    return c(environ, start_response)
  Module loggerhead.controllers, line 104, in __call__
    values = self.get_values(path, self.kwargs, headers)
  Module loggerhead.controllers.filediff_ui, line 88, in get_values
    self._history._branch.repository, file_id, compare_revid, revid)
  Module loggerhead.controllers.filediff_ui, line 64, in diff_chunks_for_file
    for r, bytes_iter in repository.iter_files_bytes(args):
  Module bzrlib.vf_repository, line 1565, in iter_files_bytes
    raise errors.RevisionNotPresent(record.key[1], record.key[0])
RevisionNotPresent: Revision {abel} not present in "<email address hidden>".

I think that we have a sync/cache/stale page issue where a user can request an non-existent rev from loggerhead. The latter oops is likely to be a stale URL after Lp and Loggerhead know the truth. The first error is caused when one side does not know the truth. The first error is raised when Lp is checking for valid path segments into the bzr dir, but instead only has +filediff. I think this means that the real path evaluated to nothing, so only that last segment of +fileview was present.

1. the callsite could raise a different error for this case, it probably should have been cause higher up the call chain.
2. Loggerhead is not handling the oops correctly. This should be a 404 since the resource was deleted and the page should make that clear...reloading the page will not make the page load.

William Grant (wgrant) on 2012-10-18
tags: added: codebrowse
Curtis Hovey (sinzui) wrote :

All the examples use the branch 's project or series alias. ie:
   /launchpad instead of /~launchpad-pqm/launchpad
   /launchpad/devel instead of /~launchpad-pqm/launchpad/devel

OOPS-d9ab95c6985913b6b3a231f960144322 shows
The code that
 Module launchpad_loggerhead.app, line 277, in __call__
    lp_server.get_url().strip(':/'), branch_url)
  Module lp.codehosting.safe_open, line 351, in safe_open
    ignore_fallbacks=ignore_fallbacks)
  Module lp.codehosting.safe_open, line 335, in open
    url = self.checkAndFollowBranchReference(url)
  Module lp.codehosting.safe_open, line 252, in checkAndFollowBranchReference
    next_url = self.followReference(url)
  Module lp.codehosting.safe_open, line 295, in followReference
    bzrdir = self._open_dir(url)
  Module lp.codehosting.safe_open, line 326, in _open_dir
    redirected)
  Module bzrlib.transport, line 1718, in do_catching_redirections
    return action(transport)
  Module lp.codehosting.safe_open, line 319, in find_format
    return transport, prober.probe_transport(transport)
  Module bzrlib.bzrdir, line 1250, in probe_transport
    format_string = transport.get_bytes(".bzr/branch-format")
  Module lp.codehosting.vfs.transport, line 305, in get_bytes
    return extract_result(self._async_transport.get_bytes(relpath))
  Module lp.services.twistedsupport, line 102, in extract_result
    failures[0].raiseException()
  Module twisted.python.failure, line 370, in raiseException
    raise self.type, self.value, self.tb
PermissionDenied: Permission denied: "Cannot create '+filediff'. Only Bazaar branches are allowed."

Which might mean that /launchpad did not expand to /~launchpad-pqm/launchpad so the URL did not map to a path on disk. As Lp stepped through the mapped parts to directories, it encountered "+filediff" because the bad mapping was shorter than expected -- it was missing the user.

We might be able to verify this in a test by passing bad path to one of the methods in the call chain and verify it does ot does not expand the user.

j.c.sackett (jcsackett) on 2012-10-31
Changed in launchpad:
assignee: nobody → j.c.sackett (jcsackett)
j.c.sackett (jcsackett) on 2012-11-06
Changed in launchpad:
status: Triaged → In Progress
Curtis Hovey (sinzui) wrote :

he QA for this is to
Visit https://bazaar.qastaging.launchpad.net/+branch/launchpad/revision/14654
Used the expand all link
verify the diffs load

Launchpad QA Bot (lpqabot) wrote :
tags: added: qa-needstesting
Changed in launchpad:
status: In Progress → Fix Committed
Curtis Hovey (sinzui) on 2012-11-14
tags: added: qa-oj
removed: qa-needstesting
tags: added: qa-ok
removed: qa-oj
William Grant (wgrant) on 2012-11-15
Changed in launchpad:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers