rubyamf / rocketamf

52 stars 34 forks source link

Can't dereference cached objects, strings and traits #5

Closed anon404 closed 12 years ago

anon404 commented 12 years ago

Objects, Traits and Strings aren't cached properly as they're deserialized, and the parser chokes when trying to dereference said elements

deserializer.rb:

36,38c36,38
<           @string_cache = []
<           @object_cache = []
<           @trait_cache = []

---
>           @string_cache = [] unless @string_cache
>           @object_cache = [] unless @object_cache
>           @trait_cache = [] unless @trait_cache
warhammerkid commented 12 years ago

The currently coded behavior is intentional - calling deserialize multiple times on the same stream resets the reference count every time. Can you provide some sample code for the situation you are running into or a binary dump of the stream you're trying to deserialize? The binary dump can be e-mailed to me at the address on my github profile, or you can post a hexdump of it here.

Teshootub7 commented 12 years ago

Hi,

I know this behaviour is intensional. But the current implementation may cause a problem in some situation. For example, the following file won't be deserialized and throws an error.

% hexdump -C req1 
00000000  00 03 00 00 00 01 00 17  44 69 73 70 61 74 63 68  |........Dispatch|
00000010  53 65 72 76 69 63 65 2e  65 78 65 63 75 74 65 00  |Service.execute.|
00000020  02 2f 31 00 00 00 46 0a  00 00 00 02 11 0a 0b 01  |./1...F.........|
00000030  05 72 64 01 07 73 65 71  04 01 01 11 09 03 01 0a  |.rd..seq........|
00000040  01 17 73 65 72 76 69 63  65 4e 61 6d 65 06 2d 55  |..serviceName.-U|
00000050  73 65 72 53 65 72 76 69  63 65 2e 5f 67 65 74 50  |serService._getP|
00000060  6c 61 79 65 72 0b 70 61  72 61 6d 01 01           |layer.param..|
0000006d
% ruby1.9.1 -I rocketamf-git/lib -r rocketamf -e 'RocketAMF::Envelope.new.populate_from_stream(IO.read("req1"))'
/home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:350:in `amf3_read_object': undefined method `[]' for nil:NilClass (NoMethodError)
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:197:in `amf3_deserialize'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:311:in `block in amf3_read_array'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:311:in `upto'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:311:in `amf3_read_array'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:195:in `amf3_deserialize'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:39:in `deserialize'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:87:in `amf0_deserialize'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:120:in `block in amf0_read_array'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:119:in `upto'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:119:in `amf0_read_array'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:75:in `amf0_deserialize'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/deserializer.rb:34:in `deserialize'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/remoting.rb:47:in `block in populate_from_stream'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/remoting.rb:39:in `upto'
        from /home/qw/tmp/rocketamf-git/lib/rocketamf/pure/remoting.rb:39:in `populate_from_stream'
        from -e:1:in `<main>'

After some investigations for the source code of RocketAMF, I noticed that the cache variables are wrongly cleared, so references couldn't be dereferenced. In amf0_deserialize, deserialize is called when AMF0_AMF3_MARKER is found.

  def amf0_deserialize type=nil
    type = read_int8 @source unless type
    case type
    [...]
    when AMF0_AMF3_MARKER
      deserialize(3, nil)

Since deserialize reset the caches, dereferences afterward won't work properly.

  def deserialize version, source
  [...]
      @string_cache = []
      @object_cache = []
      @trait_cache = []

I think amf0_deserialize should call amf3_deserialize instead of deserialize when it found AMF0_AMF3_MARKER.

Additionally, the current deserialize initializes the variables @string_cache, @object_cache and @trait_cache only when the method is called with version = 3. Since AMF0_AMF3_MARKER switches from AMF-0 to AMF-3, we should initialize these variables not only version = 3 but version = 0.

I made a patch to fix the problem.

diff --git a/lib/rocketamf/pure/deserializer.rb b/lib/rocketamf/pure/deserializer.rb
index ba71865..bb4d1ce 100644
--- a/lib/rocketamf/pure/deserializer.rb
+++ b/lib/rocketamf/pure/deserializer.rb
@@ -29,13 +29,13 @@ module RocketAMF
           raise AMFError, "no source to deserialize"
         end

+        @string_cache = []
+        @object_cache = []
+        @trait_cache = []
         if @version == 0
           @ref_cache = []
           return amf0_deserialize
         else
-          @string_cache = []
-          @object_cache = []
-          @trait_cache = []
           return amf3_deserialize
         end
       end
@@ -84,7 +84,7 @@ module RocketAMF
         when AMF0_TYPED_OBJECT_MARKER
           amf0_read_typed_object
         when AMF0_AMF3_MARKER
-          deserialize(3, nil)
+          amf3_deserialize
         else
           raise AMFError, "Invalid type: #{type}"
         end

Regards,