Bugzilla – Bug 4061
Bug in implementation of native function read_flags.
Last modified: 2013-06-16 01:49:15 PDT
Created attachment 1758 [details] [review] Proposed patch By default meaning "flag" is a letter from 'a' to 'z', but function read_flags tries to convert any char from 1 to 255 into flag. Proposed patch adds check for char is in range from 'a' to 'z'. Please check it because i didn't tried to compile with it.
The allowed input range is undefined, this patch would make it explicit. Probably harmless, I suspect both that: 1. this bug doesn't really effect anyone 2. the patch would not regress compatibility
I didn't understand the conclusion: will you confirm it or not? Also have a look at get_flags function implementation - it converts int to chars flags and it checks only for 0 to 25 bits (i.e. 'a' to 'z'). So may be read_flags should be opposite? Because now you can send "!$614^!46" to read_flags and it will return some int value. If then you will pass that value to get_flags you will receive some flags set.
reread comment #1 read_flags does not specify valid inputs or outputs. so technically the functionality could be seen as legal, and abused or relied on in some way by plugins. on the other hand, it's unlikely. for both of these reasons i think it's okay to take the patch, and that the bug is very minor. but i still want to verify the potential compatibility impact.
Comment on attachment 1758 [details] [review] Proposed patch Given the compatibility risk, I'll let this sit for 1.8.2. Flipping review? to arkshine.
Comment on attachment 1758 [details] [review] Proposed patch 1) The patch is wrong, it may cause problems (i.e: infinite loop if character is not between 'a' and 'z' since *c would not be increased) 2) The patch will broke non-conventional usage of the native, even if such usage is unlikely, there is still a potential risk considering how the native is often used. I would conclude the native doesn't require fixing. It would be safe to keep the actual behavior.