[Patches] #100874: prescod -> prescod [Rejected] Better error message with UnboundLocalError

[Ka-Ping Yee]

http://sourceforge.net/patch/?func=detailpatch&patch_id=100874&group_id=5470

Status: Rejected
Submitted By: prescod
Category: None
Summary: Better error message with UnboundLocalError
Assigned To: prescod
Patch: View Raw Patch
Date: 2000-Jul-12 20:29

2000-Jul-12 21:56 - tim_one wrote:
    Rejected and assigned back to Paul. I like the idea fine, the rejection is for style reasons and a repeated security hole. Please make the code look like the rest of ceval.c -- idiosyncracies (that aren't Guido's <0.3 wink>) are harmful in group-maintained code. Examples:
    
    > PyObject *nm=NULL;
    
    Should be a space on each side of "=". Ditto throughout the patch. BTW, is "nm" meant to be an abbreviation for "name"? If so, please spell out the two extra letters. If it's meant to be an abbreviation for New Mexico, capitalize it <wink>.
    
    > if(!nm) break;
    
    "if" isn't a function call: use a space between "if" and "(". It's also bad to slop "break" on the same line: makes it much harder to set a useful breakpoint here in a debugger. Everywhere else in the code this line would be spelled (if Guido wrote it -- and the "\t" business here is because SF comments don't preserve indentation):
    
    \tif (!nm)
    \t\tbreak
    
    > sprintf( buf, "Local variable %s
    
    No space between "(" and "buf" here.
    
    > PyExc_UnboundLocalError, buf );
    
    No space between "buf" and ")" here.
    
    > char buf[500];
    > ... followed by an unchecked sprintf into buf.
    
    This is a security hole: Python doesn't restrict identifiers to being no longer than 500 chars, so a hacker can easily trick this code into overwriting the buffer bounds. Easy but tedious to fix (e.g., #define the buf length, and use runtime code in conjunction with strncpy to guarantee buf's bounds are respected).

2000-Jul-12 21:56 - tim_one changed patch_status_id (previously Open)
2000-Jul-12 21:56 - tim_one changed assigned_to (previously tim_one)
2000-Jul-12 20:50 - prescod changed assigned_to (previously none)
2000-Jul-12 20:50 - prescod changed summary (previously Better error message with UnboundLocalError (for Tim Peters))