You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Dayo Esho (JIRA)" <ji...@apache.org> on 2009/07/22 03:34:14 UTC
[jira] Created: (THRIFT-547) Thrift deserializer hangs when
deserializing empty string
Thrift deserializer hangs when deserializing empty string
---------------------------------------------------------
Key: THRIFT-547
URL: https://issues.apache.org/jira/browse/THRIFT-547
Project: Thrift
Issue Type: Bug
Components: Library (Ruby)
Affects Versions: 0.1
Environment: ruby 1.8.6
Reporter: Dayo Esho
Priority: Minor
Expect this to throw an error on empty strings and any other strings that cannot be deserialized. Here is some code to reproduce:
require 'thrift'
class MyClass
include ::Thrift::Struct
FIELDS = {}
def struct_fields; FIELDS; end
def validate; end
end
deserializer = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new)
deserializer.deserialize(MyClass.new, '') ###### hangs
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-547) Thrift deserializer hangs when
deserializing empty string
Posted by "Kevin Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12737007#action_12737007 ]
Kevin Clark commented on THRIFT-547:
------------------------------------
There's a ruby-debug require in base_transport_spec that needs to be removed.
Also, I think maybe we're raising too early. I think if you ask for more data than exists, the data that can be fetched should be returned, just like transports on the wire. Once we get to 0 bytes though, I agree, we should be raising. eg:
# assume transport has "foobar" in it
@transport.read(10) # => "foobar"
@transport.available == 0 # => true
@transport.read(10) # raises
That way if a user is just asking for the most data they'd like to store (just give me up to 1k, 4k, etc) things work as expected, but once the buffer is empty we get the exception that's expected. What do you think?
> Thrift deserializer hangs when deserializing empty string
> ---------------------------------------------------------
>
> Key: THRIFT-547
> URL: https://issues.apache.org/jira/browse/THRIFT-547
> Project: Thrift
> Issue Type: Bug
> Components: Library (Ruby)
> Affects Versions: 0.1
> Environment: ruby 1.8.6
> Reporter: Dayo Esho
> Assignee: Bryan Duxbury
> Priority: Minor
> Attachments: thrift-547.patch
>
>
> Expect this to throw an error on empty strings and any other strings that cannot be deserialized. Here is some code to reproduce:
> require 'thrift'
> class MyClass
> include ::Thrift::Struct
> FIELDS = {}
> def struct_fields; FIELDS; end
> def validate; end
> end
> deserializer = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new)
> deserializer.deserialize(MyClass.new, '') ###### hangs
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-547) Thrift deserializer hangs when
deserializing empty string
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-547?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-547:
---------------------------------
Patch Info: [Patch Available]
> Thrift deserializer hangs when deserializing empty string
> ---------------------------------------------------------
>
> Key: THRIFT-547
> URL: https://issues.apache.org/jira/browse/THRIFT-547
> Project: Thrift
> Issue Type: Bug
> Components: Library (Ruby)
> Affects Versions: 0.1
> Environment: ruby 1.8.6
> Reporter: Dayo Esho
> Assignee: Bryan Duxbury
> Priority: Minor
> Attachments: thrift-547.patch
>
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> Expect this to throw an error on empty strings and any other strings that cannot be deserialized. Here is some code to reproduce:
> require 'thrift'
> class MyClass
> include ::Thrift::Struct
> FIELDS = {}
> def struct_fields; FIELDS; end
> def validate; end
> end
> deserializer = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new)
> deserializer.deserialize(MyClass.new, '') ###### hangs
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-547) Thrift deserializer hangs when
deserializing empty string
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12737150#action_12737150 ]
Bryan Duxbury commented on THRIFT-547:
--------------------------------------
I see what you are saying. I'm thinking it's just not really useful in this circumstance though. All Thrift transports are tailor-made for serialization and deserialization, and Thrift structs don't just pull in a big blob of data from a memory transport - they get exactly the number of bytes they expect to be there. The first time there is an underrun, it's fatal. If we raised only once there was finally exactly zero bytes available, it wouldn't change anything. The error would just come on the second iteration of read_all instead of the first.
> Thrift deserializer hangs when deserializing empty string
> ---------------------------------------------------------
>
> Key: THRIFT-547
> URL: https://issues.apache.org/jira/browse/THRIFT-547
> Project: Thrift
> Issue Type: Bug
> Components: Library (Ruby)
> Affects Versions: 0.1
> Environment: ruby 1.8.6
> Reporter: Dayo Esho
> Assignee: Bryan Duxbury
> Priority: Minor
> Attachments: thrift-547.patch
>
>
> Expect this to throw an error on empty strings and any other strings that cannot be deserialized. Here is some code to reproduce:
> require 'thrift'
> class MyClass
> include ::Thrift::Struct
> FIELDS = {}
> def struct_fields; FIELDS; end
> def validate; end
> end
> deserializer = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new)
> deserializer.deserialize(MyClass.new, '') ###### hangs
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-547) Thrift deserializer hangs when
deserializing empty string
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-547?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-547:
---------------------------------
Attachment: thrift-547.patch
This patch solves the problem. If the MemoryBuffer can't return the number of bytes requested, you get an EOFError now, which prevents the endless looping.
> Thrift deserializer hangs when deserializing empty string
> ---------------------------------------------------------
>
> Key: THRIFT-547
> URL: https://issues.apache.org/jira/browse/THRIFT-547
> Project: Thrift
> Issue Type: Bug
> Components: Library (Ruby)
> Affects Versions: 0.1
> Environment: ruby 1.8.6
> Reporter: Dayo Esho
> Assignee: Bryan Duxbury
> Priority: Minor
> Attachments: thrift-547.patch
>
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> Expect this to throw an error on empty strings and any other strings that cannot be deserialized. Here is some code to reproduce:
> require 'thrift'
> class MyClass
> include ::Thrift::Struct
> FIELDS = {}
> def struct_fields; FIELDS; end
> def validate; end
> end
> deserializer = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new)
> deserializer.deserialize(MyClass.new, '') ###### hangs
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (THRIFT-547) Thrift deserializer hangs when
deserializing empty string
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-547?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury resolved THRIFT-547.
----------------------------------
Resolution: Fixed
Ok, I removed the ruby-debug require and committed. Thanks for the review. I hope the way I fixed this issue won't come up again :)
> Thrift deserializer hangs when deserializing empty string
> ---------------------------------------------------------
>
> Key: THRIFT-547
> URL: https://issues.apache.org/jira/browse/THRIFT-547
> Project: Thrift
> Issue Type: Bug
> Components: Library (Ruby)
> Affects Versions: 0.1
> Environment: ruby 1.8.6
> Reporter: Dayo Esho
> Assignee: Bryan Duxbury
> Priority: Minor
> Attachments: thrift-547.patch
>
>
> Expect this to throw an error on empty strings and any other strings that cannot be deserialized. Here is some code to reproduce:
> require 'thrift'
> class MyClass
> include ::Thrift::Struct
> FIELDS = {}
> def struct_fields; FIELDS; end
> def validate; end
> end
> deserializer = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new)
> deserializer.deserialize(MyClass.new, '') ###### hangs
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (THRIFT-547) Thrift deserializer hangs
when deserializing empty string
Posted by "Kevin Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12737007#action_12737007 ]
Kevin Clark edited comment on THRIFT-547 at 7/29/09 10:52 PM:
--------------------------------------------------------------
There's a ruby-debug require in base_transport_spec that needs to be removed.
Also, I think maybe we're raising too early. I think if you ask for more data than exists, the data that can be fetched should be returned, just like transports on the wire. Once we get to 0 bytes though, I agree, we should be raising. eg:
assume transport has "foobar" in it
@transport.read(10) # => "foobar"
@transport.available == 0 # => true
@transport.read(10) # raises
That way if a user is just asking for the most data they'd like to store (just give me up to 1k, 4k, etc) things work as expected, but once the buffer is empty we get the exception that's expected. What do you think?
was (Author: kclark):
There's a ruby-debug require in base_transport_spec that needs to be removed.
Also, I think maybe we're raising too early. I think if you ask for more data than exists, the data that can be fetched should be returned, just like transports on the wire. Once we get to 0 bytes though, I agree, we should be raising. eg:
# assume transport has "foobar" in it
@transport.read(10) # => "foobar"
@transport.available == 0 # => true
@transport.read(10) # raises
That way if a user is just asking for the most data they'd like to store (just give me up to 1k, 4k, etc) things work as expected, but once the buffer is empty we get the exception that's expected. What do you think?
> Thrift deserializer hangs when deserializing empty string
> ---------------------------------------------------------
>
> Key: THRIFT-547
> URL: https://issues.apache.org/jira/browse/THRIFT-547
> Project: Thrift
> Issue Type: Bug
> Components: Library (Ruby)
> Affects Versions: 0.1
> Environment: ruby 1.8.6
> Reporter: Dayo Esho
> Assignee: Bryan Duxbury
> Priority: Minor
> Attachments: thrift-547.patch
>
>
> Expect this to throw an error on empty strings and any other strings that cannot be deserialized. Here is some code to reproduce:
> require 'thrift'
> class MyClass
> include ::Thrift::Struct
> FIELDS = {}
> def struct_fields; FIELDS; end
> def validate; end
> end
> deserializer = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new)
> deserializer.deserialize(MyClass.new, '') ###### hangs
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-547) Thrift deserializer hangs when
deserializing empty string
Posted by "Kevin Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12737318#action_12737318 ]
Kevin Clark commented on THRIFT-547:
------------------------------------
Yeah, the goal is that it doesn't, functionally, change anything, but the behavior is more consistent with the other transports.
But that's not a blocker for me. I think it's more important to get the patch in. I'd +1 it post ruby-debug removal if you feel strongly about it.
> Thrift deserializer hangs when deserializing empty string
> ---------------------------------------------------------
>
> Key: THRIFT-547
> URL: https://issues.apache.org/jira/browse/THRIFT-547
> Project: Thrift
> Issue Type: Bug
> Components: Library (Ruby)
> Affects Versions: 0.1
> Environment: ruby 1.8.6
> Reporter: Dayo Esho
> Assignee: Bryan Duxbury
> Priority: Minor
> Attachments: thrift-547.patch
>
>
> Expect this to throw an error on empty strings and any other strings that cannot be deserialized. Here is some code to reproduce:
> require 'thrift'
> class MyClass
> include ::Thrift::Struct
> FIELDS = {}
> def struct_fields; FIELDS; end
> def validate; end
> end
> deserializer = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new)
> deserializer.deserialize(MyClass.new, '') ###### hangs
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-547) Thrift deserializer hangs when
deserializing empty string
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-547?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-547:
---------------------------------
Remaining Estimate: (was: 2h)
Original Estimate: (was: 2h)
> Thrift deserializer hangs when deserializing empty string
> ---------------------------------------------------------
>
> Key: THRIFT-547
> URL: https://issues.apache.org/jira/browse/THRIFT-547
> Project: Thrift
> Issue Type: Bug
> Components: Library (Ruby)
> Affects Versions: 0.1
> Environment: ruby 1.8.6
> Reporter: Dayo Esho
> Assignee: Bryan Duxbury
> Priority: Minor
> Attachments: thrift-547.patch
>
>
> Expect this to throw an error on empty strings and any other strings that cannot be deserialized. Here is some code to reproduce:
> require 'thrift'
> class MyClass
> include ::Thrift::Struct
> FIELDS = {}
> def struct_fields; FIELDS; end
> def validate; end
> end
> deserializer = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new)
> deserializer.deserialize(MyClass.new, '') ###### hangs
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (THRIFT-547) Thrift deserializer hangs when
deserializing empty string
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-547?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury reassigned THRIFT-547:
------------------------------------
Assignee: Bryan Duxbury
> Thrift deserializer hangs when deserializing empty string
> ---------------------------------------------------------
>
> Key: THRIFT-547
> URL: https://issues.apache.org/jira/browse/THRIFT-547
> Project: Thrift
> Issue Type: Bug
> Components: Library (Ruby)
> Affects Versions: 0.1
> Environment: ruby 1.8.6
> Reporter: Dayo Esho
> Assignee: Bryan Duxbury
> Priority: Minor
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> Expect this to throw an error on empty strings and any other strings that cannot be deserialized. Here is some code to reproduce:
> require 'thrift'
> class MyClass
> include ::Thrift::Struct
> FIELDS = {}
> def struct_fields; FIELDS; end
> def validate; end
> end
> deserializer = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new)
> deserializer.deserialize(MyClass.new, '') ###### hangs
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.