Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cgitb.html fails if getattr call raises exception #48893

Closed
amc1 mannequin opened this issue Dec 12, 2008 · 11 comments
Closed

cgitb.html fails if getattr call raises exception #48893

amc1 mannequin opened this issue Dec 12, 2008 · 11 comments
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@amc1
Copy link
Mannequin

amc1 mannequin commented Dec 12, 2008

BPO 4643
Nosy @karlcow, @iritkatriel
PRs
  • bpo-4643: Handles failure with getattr exception in cgitb #24038
  • Superseder
  • bpo-1047397: cgitb failures
  • Files
  • attrtest.py
  • issue4643.patch: patch and test case for issue 4643
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2021-01-02.19:28:42.567>
    created_at = <Date 2008-12-12.14:48:32.685>
    labels = ['easy', 'type-bug', '3.8', '3.9', '3.10', 'library']
    title = 'cgitb.html fails if getattr call raises exception'
    updated_at = <Date 2021-01-02.19:28:42.566>
    user = 'https://bugs.python.org/amc1'

    bugs.python.org fields:

    activity = <Date 2021-01-02.19:28:42.566>
    actor = 'iritkatriel'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-01-02.19:28:42.567>
    closer = 'iritkatriel'
    components = ['Library (Lib)']
    creation = <Date 2008-12-12.14:48:32.685>
    creator = 'amc1'
    dependencies = []
    files = ['12330', '28229']
    hgrepos = []
    issue_num = 4643
    keywords = ['patch', 'easy']
    message_count = 11.0
    messages = ['77672', '78005', '78457', '78503', '177060', '381025', '381026', '384170', '384183', '384221', '384237']
    nosy_count = 5.0
    nosy_names = ['amc1', 'ggenellina', 'karlcow', 'arthur.petitpierre', 'iritkatriel']
    pr_nums = ['24038']
    priority = 'normal'
    resolution = 'duplicate'
    stage = 'resolved'
    status = 'closed'
    superseder = '1047397'
    type = 'behavior'
    url = 'https://bugs.python.org/issue4643'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    @amc1
    Copy link
    Mannequin Author

    amc1 mannequin commented Dec 12, 2008

    If cgitb.html tries to get the value of an attribute from an object, and
    the getattr call causes an exception to be raised (other than an
    AttributeError), then cgitb.html fails to work:

    If you run the attached file in Python 2.5.2 or 2.6, you get the
    following output:

    ----

    A: the letter a
    B:
    Something went wrong - attempting to generate HTML stack trace.
    Error generating HTML stack trace!
    Traceback (most recent call last):
      File "attrtest.py", line 21, in <module>
        html_text = cgitb.html(sys.exc_info())
      File "C:\python26\lib\cgitb.py", line 133, in html
        vars = scanvars(reader, frame, locals)
      File "C:\python26\lib\cgitb.py", line 84, in scanvars
        value = getattr(parent, token, __UNDEF__)
      File "attrtest.py", line 8, in __getattr__
        return str(slf) # Intentional NameError
    NameError: global name 'slf' is not defined

    The problem is in the scanvars function - it offers no protection
    against any unexpected exceptions that occur (that escape the getattr
    call) - which can be a problem if it is the same problematic code that
    caused cgitb.html to be called in the first place.

    I think scanvars should catch any exceptions that come from the getattr
    call and either mention that the attribute value could not be
    determined, or simply omit the mention of the attribute at all.

    If the line in the attached file is commented out, then the next line is
    caught appropriately and formatted correctly (the offending code occurs
    in the same block, but doesn't cause the same problem because it raises
    an AttributeError).

    @amc1 amc1 mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Dec 12, 2008
    @amc1
    Copy link
    Mannequin Author

    amc1 mannequin commented Dec 18, 2008

    In terms of patching scanvars, I came up with the following solution:

    ORIGINAL:
    if parent is not __UNDEF__:
    value = getattr(parent, token, __UNDEF__)
    vars.append((prefix + token, prefix, value))

    SOLUTION:
    if parent is not __UNDEF__:
    try:
    value = getattr(parent, token, __UNDEF__)
    except Exception:
    value = __UNDEF__
    vars.append((prefix + token, prefix, value))

    I think this makes the most sense - it requires a small change, and it
    won't have any strange side effect. One slightly undesirable aspect is
    that for an attribute value which could not be determined (due to this
    problem), it will say that it was "undefined", which isn't entirely
    accurate.

    My initial patch changed value to be a string to be "could not determine
    value", but if the line of code looked like this:
    print 'B:', weird.b.upper()

    Then for something which shouldn't work, it would determine that the
    value of weird.b.upper is the string method - which isn't what we're after.

    So I would recommend my original patch (as described above) as the best
    solution.

    @amc1
    Copy link
    Mannequin Author

    amc1 mannequin commented Dec 29, 2008

    In the interests of getting this fixed (and not letting it die), should
    I submit a proper patch? I suppose I would have to do one for each
    version of Python that is affected (which is all of them, really).

    @ggenellina
    Copy link
    Mannequin

    ggenellina mannequin commented Dec 30, 2008

    I believe a patch against the trunk would be enough, but should include
    a test case.

    @arthurpetitpierre
    Copy link
    Mannequin

    arthurpetitpierre mannequin commented Dec 6, 2012

    I attached a patch containing both the fix suggested by Allan and a test case.
    Tested against trunk and python2.7.

    @iritkatriel
    Copy link
    Member

    The issue still occurs in 3.10. Python 3 version of the script:

    import cgitb
    
    class WeirdObject(object):
        def __getattr__(self, attr):
            if attr == 'a':
                return 'the letter a'
            elif attr == 'b':
                return str(slf) # Intentional NameError
            raise AttributeError(attr)

    try:
    weird = WeirdObject()
    print('A:', weird.a)
    print('B:', weird.b)
    print('C:', weird.c)
    except Exception as e:
    import sys
    print('\nSomething went wrong - attempting to generate HTML stack trace.')
    try:
    html_text = cgitb.html(sys.exc_info())
    except:
    print('Error generating HTML stack trace!')
    raise
    else:
    print('Here is stack trace in HTML:\n', html_text)

    @iritkatriel iritkatriel added 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes labels Nov 15, 2020
    @iritkatriel
    Copy link
    Member

    Arthur, are you interested in converting your patch to a github pull request?

    @karlcow
    Copy link
    Mannequin

    karlcow mannequin commented Jan 1, 2021

    Converted into GitHub PR #24038

    @iritkatriel
    Copy link
    Member

    On closer scrutiny I'm not sure this patch should be merged.

    The getattr call here has a default value, so it should not raise AttributeError. It should also not raise any other exception because a valid implementation of __getattr__ should raise only AttributeError:

    https://docs.python.org/3/reference/datamodel.html#object.\_\_getattr__

    @karlcow
    Copy link
    Mannequin

    karlcow mannequin commented Jan 2, 2021

    The getattr call here has a default value, so it should not raise AttributeError. It should also not raise any other exception because a valid implementation of __getattr__ should raise only AttributeError:

    but probably the intent of the patch here is to surface a meaningful error with regards to the script, more than an error on the cgitb python lib itself as this is a traceback tool. A bit like an HTML validator which continue to process things to give some kind of meaning.

    Diving into previous issues about scanvars
    https://bugs.python.org/issue966992

    Similar
    https://bugs.python.org/issue1047397 The last comment is basically this issue here.

    There is a patch which is lot better for this bug and which addresses the issues here.
    #15094

    It has not been merged yet. Not sure why or if there is anything missing.

    Probably this bug could be closed as a duplicate of https://bugs.python.org/issue966992

    And someone needs to push to push the latest bits for #15094

    What do you think @iritkatriel ?

    I will close my PR.

    @iritkatriel
    Copy link
    Member

    I agree that this can be closed as duplicate. Thanks, Karl, for the research.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants