Re: [MacRuby-devel] [macruby-changes] [5186] MacRuby/trunk/hash.h
Hi Watson, This isn't good, rhash_modify() must be very fast, so calling OBJ_FROZEN and OBJ_UNTRUSTED is not good there. Can we look up the mask flag as before? Laurent On Jan 21, 2011, at 7:51 AM, source_changes@macosforge.org wrote:
Revision 5186 Author watson1978@gmail.com Date 2011-01-21 07:51:01 -0800 (Fri, 21 Jan 2011) Log Message
More method of Hash will throw a SecurityError when $SAFE is 4.
Test Script: {{{ h = {} $SAFE = 4 h['a'] = 1.0 }}} Modified Paths
MacRuby/trunk/hash.h Diff
Modified: MacRuby/trunk/hash.h (5185 => 5186)
--- MacRuby/trunk/hash.h 2011-01-21 02:20:08 UTC (rev 5185) +++ MacRuby/trunk/hash.h 2011-01-21 15:51:01 UTC (rev 5186) @@ -41,14 +41,11 @@ static inline void rhash_modify(VALUE hash) { - const long mask = RBASIC(hash)->flags; - if ((mask & FL_FREEZE) == FL_FREEZE) { - rb_raise(rb_eRuntimeError, "can't modify frozen/immutable hash"); + if (OBJ_FROZEN(hash)) { + rb_error_frozen("hash"); } - if ((mask & FL_TAINT) == FL_TAINT) { - if (rb_safe_level() >= 4) { - rb_raise(rb_eSecurityError, "Insecure: can't modify hash"); - } + if (!OBJ_UNTRUSTED(hash) && rb_safe_level() >= 4) { + rb_raise(rb_eSecurityError, "Insecure: can't modify hash"); } }
_______________________________________________ macruby-changes mailing list macruby-changes@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-changes
Hi Laurent, There is only the extremely slight difference. I think that I can ignore this. How do you think? I attached a patch and benchmark script. ** Result of r5185 $ macruby bm_hash.rb Rehearsal ------------------------------------ 13.640000 0.210000 13.850000 ( 9.114591) -------------------------- total: 13.850000sec user system total real 13.720000 0.240000 13.960000 ( 9.172110) ** Result of r5186 $ macruby bm_hash.rb Rehearsal ------------------------------------ 13.660000 0.210000 13.870000 ( 9.239979) -------------------------- total: 13.870000sec user system total real 13.850000 0.220000 14.070000 ( 9.412910) ** Result of appling attached patch $ macruby bm_hash.rb Rehearsal ------------------------------------ 13.660000 0.200000 13.860000 ( 9.171733) -------------------------- total: 13.860000sec user system total real 13.790000 0.220000 14.010000 ( 9.271439) 2011/1/22 Laurent Sansonetti <lsansonetti@apple.com>:
Hi Watson, This isn't good, rhash_modify() must be very fast, so calling OBJ_FROZEN and OBJ_UNTRUSTED is not good there. Can we look up the mask flag as before? Laurent On Jan 21, 2011, at 7:51 AM, source_changes@macosforge.org wrote:
Revision 5186 Author watson1978@gmail.com Date 2011-01-21 07:51:01 -0800 (Fri, 21 Jan 2011)
Log Message
More method of Hash will throw a SecurityError when $SAFE is 4.
Test Script: {{{ h = {} $SAFE = 4 h['a'] = 1.0 }}}
Modified Paths
MacRuby/trunk/hash.h
Diff
Modified: MacRuby/trunk/hash.h (5185 => 5186)
--- MacRuby/trunk/hash.h 2011-01-21 02:20:08 UTC (rev 5185) +++ MacRuby/trunk/hash.h 2011-01-21 15:51:01 UTC (rev 5186) @@ -41,14 +41,11 @@ static inline void rhash_modify(VALUE hash) { - const long mask = RBASIC(hash)->flags; - if ((mask & FL_FREEZE) == FL_FREEZE) { - rb_raise(rb_eRuntimeError, "can't modify frozen/immutable hash"); + if (OBJ_FROZEN(hash)) { + rb_error_frozen("hash"); } - if ((mask & FL_TAINT) == FL_TAINT) { - if (rb_safe_level() >= 4) { - rb_raise(rb_eSecurityError, "Insecure: can't modify hash"); - } + if (!OBJ_UNTRUSTED(hash) && rb_safe_level() >= 4) { + rb_raise(rb_eSecurityError, "Insecure: can't modify hash"); } }
_______________________________________________ macruby-changes mailing list macruby-changes@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-changes
_______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
Hi Watson, Sorry for late response. I think your benchmark code contains too much side effects (such as the allocation of 'a' string and hash internals during rehash). If you just benchmark h[42]=42 you should see a difference. Anyways, I corrected the change to use the mask flag in r5187. Laurent On Jan 21, 2011, at 3:09 PM, Watson wrote:
Hi Laurent,
There is only the extremely slight difference. I think that I can ignore this. How do you think?
I attached a patch and benchmark script.
** Result of r5185 $ macruby bm_hash.rb Rehearsal ------------------------------------ 13.640000 0.210000 13.850000 ( 9.114591) -------------------------- total: 13.850000sec
user system total real 13.720000 0.240000 13.960000 ( 9.172110)
** Result of r5186 $ macruby bm_hash.rb Rehearsal ------------------------------------ 13.660000 0.210000 13.870000 ( 9.239979) -------------------------- total: 13.870000sec
user system total real 13.850000 0.220000 14.070000 ( 9.412910)
** Result of appling attached patch $ macruby bm_hash.rb Rehearsal ------------------------------------ 13.660000 0.200000 13.860000 ( 9.171733) -------------------------- total: 13.860000sec
user system total real 13.790000 0.220000 14.010000 ( 9.271439)
2011/1/22 Laurent Sansonetti <lsansonetti@apple.com>:
Hi Watson, This isn't good, rhash_modify() must be very fast, so calling OBJ_FROZEN and OBJ_UNTRUSTED is not good there. Can we look up the mask flag as before? Laurent On Jan 21, 2011, at 7:51 AM, source_changes@macosforge.org wrote:
Revision 5186 Author watson1978@gmail.com Date 2011-01-21 07:51:01 -0800 (Fri, 21 Jan 2011)
Log Message
More method of Hash will throw a SecurityError when $SAFE is 4.
Test Script: {{{ h = {} $SAFE = 4 h['a'] = 1.0 }}}
Modified Paths
MacRuby/trunk/hash.h
Diff
Modified: MacRuby/trunk/hash.h (5185 => 5186)
--- MacRuby/trunk/hash.h 2011-01-21 02:20:08 UTC (rev 5185) +++ MacRuby/trunk/hash.h 2011-01-21 15:51:01 UTC (rev 5186) @@ -41,14 +41,11 @@ static inline void rhash_modify(VALUE hash) { - const long mask = RBASIC(hash)->flags; - if ((mask & FL_FREEZE) == FL_FREEZE) { - rb_raise(rb_eRuntimeError, "can't modify frozen/immutable hash"); + if (OBJ_FROZEN(hash)) { + rb_error_frozen("hash"); } - if ((mask & FL_TAINT) == FL_TAINT) { - if (rb_safe_level() >= 4) { - rb_raise(rb_eSecurityError, "Insecure: can't modify hash"); - } + if (!OBJ_UNTRUSTED(hash) && rb_safe_level() >= 4) { + rb_raise(rb_eSecurityError, "Insecure: can't modify hash"); } }
_______________________________________________ macruby-changes mailing list macruby-changes@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-changes
_______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
<modify_check_patch.txt><bm_hash.rb>_______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
participants (2)
-
Laurent Sansonetti
-
Watson