[jira] [Commented] (THRIFT-4326) Ruby BufferedTransport not safe for reuse after reading corrupted input

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165759#comment-16165759
 ] 

ASF GitHub Bot commented on THRIFT-4326:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1352
  
Of course now I see job 2 is failing in nodejs as well.  Spectacular!  
Well, let's see how this fares.  If we can get job 4 to pass that would be 
ideal.  If this one doesn't work I'll run it locally myself in docker and 
validate it before merging.


> Ruby BufferedTransport not safe for reuse after reading corrupted input
> ---
>
> Key: THRIFT-4326
> URL: https://issues.apache.org/jira/browse/THRIFT-4326
> Project: Thrift
>  Issue Type: Bug
>  Components: Ruby - Library
>Affects Versions: 0.10.0
> Environment: Originally observed with Thrift 0.9.3 on Linux with Ruby 
> 2.3.4, but have also reproduced on Mac OS X with Thrift 0.10.0.
>Reporter: Ben Weintraub
>
> We've experimented with the Ruby {{BufferedTransport}} class as a wrapper 
> around the {{HttpClientTransport}} class, and found that we were getting 
> clusters sporadic {{Thrift::ProtocolException}} errors in Ruby client 
> processes after network issues caused corruption of some Thrift response 
> bodies.
> Using a bare {{HttpClientTransport}} makes these issues disappear.
> For a given service, we retain a long-lived protocol instance 
> ({{CompactProtocol}} in our case), which in turn holds a reference to a 
> long-lived {{BufferedTransport}} instance.
> The problem seems to stem from the case where the Thrift client is 
> interrupted (e.g. by a Ruby timeout exception) before consuming to the end of 
> the {{@rbuf}} instance variable in {{BufferedTransport}}, leaving {{@index}} 
> pointing to the middle of the read buffer, and meaning that when the 
> transport is re-used upon the next service call, the {{BufferedTransport}} 
> continues reading where it left off in the old buffer, rather than calling 
> through to the wrapped {{HttpClientTransport}} to read the new response 
> obtained from the last call to {{#flush}}.
> Now I know {{Timeout}} is fundamentally unsafe in Ruby and can lead to all 
> kinds of issues like this, but I've also found that this same issue can be 
> triggered by another fairly plausible scenario: if the Thrift service returns 
> a well-formed Thrift response but with N extra bytes of garbage tacked onto 
> the end, then the next N following service calls through the same 
> {{BufferedTransport}} instance will fail with a 
> {{Thrift::ProtocolException}}, as the {{BufferedTransport}} will continue 
> attempting to read the left-over bytes in {{@rbuf}}.
> The naive solution seems like it would be to just reset {{@rbuf}} from 
> {{#flush}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4326) Ruby BufferedTransport not safe for reuse after reading corrupted input

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165758#comment-16165758
 ] 

ASF GitHub Bot commented on THRIFT-4326:


Github user benweint commented on the issue:

https://github.com/apache/thrift/pull/1352
  
Yup, done. No worries - I know how big hairy test suites go :)


> Ruby BufferedTransport not safe for reuse after reading corrupted input
> ---
>
> Key: THRIFT-4326
> URL: https://issues.apache.org/jira/browse/THRIFT-4326
> Project: Thrift
>  Issue Type: Bug
>  Components: Ruby - Library
>Affects Versions: 0.10.0
> Environment: Originally observed with Thrift 0.9.3 on Linux with Ruby 
> 2.3.4, but have also reproduced on Mac OS X with Thrift 0.10.0.
>Reporter: Ben Weintraub
>
> We've experimented with the Ruby {{BufferedTransport}} class as a wrapper 
> around the {{HttpClientTransport}} class, and found that we were getting 
> clusters sporadic {{Thrift::ProtocolException}} errors in Ruby client 
> processes after network issues caused corruption of some Thrift response 
> bodies.
> Using a bare {{HttpClientTransport}} makes these issues disappear.
> For a given service, we retain a long-lived protocol instance 
> ({{CompactProtocol}} in our case), which in turn holds a reference to a 
> long-lived {{BufferedTransport}} instance.
> The problem seems to stem from the case where the Thrift client is 
> interrupted (e.g. by a Ruby timeout exception) before consuming to the end of 
> the {{@rbuf}} instance variable in {{BufferedTransport}}, leaving {{@index}} 
> pointing to the middle of the read buffer, and meaning that when the 
> transport is re-used upon the next service call, the {{BufferedTransport}} 
> continues reading where it left off in the old buffer, rather than calling 
> through to the wrapped {{HttpClientTransport}} to read the new response 
> obtained from the last call to {{#flush}}.
> Now I know {{Timeout}} is fundamentally unsafe in Ruby and can lead to all 
> kinds of issues like this, but I've also found that this same issue can be 
> triggered by another fairly plausible scenario: if the Thrift service returns 
> a well-formed Thrift response but with N extra bytes of garbage tacked onto 
> the end, then the next N following service calls through the same 
> {{BufferedTransport}} instance will fail with a 
> {{Thrift::ProtocolException}}, as the {{BufferedTransport}} will continue 
> attempting to read the left-over bytes in {{@rbuf}}.
> The naive solution seems like it would be to just reset {{@rbuf}} from 
> {{#flush}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift issue #1352: THRIFT-4326 Allow safer reuse of BufferedTransport insta...

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1352
  
Of course now I see job 2 is failing in nodejs as well.  Spectacular!  
Well, let's see how this fares.  If we can get job 4 to pass that would be 
ideal.  If this one doesn't work I'll run it locally myself in docker and 
validate it before merging.


---

[GitHub] thrift issue #1352: THRIFT-4326 Allow safer reuse of BufferedTransport insta...

[benweint]

Github user benweint commented on the issue:

https://github.com/apache/thrift/pull/1352
  
Yup, done. No worries - I know how big hairy test suites go :)


---

[jira] [Commented] (THRIFT-4326) Ruby BufferedTransport not safe for reuse after reading corrupted input

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165757#comment-16165757
 ] 

ASF GitHub Bot commented on THRIFT-4326:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1352
  
If you could rebase one more time and force push, I found out why build 
jobs 3 and 4 became unstable - I moved nodejs testing from job 3 to 4 in my 
last PR, and I just undid that.  Sorry about all this...


> Ruby BufferedTransport not safe for reuse after reading corrupted input
> ---
>
> Key: THRIFT-4326
> URL: https://issues.apache.org/jira/browse/THRIFT-4326
> Project: Thrift
>  Issue Type: Bug
>  Components: Ruby - Library
>Affects Versions: 0.10.0
> Environment: Originally observed with Thrift 0.9.3 on Linux with Ruby 
> 2.3.4, but have also reproduced on Mac OS X with Thrift 0.10.0.
>Reporter: Ben Weintraub
>
> We've experimented with the Ruby {{BufferedTransport}} class as a wrapper 
> around the {{HttpClientTransport}} class, and found that we were getting 
> clusters sporadic {{Thrift::ProtocolException}} errors in Ruby client 
> processes after network issues caused corruption of some Thrift response 
> bodies.
> Using a bare {{HttpClientTransport}} makes these issues disappear.
> For a given service, we retain a long-lived protocol instance 
> ({{CompactProtocol}} in our case), which in turn holds a reference to a 
> long-lived {{BufferedTransport}} instance.
> The problem seems to stem from the case where the Thrift client is 
> interrupted (e.g. by a Ruby timeout exception) before consuming to the end of 
> the {{@rbuf}} instance variable in {{BufferedTransport}}, leaving {{@index}} 
> pointing to the middle of the read buffer, and meaning that when the 
> transport is re-used upon the next service call, the {{BufferedTransport}} 
> continues reading where it left off in the old buffer, rather than calling 
> through to the wrapped {{HttpClientTransport}} to read the new response 
> obtained from the last call to {{#flush}}.
> Now I know {{Timeout}} is fundamentally unsafe in Ruby and can lead to all 
> kinds of issues like this, but I've also found that this same issue can be 
> triggered by another fairly plausible scenario: if the Thrift service returns 
> a well-formed Thrift response but with N extra bytes of garbage tacked onto 
> the end, then the next N following service calls through the same 
> {{BufferedTransport}} instance will fail with a 
> {{Thrift::ProtocolException}}, as the {{BufferedTransport}} will continue 
> attempting to read the left-over bytes in {{@rbuf}}.
> The naive solution seems like it would be to just reset {{@rbuf}} from 
> {{#flush}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift issue #1352: THRIFT-4326 Allow safer reuse of BufferedTransport insta...

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1352
  
If you could rebase one more time and force push, I found out why build 
jobs 3 and 4 became unstable - I moved nodejs testing from job 3 to 4 in my 
last PR, and I just undid that.  Sorry about all this...


---

[jira] [Commented] (THRIFT-4326) Ruby BufferedTransport not safe for reuse after reading corrupted input

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165724#comment-16165724
 ] 

ASF GitHub Bot commented on THRIFT-4326:


Github user benweint commented on the issue:

https://github.com/apache/thrift/pull/1352
  
Sure thing - I've done that and it looks like it did kick off another build.


> Ruby BufferedTransport not safe for reuse after reading corrupted input
> ---
>
> Key: THRIFT-4326
> URL: https://issues.apache.org/jira/browse/THRIFT-4326
> Project: Thrift
>  Issue Type: Bug
>  Components: Ruby - Library
>Affects Versions: 0.10.0
> Environment: Originally observed with Thrift 0.9.3 on Linux with Ruby 
> 2.3.4, but have also reproduced on Mac OS X with Thrift 0.10.0.
>Reporter: Ben Weintraub
>
> We've experimented with the Ruby {{BufferedTransport}} class as a wrapper 
> around the {{HttpClientTransport}} class, and found that we were getting 
> clusters sporadic {{Thrift::ProtocolException}} errors in Ruby client 
> processes after network issues caused corruption of some Thrift response 
> bodies.
> Using a bare {{HttpClientTransport}} makes these issues disappear.
> For a given service, we retain a long-lived protocol instance 
> ({{CompactProtocol}} in our case), which in turn holds a reference to a 
> long-lived {{BufferedTransport}} instance.
> The problem seems to stem from the case where the Thrift client is 
> interrupted (e.g. by a Ruby timeout exception) before consuming to the end of 
> the {{@rbuf}} instance variable in {{BufferedTransport}}, leaving {{@index}} 
> pointing to the middle of the read buffer, and meaning that when the 
> transport is re-used upon the next service call, the {{BufferedTransport}} 
> continues reading where it left off in the old buffer, rather than calling 
> through to the wrapped {{HttpClientTransport}} to read the new response 
> obtained from the last call to {{#flush}}.
> Now I know {{Timeout}} is fundamentally unsafe in Ruby and can lead to all 
> kinds of issues like this, but I've also found that this same issue can be 
> triggered by another fairly plausible scenario: if the Thrift service returns 
> a well-formed Thrift response but with N extra bytes of garbage tacked onto 
> the end, then the next N following service calls through the same 
> {{BufferedTransport}} instance will fail with a 
> {{Thrift::ProtocolException}}, as the {{BufferedTransport}} will continue 
> attempting to read the left-over bytes in {{@rbuf}}.
> The naive solution seems like it would be to just reset {{@rbuf}} from 
> {{#flush}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift issue #1352: THRIFT-4326 Allow safer reuse of BufferedTransport insta...

[benweint]

Github user benweint commented on the issue:

https://github.com/apache/thrift/pull/1352
  
Sure thing - I've done that and it looks like it did kick off another build.


---

[jira] [Commented] (THRIFT-2289) Erlang impl of Thrift JSON protocol wrongly writes/expects true/false for bools

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-2289?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165713#comment-16165713
 ] 

ASF GitHub Bot commented on THRIFT-2289:


Github user walter-weinmann commented on the issue:

https://github.com/apache/thrift/pull/1356
  
This closes #1356.


> Erlang impl of Thrift JSON protocol wrongly writes/expects true/false for 
> bools
> ---
>
> Key: THRIFT-2289
> URL: https://issues.apache.org/jira/browse/THRIFT-2289
> Project: Thrift
>  Issue Type: Bug
>  Components: Erlang - Library
>Affects Versions: 0.9.1
>Reporter: Jens Geyer
>
> Some implementations of Thrift JSON protocol wrongly write and expect 
> true/false for bools, instead of 0/1



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift issue #1356: THRIFT-2289 Erlang impl of Thrift JSON protocol wrongly ...

[walter-weinmann]

Github user walter-weinmann commented on the issue:

https://github.com/apache/thrift/pull/1356
  
This closes #1356.


---

[jira] [Commented] (THRIFT-2289) Erlang impl of Thrift JSON protocol wrongly writes/expects true/false for bools

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-2289?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165711#comment-16165711
 ] 

ASF GitHub Bot commented on THRIFT-2289:


GitHub user walter-weinmann opened a pull request:

https://github.com/apache/thrift/pull/1356

THRIFT-2289 Erlang impl of Thrift JSON protocol wrongly writes/expect…

…s true/false for bools.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/walter-weinmann/thrift master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/thrift/pull/1356.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1356


commit 6e83a3d886d9b98874b592fba42d5203a101291e
Author: walter-weinmann 
Date:   2017-09-14T04:16:30Z

THRIFT-2289 Erlang impl of Thrift JSON protocol wrongly writes/expects 
true/false for bools.




> Erlang impl of Thrift JSON protocol wrongly writes/expects true/false for 
> bools
> ---
>
> Key: THRIFT-2289
> URL: https://issues.apache.org/jira/browse/THRIFT-2289
> Project: Thrift
>  Issue Type: Bug
>  Components: Erlang - Library
>Affects Versions: 0.9.1
>Reporter: Jens Geyer
>
> Some implementations of Thrift JSON protocol wrongly write and expect 
> true/false for bools, instead of 0/1



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift pull request #1356: THRIFT-2289 Erlang impl of Thrift JSON protocol w...

[walter-weinmann]

GitHub user walter-weinmann opened a pull request:

https://github.com/apache/thrift/pull/1356

THRIFT-2289 Erlang impl of Thrift JSON protocol wrongly writes/expect…

…s true/false for bools.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/walter-weinmann/thrift master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/thrift/pull/1356.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1356


commit 6e83a3d886d9b98874b592fba42d5203a101291e
Author: walter-weinmann 
Date:   2017-09-14T04:16:30Z

THRIFT-2289 Erlang impl of Thrift JSON protocol wrongly writes/expects 
true/false for bools.




---

[jira] [Commented] (THRIFT-2289) Erlang impl of Thrift JSON protocol wrongly writes/expects true/false for bools

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-2289?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165705#comment-16165705
 ] 

ASF GitHub Bot commented on THRIFT-2289:


Github user walter-weinmann closed the pull request at:

https://github.com/apache/thrift/pull/1349


> Erlang impl of Thrift JSON protocol wrongly writes/expects true/false for 
> bools
> ---
>
> Key: THRIFT-2289
> URL: https://issues.apache.org/jira/browse/THRIFT-2289
> Project: Thrift
>  Issue Type: Bug
>  Components: Erlang - Library
>Affects Versions: 0.9.1
>Reporter: Jens Geyer
>
> Some implementations of Thrift JSON protocol wrongly write and expect 
> true/false for bools, instead of 0/1



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift pull request #1349: THRIFT-2289: Erlang impl of Thrift JSON protocol ...

[walter-weinmann]

Github user walter-weinmann closed the pull request at:

https://github.com/apache/thrift/pull/1349


---

[jira] [Commented] (THRIFT-4326) Ruby BufferedTransport not safe for reuse after reading corrupted input

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165584#comment-16165584
 ] 

ASF GitHub Bot commented on THRIFT-4326:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1352
  
@benweint could you do be a favor and do an empty amend (git commit 
--amend, and then don't change anything) and force push?  This will generate a 
new build.  It's build job 4 I am most interested in seeing pass as it runs the 
ruby tests.


> Ruby BufferedTransport not safe for reuse after reading corrupted input
> ---
>
> Key: THRIFT-4326
> URL: https://issues.apache.org/jira/browse/THRIFT-4326
> Project: Thrift
>  Issue Type: Bug
>  Components: Ruby - Library
>Affects Versions: 0.10.0
> Environment: Originally observed with Thrift 0.9.3 on Linux with Ruby 
> 2.3.4, but have also reproduced on Mac OS X with Thrift 0.10.0.
>Reporter: Ben Weintraub
>
> We've experimented with the Ruby {{BufferedTransport}} class as a wrapper 
> around the {{HttpClientTransport}} class, and found that we were getting 
> clusters sporadic {{Thrift::ProtocolException}} errors in Ruby client 
> processes after network issues caused corruption of some Thrift response 
> bodies.
> Using a bare {{HttpClientTransport}} makes these issues disappear.
> For a given service, we retain a long-lived protocol instance 
> ({{CompactProtocol}} in our case), which in turn holds a reference to a 
> long-lived {{BufferedTransport}} instance.
> The problem seems to stem from the case where the Thrift client is 
> interrupted (e.g. by a Ruby timeout exception) before consuming to the end of 
> the {{@rbuf}} instance variable in {{BufferedTransport}}, leaving {{@index}} 
> pointing to the middle of the read buffer, and meaning that when the 
> transport is re-used upon the next service call, the {{BufferedTransport}} 
> continues reading where it left off in the old buffer, rather than calling 
> through to the wrapped {{HttpClientTransport}} to read the new response 
> obtained from the last call to {{#flush}}.
> Now I know {{Timeout}} is fundamentally unsafe in Ruby and can lead to all 
> kinds of issues like this, but I've also found that this same issue can be 
> triggered by another fairly plausible scenario: if the Thrift service returns 
> a well-formed Thrift response but with N extra bytes of garbage tacked onto 
> the end, then the next N following service calls through the same 
> {{BufferedTransport}} instance will fail with a 
> {{Thrift::ProtocolException}}, as the {{BufferedTransport}} will continue 
> attempting to read the left-over bytes in {{@rbuf}}.
> The naive solution seems like it would be to just reset {{@rbuf}} from 
> {{#flush}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift issue #1352: THRIFT-4326 Allow safer reuse of BufferedTransport insta...

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1352
  
@benweint could you do be a favor and do an empty amend (git commit 
--amend, and then don't change anything) and force push?  This will generate a 
new build.  It's build job 4 I am most interested in seeing pass as it runs the 
ruby tests.


---

[jira] [Commented] (THRIFT-4326) Ruby BufferedTransport not safe for reuse after reading corrupted input

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165532#comment-16165532
 ] 

ASF GitHub Bot commented on THRIFT-4326:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1352
  
@benweint excellent, I'll keep an eye on these.


> Ruby BufferedTransport not safe for reuse after reading corrupted input
> ---
>
> Key: THRIFT-4326
> URL: https://issues.apache.org/jira/browse/THRIFT-4326
> Project: Thrift
>  Issue Type: Bug
>  Components: Ruby - Library
>Affects Versions: 0.10.0
> Environment: Originally observed with Thrift 0.9.3 on Linux with Ruby 
> 2.3.4, but have also reproduced on Mac OS X with Thrift 0.10.0.
>Reporter: Ben Weintraub
>
> We've experimented with the Ruby {{BufferedTransport}} class as a wrapper 
> around the {{HttpClientTransport}} class, and found that we were getting 
> clusters sporadic {{Thrift::ProtocolException}} errors in Ruby client 
> processes after network issues caused corruption of some Thrift response 
> bodies.
> Using a bare {{HttpClientTransport}} makes these issues disappear.
> For a given service, we retain a long-lived protocol instance 
> ({{CompactProtocol}} in our case), which in turn holds a reference to a 
> long-lived {{BufferedTransport}} instance.
> The problem seems to stem from the case where the Thrift client is 
> interrupted (e.g. by a Ruby timeout exception) before consuming to the end of 
> the {{@rbuf}} instance variable in {{BufferedTransport}}, leaving {{@index}} 
> pointing to the middle of the read buffer, and meaning that when the 
> transport is re-used upon the next service call, the {{BufferedTransport}} 
> continues reading where it left off in the old buffer, rather than calling 
> through to the wrapped {{HttpClientTransport}} to read the new response 
> obtained from the last call to {{#flush}}.
> Now I know {{Timeout}} is fundamentally unsafe in Ruby and can lead to all 
> kinds of issues like this, but I've also found that this same issue can be 
> triggered by another fairly plausible scenario: if the Thrift service returns 
> a well-formed Thrift response but with N extra bytes of garbage tacked onto 
> the end, then the next N following service calls through the same 
> {{BufferedTransport}} instance will fail with a 
> {{Thrift::ProtocolException}}, as the {{BufferedTransport}} will continue 
> attempting to read the left-over bytes in {{@rbuf}}.
> The naive solution seems like it would be to just reset {{@rbuf}} from 
> {{#flush}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift issue #1352: THRIFT-4326 Allow safer reuse of BufferedTransport insta...

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1352
  
@benweint excellent, I'll keep an eye on these.


---

Looking for Java review of a pull request

[James E. King, III]

https://github.com/apache/thrift/pull/1313

This is in regards to a long standing java issue with epoll and I would
like to see some opinions as to whether it is necessary to add to the
project.

Thanks,

Jim

[jira] [Commented] (THRIFT-4326) Ruby BufferedTransport not safe for reuse after reading corrupted input

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165525#comment-16165525
 ] 

ASF GitHub Bot commented on THRIFT-4326:


Github user benweint commented on the issue:

https://github.com/apache/thrift/pull/1352
  
Thanks @jeking3! I'v just rebased and pushed, so hopefully that'll make CI 
happy.


> Ruby BufferedTransport not safe for reuse after reading corrupted input
> ---
>
> Key: THRIFT-4326
> URL: https://issues.apache.org/jira/browse/THRIFT-4326
> Project: Thrift
>  Issue Type: Bug
>  Components: Ruby - Library
>Affects Versions: 0.10.0
> Environment: Originally observed with Thrift 0.9.3 on Linux with Ruby 
> 2.3.4, but have also reproduced on Mac OS X with Thrift 0.10.0.
>Reporter: Ben Weintraub
>
> We've experimented with the Ruby {{BufferedTransport}} class as a wrapper 
> around the {{HttpClientTransport}} class, and found that we were getting 
> clusters sporadic {{Thrift::ProtocolException}} errors in Ruby client 
> processes after network issues caused corruption of some Thrift response 
> bodies.
> Using a bare {{HttpClientTransport}} makes these issues disappear.
> For a given service, we retain a long-lived protocol instance 
> ({{CompactProtocol}} in our case), which in turn holds a reference to a 
> long-lived {{BufferedTransport}} instance.
> The problem seems to stem from the case where the Thrift client is 
> interrupted (e.g. by a Ruby timeout exception) before consuming to the end of 
> the {{@rbuf}} instance variable in {{BufferedTransport}}, leaving {{@index}} 
> pointing to the middle of the read buffer, and meaning that when the 
> transport is re-used upon the next service call, the {{BufferedTransport}} 
> continues reading where it left off in the old buffer, rather than calling 
> through to the wrapped {{HttpClientTransport}} to read the new response 
> obtained from the last call to {{#flush}}.
> Now I know {{Timeout}} is fundamentally unsafe in Ruby and can lead to all 
> kinds of issues like this, but I've also found that this same issue can be 
> triggered by another fairly plausible scenario: if the Thrift service returns 
> a well-formed Thrift response but with N extra bytes of garbage tacked onto 
> the end, then the next N following service calls through the same 
> {{BufferedTransport}} instance will fail with a 
> {{Thrift::ProtocolException}}, as the {{BufferedTransport}} will continue 
> attempting to read the left-over bytes in {{@rbuf}}.
> The naive solution seems like it would be to just reset {{@rbuf}} from 
> {{#flush}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift issue #1352: THRIFT-4326 Allow safer reuse of BufferedTransport insta...

[benweint]

Github user benweint commented on the issue:

https://github.com/apache/thrift/pull/1352
  
Thanks @jeking3! I'v just rebased and pushed, so hopefully that'll make CI 
happy.


---

[jira] [Commented] (THRIFT-4232) ./configure does bad ant version check

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4232?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165505#comment-16165505
 ] 

ASF GitHub Bot commented on THRIFT-4232:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1354#discussion_r138770754
  
--- Diff: aclocal/ax_javac_and_java.m4 ---
@@ -118,7 +118,7 @@ AC_DEFUN([AX_CHECK_JAVA_CLASS],
 AC_DEFUN([AX_CHECK_ANT_VERSION],
  [
   AC_MSG_CHECKING(for ant version > $2)
-  ANT_VALID=`expr $($1 -version 2>/dev/null | sed -n 's/.*version 
\(@<:@0-9\.@:>@*\).*/\1/p') \>= $2`
+  ANT_VALID=`expr "x$(printf "$2\n$($1 -version 2>/dev/null | sed 
-n 's/.*version \(@<:@0-9\.@:>@*\).*/\1/p')" | sort -t '.' -k 1,1 -k 2,2 -k 3,3 
-g | sed -n 1p)" = "x$2"`
--- End diff --

I don't see this working on my bash shell:

```
jking@ubuntu:~/thrift/github/thrift$ ant -version
Apache Ant(TM) version 1.9.8 compiled on January 19 2017

jking@ubuntu:~/thrift/github/thrift$ ant -version | sed -n 's/.*version 
\(@<:@0-9\.@:>@*\).*/\1/p'
(no output)
```

Try this instead:
```
jking@ubuntu:~/thrift/github/thrift$ ant -version | cut -d' ' -f4
1.9.8
```

Test:

If I require 1.9.9, fails, if I require 1.9.7, passes:
```
jking@ubuntu:~/thrift/github/thrift$ expr "x$(printf "1.9.9\n$(ant -version 
| cut -d' ' -f4)" | sort -t '.' -k 1,1 -k 2,2 -k 3,3 -g | sed -n 1p)" = "x1.9.9"
0
jking@ubuntu:~/thrift/github/thrift$ expr "x$(printf "1.9.7\n$(ant -version 
| cut -d' ' -f4)" | sort -t '.' -k 1,1 -k 2,2 -k 3,3 -g | sed -n 1p)" = "x1.9.7"
1
```


> ./configure does bad ant version check
> --
>
> Key: THRIFT-4232
> URL: https://issues.apache.org/jira/browse/THRIFT-4232
> Project: Thrift
>  Issue Type: Bug
>  Components: Build Process
>Affects Versions: 0.10.0
> Environment: OSX 10.12.5, running ant 1.10.1
>Reporter: David Woodward
>   Original Estimate: 3h
>  Remaining Estimate: 3h
>
> On line 18869 of the configure script, it checks that the ant version is >= 
> 1.7. It uses some kind of string comparison. This breaks for my current ant 
> version (1.10). It seems to think that 1.10 is not >= 1.7. I think this is 
> because it's comparing strings without taking into account what the strings 
> actually mean. Something like this might be a possible patch:
> https://stackoverflow.com/questions/16989598/bash-comparing-version-numbers
> This should be fixed because it means that people with new ant versions can't 
> build the java thrift library.
> Also it should be checked to see if other parts of the configure process are 
> using these kinds of faulty version checks.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift pull request #1354: THRIFT-4232 ./configure does bad ant version chec...

[jeking3]

Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1354#discussion_r138770754
  
--- Diff: aclocal/ax_javac_and_java.m4 ---
@@ -118,7 +118,7 @@ AC_DEFUN([AX_CHECK_JAVA_CLASS],
 AC_DEFUN([AX_CHECK_ANT_VERSION],
  [
   AC_MSG_CHECKING(for ant version > $2)
-  ANT_VALID=`expr $($1 -version 2>/dev/null | sed -n 's/.*version 
\(@<:@0-9\.@:>@*\).*/\1/p') \>= $2`
+  ANT_VALID=`expr "x$(printf "$2\n$($1 -version 2>/dev/null | sed 
-n 's/.*version \(@<:@0-9\.@:>@*\).*/\1/p')" | sort -t '.' -k 1,1 -k 2,2 -k 3,3 
-g | sed -n 1p)" = "x$2"`
--- End diff --

I don't see this working on my bash shell:

```
jking@ubuntu:~/thrift/github/thrift$ ant -version
Apache Ant(TM) version 1.9.8 compiled on January 19 2017

jking@ubuntu:~/thrift/github/thrift$ ant -version | sed -n 's/.*version 
\(@<:@0-9\.@:>@*\).*/\1/p'
(no output)
```

Try this instead:
```
jking@ubuntu:~/thrift/github/thrift$ ant -version | cut -d' ' -f4
1.9.8
```

Test:

If I require 1.9.9, fails, if I require 1.9.7, passes:
```
jking@ubuntu:~/thrift/github/thrift$ expr "x$(printf "1.9.9\n$(ant -version 
| cut -d' ' -f4)" | sort -t '.' -k 1,1 -k 2,2 -k 3,3 -g | sed -n 1p)" = "x1.9.9"
0
jking@ubuntu:~/thrift/github/thrift$ expr "x$(printf "1.9.7\n$(ant -version 
| cut -d' ' -f4)" | sort -t '.' -k 1,1 -k 2,2 -k 3,3 -g | sed -n 1p)" = "x1.9.7"
1
```


---

[jira] [Commented] (THRIFT-2289) Erlang impl of Thrift JSON protocol wrongly writes/expects true/false for bools

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-2289?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165334#comment-16165334
 ] 

ASF GitHub Bot commented on THRIFT-2289:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1349
  
If you rebase on master it will help, but I can't guarantee it will make 
everything clean - will be better.


> Erlang impl of Thrift JSON protocol wrongly writes/expects true/false for 
> bools
> ---
>
> Key: THRIFT-2289
> URL: https://issues.apache.org/jira/browse/THRIFT-2289
> Project: Thrift
>  Issue Type: Bug
>  Components: Erlang - Library
>Affects Versions: 0.9.1
>Reporter: Jens Geyer
>
> Some implementations of Thrift JSON protocol wrongly write and expect 
> true/false for bools, instead of 0/1



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift issue #1349: THRIFT-2289: Erlang impl of Thrift JSON protocol wrongly...

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1349
  
If you rebase on master it will help, but I can't guarantee it will make 
everything clean - will be better.


---

[jira] [Commented] (THRIFT-4328) Travis CI builds are timing out (job 1) and haxe builds are failing since 9/11

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4328?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165333#comment-16165333
 ] 

ASF GitHub Bot commented on THRIFT-4328:


Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1351


> Travis CI builds are timing out (job 1) and haxe builds are failing since 9/11
> --
>
> Key: THRIFT-4328
> URL: https://issues.apache.org/jira/browse/THRIFT-4328
> Project: Thrift
>  Issue Type: Bug
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: travis CI
>Reporter: James E. King, III
>Assignee: James E. King, III
>Priority: Blocker
> Fix For: 0.11.0
>
>
> Need to split binary protocol cross tests to a new build job (previous 
> combination of build jobs was too aggressive).
> Need to fix haxe problem with builds that starts on 9/11.  Looks like a new 
> version of hxcpp is to blame, and pinning to the previous one resolves it.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Resolved] (THRIFT-4328) Travis CI builds are timing out (job 1) and haxe builds are failing since 9/11

[James E. King, III (JIRA)]

 [ 
https://issues.apache.org/jira/browse/THRIFT-4328?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III resolved THRIFT-4328.

Resolution: Fixed

> Travis CI builds are timing out (job 1) and haxe builds are failing since 9/11
> --
>
> Key: THRIFT-4328
> URL: https://issues.apache.org/jira/browse/THRIFT-4328
> Project: Thrift
>  Issue Type: Bug
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: travis CI
>Reporter: James E. King, III
>Assignee: James E. King, III
>Priority: Blocker
> Fix For: 0.11.0
>
>
> Need to split binary protocol cross tests to a new build job (previous 
> combination of build jobs was too aggressive).
> Need to fix haxe problem with builds that starts on 9/11.  Looks like a new 
> version of hxcpp is to blame, and pinning to the previous one resolves it.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift pull request #1351: THRIFT-4328: fix broken travis CI builds, enable ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1351


---

Build failure - js experts?

[James E. King, III]

I can't figure out why the UBSan build job is failing in travis.
I have marked the build job as failure allowed for now.
Here's an example job:
https://travis-ci.org/apache/thrift/jobs/275182729

make[5]: Entering directory '/thrift/src/node_modules/ws'

  CXX(target) Release/obj.target/bufferutil/src/bufferutil.o

bufferutil.target.mk:97: recipe for target
'Release/obj.target/bufferutil/src/bufferutil.o' failed

make[5]: Leaving directory '/thrift/src/node_modules/ws/build'

npm notice created a lockfile as package-lock.json. You should commit this
file.

added 354 packages in 15.164s

Running "shell:ThriftGen" (shell) task

[WARNING:/thrift/src/test/ThriftTest.thrift:45] No generator named
'noexist' could be found!

[WARNING:/thrift/src/test/ThriftTest.thrift:47] cpp generator does not
accept 'noexist' as sub-namespace!

Running "external_daemon:ThriftTestServer" (external_daemon) task

>> Started ThriftTestServer

Running "external_daemon:ThriftTestServer_TLS" (external_daemon) task

>> Started ThriftTestServer_TLS

Running "qunit:ThriftJS" (qunit) task

Testing http://localhost:8088/test-nojq.html ...OK

>> 27 tests completed with 0 failed, 0 skipped, and 0 todo.

>> 99 assertions (in 454ms), passed: 99, failed: 0

Running "qunit:ThriftJS_TLS" (qunit) task

Testing https://localhost:8089/test-nojq.html ...OK

>> 27 tests completed with 0 failed, 0 skipped, and 0 todo.

>> 99 assertions (in 971ms), passed: 99, failed: 0

Running "shell:ThriftGenJQ" (shell) task

[WARNING:/thrift/src/test/ThriftTest.thrift:45] No generator named
'noexist' could be found!

[WARNING:/thrift/src/test/ThriftTest.thrift:47] cpp generator does not
accept 'noexist' as sub-namespace!

Running "qunit:ThriftJSJQ" (qunit) task

Testing http://localhost:8088/test.html ...OK

>> 31 tests completed with 0 failed, 0 skipped, and 0 todo.

>> 110 assertions (in 689ms), passed: 110, failed: 0

Running "qunit:ThriftJSJQ_TLS" (qunit) task

Testing https://localhost:8089/test.html ...OK

>> 31 tests completed with 0 failed, 0 skipped, and 0 todo.

>> 110 assertions (in 1239ms), passed: 110, failed: 0

Running "concat:dist" (concat) task

Running "uglify:dist" (uglify) task

File dist/thrift.min.js created: 46.89 kB → 15.4 kB

>> 1 file created.

Running "jsdoc:dist" (jsdoc) task

>> fs.js:1919

>>   binding.copyFile(src, dest, flags);

>>   ^

>>

>> Error: EISDIR: illegal operation on a directory, copyfile
'/thrift/src/lib/js/node_modules/jsdoc/templates/default/static/fonts/OpenSans-Bold-webfont.eot'
-> 'doc/fonts'

>> at Object.fs.copyFileSync (fs.js:1919:11)

>> at
/thrift/src/lib/js/node_modules/jsdoc/templates/default/publish.js:471:12

>> at Array.forEach ()

>> at Object.exports.publish
(/thrift/src/lib/js/node_modules/jsdoc/templates/default/publish.js:468:17)

>> at Object.module.exports.cli.generateDocs
(/thrift/src/lib/js/node_modules/jsdoc/cli.js:430:39)

>> at Object.module.exports.cli.processParseResults
(/thrift/src/lib/js/node_modules/jsdoc/cli.js:383:20)

>> at module.exports.cli.main
(/thrift/src/lib/js/node_modules/jsdoc/cli.js:227:14)

>> at Object.module.exports.cli.runCommand
(/thrift/src/lib/js/node_modules/jsdoc/cli.js:180:5)

>> at /thrift/src/lib/js/node_modules/jsdoc/jsdoc.js:103:9

>> at Object.
(/thrift/src/lib/js/node_modules/jsdoc/jsdoc.js:104:3)

>> at Module._compile (module.js:624:30)

>> at Object.Module._extensions..js (module.js:635:10)

>> at Module.load (module.js:545:32)

>> at tryModuleLoad (module.js:508:12)

>> at Function.Module._load (module.js:500:3)

>> at Function.Module.runMain (module.js:665:10)

Warning: jsdoc terminated with a non-zero exit code Use --force to continue.

Aborted due to warnings.

Makefile:805: recipe for target 'check-local' failed

make[4]: *** [check-local] Error 3

make[4]: Leaving directory '/thrift/src/lib/js'

Makefile:680: recipe for target 'check-am' failed

make[3]: *** [check-am] Error 2

make[3]: Leaving directory '/thrift/src/lib/js'

Makefile:531: recipe for target 'check-recursive' failed

make[2]: *** [check-recursive] Error 1

make[2]: Leaving directory '/thrift/src/lib/js'

Makefile:576: recipe for target 'check-recursive' failed

make[1]: *** [check-recursive] Error 1

make[1]: Leaving directory '/thrift/src/lib'

Makefile:655: recipe for target 'check-recursive' failed

make: *** [check-recursive] Error 1

[jira] [Commented] (THRIFT-4064) Update node library dependencies

[James E. King, III (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4064?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165185#comment-16165185
 ] 

James E. King, III commented on THRIFT-4064:


I tried to re-apply the pull request and run make check in the ubuntu-xenial 
image.
I found this is happening:
{noformat}
root@a588b4cf0ace:/thrift/src/lib/nodejs# 
NODE_PATH=/thrift/src/lib/nodejs/test:/thrift/src/lib/nodejs/test/../lib node 
/thrift/src/lib/nodejs/test/browser_client.js
TAP version 13
# NodeJS Style Callback Client Tests

/thrift/src/lib/nodejs/lib/thrift/xhr_connection.js:84
  throw "Your browser doesn't support XHR.";
  ^
Your browser doesn't support XHR.
{noformat}

Perhaps that will help someone else get started.  To make it fail faster I 
moved the call to testBrowser above the integration tests when I was working on 
it.

> Update node library dependencies
> 
>
> Key: THRIFT-4064
> URL: https://issues.apache.org/jira/browse/THRIFT-4064
> Project: Thrift
>  Issue Type: Improvement
>  Components: Node.js - Library
>Affects Versions: 0.10.0
>Reporter: Andres Suarez
>  Labels: security-issue
>
> ws@0.4.32 is really old and presents issues for users using modern versions 
> of Node (see 
> https://github.com/apache/thrift/pull/672#issuecomment-276678791). Its should 
> be updated.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift issue #1355: Minimal C# library version for .NET Standard 1.4, 1.6, ....

[aloneguid]

Github user aloneguid commented on the issue:

https://github.com/apache/thrift/pull/1355
  
Sorry guys I'm giving up. Personally I think getting a real value from 
contributing to main repo is not worth the effort due to many showstoppers. 
This may be a reason why there are so many forked versions exist. I don't mean 
to offend anyone here.


---

[GitHub] thrift pull request #1355: Minimal C# library version for .NET Standard 1.4,...

[aloneguid]

Github user aloneguid closed the pull request at:

https://github.com/apache/thrift/pull/1355


---

[jira] [Commented] (THRIFT-4327) Improve TimerManager API to allow removing specific task

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4327?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165124#comment-16165124
 ] 

ASF GitHub Bot commented on THRIFT-4327:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1353#discussion_r138711396
  
--- Diff: lib/cpp/src/thrift/concurrency/TimerManager.h ---
@@ -100,13 +108,26 @@ class TimerManager {
*/
   virtual void remove(stdcxx::shared_ptr task);
 
+  /**
+   * Removes a single pending task
+   *
+   * @param timer The timer to remove. The timer is returned when calling 
the
+   * add() method.
+   * @throws NoSuchTaskException Specified task doesn't exist. It was 
either
+   * processed already or this call was made 
for a
+   * task that was never added to this timer
+   *
+   * @throws UncancellableTaskException Specified task is already being
+   *executed or has completed 
execution.
+   */
+  virtual void remove(Timer timer);
--- End diff --

Was removal of a task insufficient in some way?


> Improve TimerManager API to allow removing specific task
> 
>
> Key: THRIFT-4327
> URL: https://issues.apache.org/jira/browse/THRIFT-4327
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Library
>Reporter: Francois Ferrand
>
> The TimerManager::remove() method removes all timers with the specified 
> callback, and does so by traversing the list of timers.
> This should be improved by returning a "handle" in `TimerManager::add`, and 
> supporting efficiently removing a single timer from its handle:
> {code:java}
> class TimerManager {
>  Timer add(shared_ptr task, const struct timeval& value);
>  void remove(Timer t);
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4327) Improve TimerManager API to allow removing specific task

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4327?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165125#comment-16165125
 ] 

ASF GitHub Bot commented on THRIFT-4327:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1353#discussion_r138711309
  
--- Diff: lib/cpp/test/concurrency/TimerManagerTests.h ---
@@ -192,6 +192,38 @@ class TimerManagerTests {
 return true;
   }
 
+/**
+   * This test creates two tasks, removes the first one then waits for the 
second one. It then
+   * verifies that the timer manager properly clean up itself and the 
remaining orphaned timeout
+   * task when the manager goes out of scope and its destructor is called.
+   */
+  bool test03(int64_t timeout = 1000LL) {
+TimerManager timerManager;
+timerManager.threadFactory(shared_ptr(new 
PlatformThreadFactory()));
+timerManager.start();
+assert(timerManager.state() == TimerManager::STARTED);
+
+Synchronized s(_monitor);
+
+// Setup the two tasks
+shared_ptr taskToRemove
+= shared_ptr(new 
TimerManagerTests::Task(_monitor, timeout / 2));
+TimerManager::Timer timer = timerManager.add(taskToRemove, 
taskToRemove->_timeout);
+
+shared_ptr task
+  = shared_ptr(new 
TimerManagerTests::Task(_monitor, timeout));
+timerManager.add(task, task->_timeout);
+
+// Remove one task and wait until the other has completed
+timerManager.remove(timer);
--- End diff --

recommend calling remove a second time and validating expected behavior


> Improve TimerManager API to allow removing specific task
> 
>
> Key: THRIFT-4327
> URL: https://issues.apache.org/jira/browse/THRIFT-4327
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Library
>Reporter: Francois Ferrand
>
> The TimerManager::remove() method removes all timers with the specified 
> callback, and does so by traversing the list of timers.
> This should be improved by returning a "handle" in `TimerManager::add`, and 
> supporting efficiently removing a single timer from its handle:
> {code:java}
> class TimerManager {
>  Timer add(shared_ptr task, const struct timeval& value);
>  void remove(Timer t);
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4327) Improve TimerManager API to allow removing specific task

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4327?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16165123#comment-16165123
 ] 

ASF GitHub Bot commented on THRIFT-4327:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1353#discussion_r138711130
  
--- Diff: lib/cpp/src/thrift/concurrency/TimerManager.h ---
@@ -69,28 +72,33 @@ class TimerManager {
*
* @param task The task to execute
* @param timeout Time in milliseconds to delay before executing task
+   * @return Handle of the timer, which can be used to remose the timer.
--- End diff --

remose == remove?


> Improve TimerManager API to allow removing specific task
> 
>
> Key: THRIFT-4327
> URL: https://issues.apache.org/jira/browse/THRIFT-4327
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Library
>Reporter: Francois Ferrand
>
> The TimerManager::remove() method removes all timers with the specified 
> callback, and does so by traversing the list of timers.
> This should be improved by returning a "handle" in `TimerManager::add`, and 
> supporting efficiently removing a single timer from its handle:
> {code:java}
> class TimerManager {
>  Timer add(shared_ptr task, const struct timeval& value);
>  void remove(Timer t);
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] thrift pull request #1353: THRIFT-4327: add API to efficiently remove a sing...

[jeking3]

Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1353#discussion_r138711396
  
--- Diff: lib/cpp/src/thrift/concurrency/TimerManager.h ---
@@ -100,13 +108,26 @@ class TimerManager {
*/
   virtual void remove(stdcxx::shared_ptr task);
 
+  /**
+   * Removes a single pending task
+   *
+   * @param timer The timer to remove. The timer is returned when calling 
the
+   * add() method.
+   * @throws NoSuchTaskException Specified task doesn't exist. It was 
either
+   * processed already or this call was made 
for a
+   * task that was never added to this timer
+   *
+   * @throws UncancellableTaskException Specified task is already being
+   *executed or has completed 
execution.
+   */
+  virtual void remove(Timer timer);
--- End diff --

Was removal of a task insufficient in some way?


---

[GitHub] thrift pull request #1353: THRIFT-4327: add API to efficiently remove a sing...

[jeking3]

Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1353#discussion_r138711309
  
--- Diff: lib/cpp/test/concurrency/TimerManagerTests.h ---
@@ -192,6 +192,38 @@ class TimerManagerTests {
 return true;
   }
 
+/**
+   * This test creates two tasks, removes the first one then waits for the 
second one. It then
+   * verifies that the timer manager properly clean up itself and the 
remaining orphaned timeout
+   * task when the manager goes out of scope and its destructor is called.
+   */
+  bool test03(int64_t timeout = 1000LL) {
+TimerManager timerManager;
+timerManager.threadFactory(shared_ptr(new 
PlatformThreadFactory()));
+timerManager.start();
+assert(timerManager.state() == TimerManager::STARTED);
+
+Synchronized s(_monitor);
+
+// Setup the two tasks
+shared_ptr taskToRemove
+= shared_ptr(new 
TimerManagerTests::Task(_monitor, timeout / 2));
+TimerManager::Timer timer = timerManager.add(taskToRemove, 
taskToRemove->_timeout);
+
+shared_ptr task
+  = shared_ptr(new 
TimerManagerTests::Task(_monitor, timeout));
+timerManager.add(task, task->_timeout);
+
+// Remove one task and wait until the other has completed
+timerManager.remove(timer);
--- End diff --

recommend calling remove a second time and validating expected behavior


---

[GitHub] thrift issue #1355: Minimal C# library version for .NET Standard 1.4, 1.6, ....

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1355
  
Is there a Jira ticket for this work?
https://thrift.apache.org/docs/HowToContribute


---

[jira] [Commented] (THRIFT-3351) Publishing Dart Bindings for Thrift to Pub (pub.dartlang.org)

[Mark Erickson (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-3351?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16164769#comment-16164769
 ] 

Mark Erickson commented on THRIFT-3351:
---

[~jfarrell] Unfortunately I think a Google account is required (gmail or Google 
Apps) - https://www.dartlang.org/tools/pub/cmd/pub-uploader

"Note that uploaders are identified by their Google accounts, so use a Gmail or 
Google Apps email address for any new uploaders."

> Publishing Dart Bindings for Thrift to Pub (pub.dartlang.org)
> -
>
> Key: THRIFT-3351
> URL: https://issues.apache.org/jira/browse/THRIFT-3351
> Project: Thrift
>  Issue Type: Story
>  Components: Dart - Library
>Reporter: Evan Weible
>Assignee: Jake Farrell
>
> The thrift library for Dart has been released in 0.10.0
> In order for developers to actually make use of the library, the Dart package 
> needs to be published to https://pub.dartlang.org. The current workaround is 
> to create a new GitHub repo with a copy of the Dart thrift library, and 
> reference that in consuming Dart code via a Git reference, which is not ideal.
> This may become a little better with Dart 1.25.0 (not released yet), which 
> [adds the 
> ability|https://github.com/dart-lang/sdk/blob/1.25.0-dev.16.3/CHANGELOG.md#tool-changes]
>  to reference a library in a subdirectory via git. But it would still be 
> better to explicitly publish releases for Dart to pub.dartlang.org.
> More information here: 
> - https://www.dartlang.org/tools/pub/publishing
> - https://www.dartlang.org/tools/pub/cmd/pub-lish.html
> Since we're creating the Dart bindings for Thrift, we are also more than 
> willing to publish them to Pub - but we wanted to give the apache thrift team 
> the opportunity to handle this process. Ownership of the publishing process 
> can be shared by adding multiple email addresses to the uploader list, and 
> the uploader list can be updated at any time. It does require a Google email 
> address (gmail or Google Apps) to perform the upload.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4260) Go context generation issue. Context is parameter in Interface not in implementation

[Jens Geyer (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16164535#comment-16164535
 ] 

Jens Geyer commented on THRIFT-4260:


{quote}
Is this in the latest version of thrift? It seems that auto-generated code does 
not have `context` in its function signatures, but thrift itself requires them, 
so auto-generated code does not compile with the version of thrift that 
generated it!
{quote}

If by "latest" you mean "dev", then  yes. All explained above in great detail. 

> Go context generation issue. Context is parameter in Interface not in 
> implementation
> 
>
> Key: THRIFT-4260
> URL: https://issues.apache.org/jira/browse/THRIFT-4260
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: Bas van Beek
>Assignee: taozle
>Priority: Blocker
> Fix For: 0.11.0
>
>
> Unfortunately the Go library was updated before a new Compiler was released. 
> Having thrift compiled code be part in a project prior to the thrift context 
> library addition breaks due to the dependency on the thrift go library. See: 
> https://github.com/openzipkin/zipkin-go-opentracing/issues/68
> I tried to resolve be installing compiler from latest source but found the 
> generated source to be incorrect. The generated Go interface for the service 
> client includes context.Context as the first parameter of the service method. 
> The generated client implementation however does not.
> The following thrift code:
> {code:none}
> enum ResultCode
> {
>   OK,
>   TRY_LATER
> }
> struct LogEntry
> {
>   1:  string category,
>   2:  string message
> }
> service Scribe
> {
>   ResultCode Log(1: list messages);
> }
> {code}
> Generated the following Go code:
> {code:none}
> type Scribe interface {
>   // Parameters:
>   //  - Messages
>   Log(ctx context.Context, messages []*LogEntry) (r ResultCode, err error)
> }
> ...
> // Parameters:
> //  - Messages
> func (p *ScribeClient) Log(messages []*LogEntry) (r ResultCode, err error) {
>   if err = p.sendLog(messages); err != nil {
>   return
>   }
>   return p.recvLog()
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)