[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


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

https://github.com/apache/thrift/pull/1197#discussion_r101909371
  
--- Diff: test/tests.json ---
@@ -606,5 +606,23 @@
   "compact"
 ],
 "workdir": "rs/bin"
+  },
+  {
+"name": "secure",
+"client": {
+  "command": [
+"test_secure.bash"
+  ]
+},
+"transports": [
+  "buffered"
+],
+"sockets": [
+  "ip-ssl"
+],
+"protocols": [
+  "binary"
+],
+"workdir": "secure"
--- End diff --

Simply moving the same JSON entry (with relative file path adjustment) 
might get this included to `make crossfeature`.
In that case, known_failure file is also in `features/known_...`.

Otherwise we can leave it as is and plan to relocate later.
I guess I'll need to put some information to `test/README.md` or create 
`test/features/README.md`


> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


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

https://github.com/apache/thrift/pull/1197#discussion_r101909372
  
--- Diff: lib/py/src/transport/sslcompat.py ---
@@ -66,6 +66,14 @@ def legacy_validate_callback(cert, hostname):
 
--- End diff --

As far as I can see, this does not change any of our docker image behaviors 
for python:
SSLv23 with NO_SSL* is being used for python 3 and debian python 2.

About defaulting to TLSv1.2, Python < 2.7.9 and C# with .NET 3.5 compat do 
not seem to have TLS 1.1 and 1.2 at all.
The highest versions listed in doc is TLSv1.0.

For C#, once dropping .NET 3.5, it would be able to support TLS 1.0, 1.1 
and 1.2 at once because SslProtocols is an explicit "flags" enum that can be 
combined with pipe like `Tls12 | Tls11 | Tls`.
Though I agree that it's not possible to do forward compatible "SSLv23 
minus SSL 2 and 3".


> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


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

https://github.com/apache/thrift/pull/1197#discussion_r101909373
  
--- Diff: test/secure/test_secure.bash ---
@@ -0,0 +1,69 @@
+#!/bin/bash
+
+#
+# Checks various desired attributes in SSL/TLS implementations.
+#
+
+THRIFTHOST=localhost
+THRIFTPORT=9090
+
+while [[ $# -ge 1 ]]; do
+  arg="$1"
+  argIN=(${arg//=/ })
+
+  case ${argIN[0]} in
+-h|--host)
+THRIFTHOST=${argIN[1]}
+shift # past argument
+;;
+-p|--port)
+THRIFTPORT=${argIN[1]}
+shift # past argument
+;;
+*)
+  # unknown option ignored
+;;
+  esac
+
+  shift   # past argument or value
+done
+
+#
+# Negotiation Test Expectations
+#
+
+declare -A EXPECT_NEGOTIATE
+EXPECT_NEGOTIATE[ssl3]=0
+EXPECT_NEGOTIATE[tls1]=1
+EXPECT_NEGOTIATE[tls1_1]=1
+EXPECT_NEGOTIATE[tls1_2]=1
--- End diff --

Sounds reasonable to me.


> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[nsuke]

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

https://github.com/apache/thrift/pull/1197#discussion_r101909371
  
--- Diff: test/tests.json ---
@@ -606,5 +606,23 @@
   "compact"
 ],
 "workdir": "rs/bin"
+  },
+  {
+"name": "secure",
+"client": {
+  "command": [
+"test_secure.bash"
+  ]
+},
+"transports": [
+  "buffered"
+],
+"sockets": [
+  "ip-ssl"
+],
+"protocols": [
+  "binary"
+],
+"workdir": "secure"
--- End diff --

Simply moving the same JSON entry (with relative file path adjustment) 
might get this included to `make crossfeature`.
In that case, known_failure file is also in `features/known_...`.

Otherwise we can leave it as is and plan to relocate later.
I guess I'll need to put some information to `test/README.md` or create 
`test/features/README.md`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[nsuke]

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

https://github.com/apache/thrift/pull/1197#discussion_r101909373
  
--- Diff: test/secure/test_secure.bash ---
@@ -0,0 +1,69 @@
+#!/bin/bash
+
+#
+# Checks various desired attributes in SSL/TLS implementations.
+#
+
+THRIFTHOST=localhost
+THRIFTPORT=9090
+
+while [[ $# -ge 1 ]]; do
+  arg="$1"
+  argIN=(${arg//=/ })
+
+  case ${argIN[0]} in
+-h|--host)
+THRIFTHOST=${argIN[1]}
+shift # past argument
+;;
+-p|--port)
+THRIFTPORT=${argIN[1]}
+shift # past argument
+;;
+*)
+  # unknown option ignored
+;;
+  esac
+
+  shift   # past argument or value
+done
+
+#
+# Negotiation Test Expectations
+#
+
+declare -A EXPECT_NEGOTIATE
+EXPECT_NEGOTIATE[ssl3]=0
+EXPECT_NEGOTIATE[tls1]=1
+EXPECT_NEGOTIATE[tls1_1]=1
+EXPECT_NEGOTIATE[tls1_2]=1
--- End diff --

Sounds reasonable to me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[nsuke]

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

https://github.com/apache/thrift/pull/1197#discussion_r101909372
  
--- Diff: lib/py/src/transport/sslcompat.py ---
@@ -66,6 +66,14 @@ def legacy_validate_callback(cert, hostname):
 
--- End diff --

As far as I can see, this does not change any of our docker image behaviors 
for python:
SSLv23 with NO_SSL* is being used for python 3 and debian python 2.

About defaulting to TLSv1.2, Python < 2.7.9 and C# with .NET 3.5 compat do 
not seem to have TLS 1.1 and 1.2 at all.
The highest versions listed in doc is TLSv1.0.

For C#, once dropping .NET 3.5, it would be able to support TLS 1.0, 1.1 
and 1.2 at once because SslProtocols is an explicit "flags" enum that can be 
combined with pipe like `Tls12 | Tls11 | Tls`.
Though I agree that it's not possible to do forward compatible "SSLv23 
minus SSL 2 and 3".


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


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

https://github.com/apache/thrift/pull/1197#discussion_r101909066
  
--- Diff: test/secure/test_secure.bash ---
@@ -0,0 +1,69 @@
+#!/bin/bash
+
+#
+# Checks various desired attributes in SSL/TLS implementations.
+#
+
+THRIFTHOST=localhost
+THRIFTPORT=9090
+
+while [[ $# -ge 1 ]]; do
+  arg="$1"
+  argIN=(${arg//=/ })
+
+  case ${argIN[0]} in
+-h|--host)
+THRIFTHOST=${argIN[1]}
+shift # past argument
+;;
+-p|--port)
+THRIFTPORT=${argIN[1]}
+shift # past argument
+;;
+*)
+  # unknown option ignored
+;;
+  esac
+
+  shift   # past argument or value
+done
+
+#
+# Negotiation Test Expectations
+#
+
+declare -A EXPECT_NEGOTIATE
+EXPECT_NEGOTIATE[ssl3]=0
+EXPECT_NEGOTIATE[tls1]=1
+EXPECT_NEGOTIATE[tls1_1]=1
+EXPECT_NEGOTIATE[tls1_2]=1
--- End diff --

What I'm thinking here is that nothing should allow for SSLv3 as you 
suggested, however it would be acceptable for csharp and d languages to only 
allow TLSv1.2, since they lack the ability to specify TLSv1.0 "or later".  So I 
can relax this test to say, "Accept either TLSv1.0 or later, or TLSv1.2 alone, 
but never allow SSLv3".  Really I could just simplify the test down to checking 
that SSLv3 is not allowed and TLSv1.2 works at this point.


> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[jeking3]

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

https://github.com/apache/thrift/pull/1197#discussion_r101909066
  
--- Diff: test/secure/test_secure.bash ---
@@ -0,0 +1,69 @@
+#!/bin/bash
+
+#
+# Checks various desired attributes in SSL/TLS implementations.
+#
+
+THRIFTHOST=localhost
+THRIFTPORT=9090
+
+while [[ $# -ge 1 ]]; do
+  arg="$1"
+  argIN=(${arg//=/ })
+
+  case ${argIN[0]} in
+-h|--host)
+THRIFTHOST=${argIN[1]}
+shift # past argument
+;;
+-p|--port)
+THRIFTPORT=${argIN[1]}
+shift # past argument
+;;
+*)
+  # unknown option ignored
+;;
+  esac
+
+  shift   # past argument or value
+done
+
+#
+# Negotiation Test Expectations
+#
+
+declare -A EXPECT_NEGOTIATE
+EXPECT_NEGOTIATE[ssl3]=0
+EXPECT_NEGOTIATE[tls1]=1
+EXPECT_NEGOTIATE[tls1_1]=1
+EXPECT_NEGOTIATE[tls1_2]=1
--- End diff --

What I'm thinking here is that nothing should allow for SSLv3 as you 
suggested, however it would be acceptable for csharp and d languages to only 
allow TLSv1.2, since they lack the ability to specify TLSv1.0 "or later".  So I 
can relax this test to say, "Accept either TLSv1.0 or later, or TLSv1.2 alone, 
but never allow SSLv3".  Really I could just simplify the test down to checking 
that SSLv3 is not allowed and TLSv1.2 works at this point.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


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

https://github.com/apache/thrift/pull/1197#discussion_r101909034
  
--- Diff: lib/py/src/transport/sslcompat.py ---
@@ -66,6 +66,14 @@ def legacy_validate_callback(cert, hostname):
 
--- End diff --

In the Ubuntu 14.04 LTS docker image, Python is version 2.7.6.  This means 
it doesn't have SSLContext.  This code detects that condition and sets a single 
version that it can use.  I am actually going to change the default to TLSv1.2 
for security purposes here.  Any language that cannot properly modify the 
openssl options to negotiate TLSv1.0 "or higher" should be forced to TLSv1.2.  
From what I can tell, csharp, d, and py2 (in the build image) lack the 
necessary code/knowledge to allow for a SSLv23 handshake and then autonegotiate 
TLSv1.0 "or higher".


> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[jeking3]

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

https://github.com/apache/thrift/pull/1197#discussion_r101909034
  
--- Diff: lib/py/src/transport/sslcompat.py ---
@@ -66,6 +66,14 @@ def legacy_validate_callback(cert, hostname):
 
--- End diff --

In the Ubuntu 14.04 LTS docker image, Python is version 2.7.6.  This means 
it doesn't have SSLContext.  This code detects that condition and sets a single 
version that it can use.  I am actually going to change the default to TLSv1.2 
for security purposes here.  Any language that cannot properly modify the 
openssl options to negotiate TLSv1.0 "or higher" should be forced to TLSv1.2.  
From what I can tell, csharp, d, and py2 (in the build image) lack the 
necessary code/knowledge to allow for a SSLv23 handshake and then autonegotiate 
TLSv1.0 "or higher".


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


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

https://github.com/apache/thrift/pull/1197#discussion_r101908742
  
--- Diff: test/tests.json ---
@@ -606,5 +606,23 @@
   "compact"
 ],
 "workdir": "rs/bin"
+  },
+  {
+"name": "secure",
+"client": {
+  "command": [
+"test_secure.bash"
+  ]
+},
+"transports": [
+  "buffered"
+],
+"sockets": [
+  "ip-ssl"
+],
+"protocols": [
+  "binary"
+],
+"workdir": "secure"
--- End diff --

I'm not certain how that is used and whether it runs as part of Travis CI.  
Is there any more documentation for the test/features area?


> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[jeking3]

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

https://github.com/apache/thrift/pull/1197#discussion_r101908742
  
--- Diff: test/tests.json ---
@@ -606,5 +606,23 @@
   "compact"
 ],
 "workdir": "rs/bin"
+  },
+  {
+"name": "secure",
+"client": {
+  "command": [
+"test_secure.bash"
+  ]
+},
+"transports": [
+  "buffered"
+],
+"sockets": [
+  "ip-ssl"
+],
+"protocols": [
+  "binary"
+],
+"workdir": "secure"
--- End diff --

I'm not certain how that is used and whether it runs as part of Travis CI.  
Is there any more documentation for the test/features area?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[jeking3]

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

https://github.com/apache/thrift/pull/1197#discussion_r101908692
  
--- Diff: lib/go/thrift/ssl_socket.go ---
@@ -20,152 +20,155 @@
 package thrift
--- End diff --

Sorry, I'm using the geany editor and I have it set to use 2 character 
spacing to match the rest of the codebase (C++ mostly).  I'll undo the 
whitespace changes here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-3891) TNonblockingServer configured with more than one IO threads does not always return from serve() upon stop()

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-3891:


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

https://github.com/apache/thrift/pull/1196#discussion_r101908414
  
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -820,9 +821,11 @@ void TNonblockingServer::TConnection::setFlags(short 
eventFlags) {
  * Closes a connection
  */
 void TNonblockingServer::TConnection::close() {
-  // Delete the registered libevent
-  if (event_del(_) == -1) {
-GlobalOutput.perror("TConnection::close() event_del", 
THRIFT_GET_SOCKET_ERROR);
+  if (eventFlags_) {
+if (event_del(_) == -1) {
+  GlobalOutput("TConnection::close event_del");
--- End diff --

Sorry I copied this from another location and it must have been using 
GlobalOutput.  I will fix this.


> TNonblockingServer configured with more than one IO threads does not always 
> return from serve() upon stop()
> ---
>
> Key: THRIFT-3891
> URL: https://issues.apache.org/jira/browse/THRIFT-3891
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Buğra Gedik
>Assignee: James E. King, III
>Priority: Minor
> Attachments: patch.diff
>
>
> Using {{TNonblockingServer}}, when the number of IO threads is > 1, there is 
> race condition in which {{stop()}} does not properly unblock {{serve()}}. 
> The problem manifests itself when {{stop()}} is called (obviously from a 
> different thread) soon after {{serve()}}. 
> The core issue is that, {{event_base_loopbreak()}} is called within the 
> {{stop()}} sequence without checking whether the IO thread has actually 
> entered its event loop. The documentation of {{event_base_loopbreak()}} says 
> (http://www.wangafu.net/~nickm/libevent-book/Ref3_eventloop.html)
> {quote}
> Note also that event_base_loopexit(base,NULL) and event_base_loopbreak(base) 
> act differently when no event loop is running: loopexit schedules the next 
> instance of the event loop to stop right after the next round of callbacks 
> are run (as if it had been invoked with EVLOOP_ONCE) whereas loopbreak only 
> stops a currently running loop, and has no effect if the event loop isn’t 
> running.
> {quote}
> Attached is a patch (against the released 0.9.3 version of the codebase).



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1196: THRIFT-3891 TNonblockingServer configured with mo...

[jeking3]

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

https://github.com/apache/thrift/pull/1196#discussion_r101908414
  
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -820,9 +821,11 @@ void TNonblockingServer::TConnection::setFlags(short 
eventFlags) {
  * Closes a connection
  */
 void TNonblockingServer::TConnection::close() {
-  // Delete the registered libevent
-  if (event_del(_) == -1) {
-GlobalOutput.perror("TConnection::close() event_del", 
THRIFT_GET_SOCKET_ERROR);
+  if (eventFlags_) {
+if (event_del(_) == -1) {
+  GlobalOutput("TConnection::close event_del");
--- End diff --

Sorry I copied this from another location and it must have been using 
GlobalOutput.  I will fix this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-3891) TNonblockingServer configured with more than one IO threads does not always return from serve() upon stop()

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-3891:


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

https://github.com/apache/thrift/pull/1196#discussion_r101908082
  
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -1520,13 +1524,9 @@ void TNonblockingIOThread::breakLoop(bool error) {
 ::abort();
   }
 
-  // sets a flag so that the loop exits on the next event
-  event_base_loopbreak(eventBase_);
+  // cause the loop to stop ASAP - even if it has things to do in it
+  event_base_loopexit(eventBase_, 0);
--- End diff --

+1
According to another look at the header doc, loopbreak is typically to be 
called from inside the loop.


> TNonblockingServer configured with more than one IO threads does not always 
> return from serve() upon stop()
> ---
>
> Key: THRIFT-3891
> URL: https://issues.apache.org/jira/browse/THRIFT-3891
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Buğra Gedik
>Assignee: James E. King, III
>Priority: Minor
> Attachments: patch.diff
>
>
> Using {{TNonblockingServer}}, when the number of IO threads is > 1, there is 
> race condition in which {{stop()}} does not properly unblock {{serve()}}. 
> The problem manifests itself when {{stop()}} is called (obviously from a 
> different thread) soon after {{serve()}}. 
> The core issue is that, {{event_base_loopbreak()}} is called within the 
> {{stop()}} sequence without checking whether the IO thread has actually 
> entered its event loop. The documentation of {{event_base_loopbreak()}} says 
> (http://www.wangafu.net/~nickm/libevent-book/Ref3_eventloop.html)
> {quote}
> Note also that event_base_loopexit(base,NULL) and event_base_loopbreak(base) 
> act differently when no event loop is running: loopexit schedules the next 
> instance of the event loop to stop right after the next round of callbacks 
> are run (as if it had been invoked with EVLOOP_ONCE) whereas loopbreak only 
> stops a currently running loop, and has no effect if the event loop isn’t 
> running.
> {quote}
> Attached is a patch (against the released 0.9.3 version of the codebase).



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-3891) TNonblockingServer configured with more than one IO threads does not always return from serve() upon stop()

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-3891:


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

https://github.com/apache/thrift/pull/1196#discussion_r101908093
  
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -820,9 +821,11 @@ void TNonblockingServer::TConnection::setFlags(short 
eventFlags) {
  * Closes a connection
  */
 void TNonblockingServer::TConnection::close() {
-  // Delete the registered libevent
-  if (event_del(_) == -1) {
-GlobalOutput.perror("TConnection::close() event_del", 
THRIFT_GET_SOCKET_ERROR);
+  if (eventFlags_) {
+if (event_del(_) == -1) {
--- End diff --

nits: single `if` with `eventFlags_ && event_del...` ?


> TNonblockingServer configured with more than one IO threads does not always 
> return from serve() upon stop()
> ---
>
> Key: THRIFT-3891
> URL: https://issues.apache.org/jira/browse/THRIFT-3891
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Buğra Gedik
>Assignee: James E. King, III
>Priority: Minor
> Attachments: patch.diff
>
>
> Using {{TNonblockingServer}}, when the number of IO threads is > 1, there is 
> race condition in which {{stop()}} does not properly unblock {{serve()}}. 
> The problem manifests itself when {{stop()}} is called (obviously from a 
> different thread) soon after {{serve()}}. 
> The core issue is that, {{event_base_loopbreak()}} is called within the 
> {{stop()}} sequence without checking whether the IO thread has actually 
> entered its event loop. The documentation of {{event_base_loopbreak()}} says 
> (http://www.wangafu.net/~nickm/libevent-book/Ref3_eventloop.html)
> {quote}
> Note also that event_base_loopexit(base,NULL) and event_base_loopbreak(base) 
> act differently when no event loop is running: loopexit schedules the next 
> instance of the event loop to stop right after the next round of callbacks 
> are run (as if it had been invoked with EVLOOP_ONCE) whereas loopbreak only 
> stops a currently running loop, and has no effect if the event loop isn’t 
> running.
> {quote}
> Attached is a patch (against the released 0.9.3 version of the codebase).



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-3891) TNonblockingServer configured with more than one IO threads does not always return from serve() upon stop()

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-3891:


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

https://github.com/apache/thrift/pull/1196#discussion_r101908056
  
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -212,7 +212,8 @@ class TNonblockingServer::TConnection {
   TConnection(THRIFT_SOCKET socket,
   TNonblockingIOThread* ioThread,
   const sockaddr* addr,
-  socklen_t addrLen) {
+  socklen_t addrLen)
+  {
--- End diff --

Enter key by accident ?


> TNonblockingServer configured with more than one IO threads does not always 
> return from serve() upon stop()
> ---
>
> Key: THRIFT-3891
> URL: https://issues.apache.org/jira/browse/THRIFT-3891
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Buğra Gedik
>Assignee: James E. King, III
>Priority: Minor
> Attachments: patch.diff
>
>
> Using {{TNonblockingServer}}, when the number of IO threads is > 1, there is 
> race condition in which {{stop()}} does not properly unblock {{serve()}}. 
> The problem manifests itself when {{stop()}} is called (obviously from a 
> different thread) soon after {{serve()}}. 
> The core issue is that, {{event_base_loopbreak()}} is called within the 
> {{stop()}} sequence without checking whether the IO thread has actually 
> entered its event loop. The documentation of {{event_base_loopbreak()}} says 
> (http://www.wangafu.net/~nickm/libevent-book/Ref3_eventloop.html)
> {quote}
> Note also that event_base_loopexit(base,NULL) and event_base_loopbreak(base) 
> act differently when no event loop is running: loopexit schedules the next 
> instance of the event loop to stop right after the next round of callbacks 
> are run (as if it had been invoked with EVLOOP_ONCE) whereas loopbreak only 
> stops a currently running loop, and has no effect if the event loop isn’t 
> running.
> {quote}
> Attached is a patch (against the released 0.9.3 version of the codebase).



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1196: THRIFT-3891 TNonblockingServer configured with mo...

[nsuke]

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

https://github.com/apache/thrift/pull/1196#discussion_r101908056
  
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -212,7 +212,8 @@ class TNonblockingServer::TConnection {
   TConnection(THRIFT_SOCKET socket,
   TNonblockingIOThread* ioThread,
   const sockaddr* addr,
-  socklen_t addrLen) {
+  socklen_t addrLen)
+  {
--- End diff --

Enter key by accident ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-3891) TNonblockingServer configured with more than one IO threads does not always return from serve() upon stop()

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-3891:


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

https://github.com/apache/thrift/pull/1196#discussion_r101908102
  
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -820,9 +821,11 @@ void TNonblockingServer::TConnection::setFlags(short 
eventFlags) {
  * Closes a connection
  */
 void TNonblockingServer::TConnection::close() {
-  // Delete the registered libevent
-  if (event_del(_) == -1) {
-GlobalOutput.perror("TConnection::close() event_del", 
THRIFT_GET_SOCKET_ERROR);
+  if (eventFlags_) {
+if (event_del(_) == -1) {
+  GlobalOutput("TConnection::close event_del");
--- End diff --

Not sure why changing from `GlobalOutput.perror` to `GlobalOutput`.


> TNonblockingServer configured with more than one IO threads does not always 
> return from serve() upon stop()
> ---
>
> Key: THRIFT-3891
> URL: https://issues.apache.org/jira/browse/THRIFT-3891
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Buğra Gedik
>Assignee: James E. King, III
>Priority: Minor
> Attachments: patch.diff
>
>
> Using {{TNonblockingServer}}, when the number of IO threads is > 1, there is 
> race condition in which {{stop()}} does not properly unblock {{serve()}}. 
> The problem manifests itself when {{stop()}} is called (obviously from a 
> different thread) soon after {{serve()}}. 
> The core issue is that, {{event_base_loopbreak()}} is called within the 
> {{stop()}} sequence without checking whether the IO thread has actually 
> entered its event loop. The documentation of {{event_base_loopbreak()}} says 
> (http://www.wangafu.net/~nickm/libevent-book/Ref3_eventloop.html)
> {quote}
> Note also that event_base_loopexit(base,NULL) and event_base_loopbreak(base) 
> act differently when no event loop is running: loopexit schedules the next 
> instance of the event loop to stop right after the next round of callbacks 
> are run (as if it had been invoked with EVLOOP_ONCE) whereas loopbreak only 
> stops a currently running loop, and has no effect if the event loop isn’t 
> running.
> {quote}
> Attached is a patch (against the released 0.9.3 version of the codebase).



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1196: THRIFT-3891 TNonblockingServer configured with mo...

[nsuke]

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

https://github.com/apache/thrift/pull/1196#discussion_r101908102
  
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -820,9 +821,11 @@ void TNonblockingServer::TConnection::setFlags(short 
eventFlags) {
  * Closes a connection
  */
 void TNonblockingServer::TConnection::close() {
-  // Delete the registered libevent
-  if (event_del(_) == -1) {
-GlobalOutput.perror("TConnection::close() event_del", 
THRIFT_GET_SOCKET_ERROR);
+  if (eventFlags_) {
+if (event_del(_) == -1) {
+  GlobalOutput("TConnection::close event_del");
--- End diff --

Not sure why changing from `GlobalOutput.perror` to `GlobalOutput`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1196: THRIFT-3891 TNonblockingServer configured with mo...

[nsuke]

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

https://github.com/apache/thrift/pull/1196#discussion_r101908093
  
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -820,9 +821,11 @@ void TNonblockingServer::TConnection::setFlags(short 
eventFlags) {
  * Closes a connection
  */
 void TNonblockingServer::TConnection::close() {
-  // Delete the registered libevent
-  if (event_del(_) == -1) {
-GlobalOutput.perror("TConnection::close() event_del", 
THRIFT_GET_SOCKET_ERROR);
+  if (eventFlags_) {
+if (event_del(_) == -1) {
--- End diff --

nits: single `if` with `eventFlags_ && event_del...` ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


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

https://github.com/apache/thrift/pull/1197#discussion_r101908015
  
--- Diff: lib/go/thrift/ssl_socket.go ---
@@ -20,152 +20,155 @@
 package thrift
--- End diff --

See 
https://github.com/golang/go/commit/014f3dcc837cb6789076cff4fccaa3bd221f823e


> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[nsuke]

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

https://github.com/apache/thrift/pull/1197#discussion_r101908015
  
--- Diff: lib/go/thrift/ssl_socket.go ---
@@ -20,152 +20,155 @@
 package thrift
--- End diff --

See 
https://github.com/golang/go/commit/014f3dcc837cb6789076cff4fccaa3bd221f823e


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


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

https://github.com/apache/thrift/pull/1197#discussion_r101907637
  
--- Diff: lib/nodejs/lib/thrift/connection.js ---
@@ -247,6 +247,11 @@ exports.createConnection = function(host, port, 
options) {
 };
 
 exports.createSSLConnection = function(host, port, options) {
+  if (!('secureProtocol' in options) && !('secureOptions' in options)) {
+options.secureProtocol = "SSLv23_method";
+options.secureOptions = 0x0300;
--- End diff --

Would be more readable with `constants.SSL_OP_NO_SSLv2 | 
constants.SSL_OP_NO_SSLv3` after requiring 'constants' at the top...


> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


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

https://github.com/apache/thrift/pull/1197#discussion_r101907319
  
--- Diff: lib/py/src/transport/sslcompat.py ---
@@ -66,6 +66,14 @@ def legacy_validate_callback(cert, hostname):
 
--- End diff --

Is it to introduce backports.ssl as an optional dependency for older 
Pythons ?
While it would make sense, there are some problematic details here.

I recommend doing it in a separate patch if you think it's worth it.
python2-secure is being listed in known_failures so this wouldn't affect 
test results.


> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


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

https://github.com/apache/thrift/pull/1197#discussion_r101907745
  
--- Diff: test/secure/test_secure.bash ---
@@ -0,0 +1,69 @@
+#!/bin/bash
+
+#
+# Checks various desired attributes in SSL/TLS implementations.
+#
+
+THRIFTHOST=localhost
+THRIFTPORT=9090
+
+while [[ $# -ge 1 ]]; do
+  arg="$1"
+  argIN=(${arg//=/ })
+
+  case ${argIN[0]} in
+-h|--host)
+THRIFTHOST=${argIN[1]}
+shift # past argument
+;;
+-p|--port)
+THRIFTPORT=${argIN[1]}
+shift # past argument
+;;
+*)
+  # unknown option ignored
+;;
+  esac
+
+  shift   # past argument or value
+done
+
+#
+# Negotiation Test Expectations
+#
+
+declare -A EXPECT_NEGOTIATE
+EXPECT_NEGOTIATE[ssl3]=0
+EXPECT_NEGOTIATE[tls1]=1
+EXPECT_NEGOTIATE[tls1_1]=1
+EXPECT_NEGOTIATE[tls1_2]=1
--- End diff --

Maybe split to multiple `tests.json` entries for no-ssl3 (and 2 ?) and tls 
1.x tests ?
We wouldn't want any of the former in known_failures.


> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


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

https://github.com/apache/thrift/pull/1197#discussion_r101907113
  
--- Diff: test/tests.json ---
@@ -606,5 +606,23 @@
   "compact"
 ],
 "workdir": "rs/bin"
+  },
+  {
+"name": "secure",
+"client": {
+  "command": [
+"test_secure.bash"
+  ]
+},
+"transports": [
+  "buffered"
+],
+"sockets": [
+  "ip-ssl"
+],
+"protocols": [
+  "binary"
+],
+"workdir": "secure"
--- End diff --

What do you think about moving them to test/features/tests.json ? Since 
it's not really a language binding.


> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[nsuke]

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

https://github.com/apache/thrift/pull/1197#discussion_r101907094
  
--- Diff: lib/go/thrift/ssl_socket.go ---
@@ -20,152 +20,155 @@
 package thrift
--- End diff --

We cannot replace indentation with spaces because, like it or not, tabs are 
the right indentation in Go world.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[nsuke]

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

https://github.com/apache/thrift/pull/1197#discussion_r101907745
  
--- Diff: test/secure/test_secure.bash ---
@@ -0,0 +1,69 @@
+#!/bin/bash
+
+#
+# Checks various desired attributes in SSL/TLS implementations.
+#
+
+THRIFTHOST=localhost
+THRIFTPORT=9090
+
+while [[ $# -ge 1 ]]; do
+  arg="$1"
+  argIN=(${arg//=/ })
+
+  case ${argIN[0]} in
+-h|--host)
+THRIFTHOST=${argIN[1]}
+shift # past argument
+;;
+-p|--port)
+THRIFTPORT=${argIN[1]}
+shift # past argument
+;;
+*)
+  # unknown option ignored
+;;
+  esac
+
+  shift   # past argument or value
+done
+
+#
+# Negotiation Test Expectations
+#
+
+declare -A EXPECT_NEGOTIATE
+EXPECT_NEGOTIATE[ssl3]=0
+EXPECT_NEGOTIATE[tls1]=1
+EXPECT_NEGOTIATE[tls1_1]=1
+EXPECT_NEGOTIATE[tls1_2]=1
--- End diff --

Maybe split to multiple `tests.json` entries for no-ssl3 (and 2 ?) and tls 
1.x tests ?
We wouldn't want any of the former in known_failures.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


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

https://github.com/apache/thrift/pull/1197#discussion_r101907094
  
--- Diff: lib/go/thrift/ssl_socket.go ---
@@ -20,152 +20,155 @@
 package thrift
--- End diff --

We cannot replace indentation with spaces because, like it or not, tabs are 
the right indentation in Go world.


> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[nsuke]

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

https://github.com/apache/thrift/pull/1197#discussion_r101907113
  
--- Diff: test/tests.json ---
@@ -606,5 +606,23 @@
   "compact"
 ],
 "workdir": "rs/bin"
+  },
+  {
+"name": "secure",
+"client": {
+  "command": [
+"test_secure.bash"
+  ]
+},
+"transports": [
+  "buffered"
+],
+"sockets": [
+  "ip-ssl"
+],
+"protocols": [
+  "binary"
+],
+"workdir": "secure"
--- End diff --

What do you think about moving them to test/features/tests.json ? Since 
it's not really a language binding.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[nsuke]

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

https://github.com/apache/thrift/pull/1197#discussion_r101907319
  
--- Diff: lib/py/src/transport/sslcompat.py ---
@@ -66,6 +66,14 @@ def legacy_validate_callback(cert, hostname):
 
--- End diff --

Is it to introduce backports.ssl as an optional dependency for older 
Pythons ?
While it would make sense, there are some problematic details here.

I recommend doing it in a separate patch if you think it's worth it.
python2-secure is being listed in known_failures so this wouldn't affect 
test results.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[nsuke]

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

https://github.com/apache/thrift/pull/1197#discussion_r101907637
  
--- Diff: lib/nodejs/lib/thrift/connection.js ---
@@ -247,6 +247,11 @@ exports.createConnection = function(host, port, 
options) {
 };
 
 exports.createSSLConnection = function(host, port, options) {
+  if (!('secureProtocol' in options) && !('secureOptions' in options)) {
+options.secureProtocol = "SSLv23_method";
+options.secureOptions = 0x0300;
--- End diff --

Would be more readable with `constants.SSL_OP_NO_SSLv2 | 
constants.SSL_OP_NO_SSLv3` after requiring 'constants' at the top...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Resolved] (THRIFT-4004) ThreadManager deadlock when adding new task

[James E. King, III (JIRA)]

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

James E. King, III resolved THRIFT-4004.

   Resolution: Invalid
Fix Version/s: 0.10.0

In the scenario you described, there would not be a deadlock.

Task #1 is added and monitor_ is signaled.
The worker does not wake up.
Task #2 inside add() would call maxMonitor_.wait() - this unlocks the mutex, 
since all of the monitors are sharing the same mutex.
The worker wakes up from waiting on monitor_ because it can acquire the mutex.  
It pops something off the queue.  Then it sees the queue is less than the 
maximum so it signals maxMonitor_.  Then it unlocks the mutex to run the task.
The add() for Task #2 would wake up from maxMonitor_ and add the task.

What makes this safe today as compared to past versions is that all three 
monitors share the same mutex.

> ThreadManager deadlock when adding new task
> ---
>
> Key: THRIFT-4004
> URL: https://issues.apache.org/jira/browse/THRIFT-4004
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Liu Lin 
>Assignee: James E. King, III
> Fix For: 0.10.0
>
>
> Set pendingTaskCountMax = 1. 
> Add only 1 worker into ThreadManager.
> When adding task, set timeout > 0.
> If there is no task, worker thread will sleep, because 
> manager_->monitor_.wait(Line 259) is called in function 
> ThreadManager::Worker::run.
> Then suppose we have 2 tasks. We add the first task by calling 
> ThreadManager::Impl::add, which will nofity monitor_. But before 
> manager_->monitor_.wait() returns, we call ThreadManager::Impl::add again. If 
> ThreadManager::Impl::add gets lock mutex_ successfully, 
> ThreadManager::Worker::run will fall asleep again because it can not lock 
> mutex_.
> Now we have tasks_.size() == pendingTaskCountMax_, so 
> ThreadManager::Impl::add will wait on maxMonitor_(Line 462). Becasue the 
> worker cannot fetch a task(because it can not lock mutex_, it will not notify 
> maxMonitor_), so ThreadManager::Impl::add will wait forever. 
> Now both the two threads wait for each other, result in deadlock. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-4084) Improve SSL security in thrift by adding a make cross client that checks to make sure SSLv3 protocol cannot be negotiated

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4084:


GitHub user jeking3 opened a pull request:

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

THRIFT-4084: Add a SSL/TLS negotiation check to crosstest to verify SSLv3 
is not active and that TLSv1.0 through 1.2 are accepted

Fixed the following server implementations to properly disable SSLv3 when 
using default settings:
go, nodejs, perl, py3

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

$ git pull https://github.com/jeking3/thrift THRIFT-4084

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

https://github.com/apache/thrift/pull/1197.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 #1197


commit 608329ba5cba941c67205f6e453b1173b6cd9c21
Author: James E. King, III 
Date:   2017-02-12T22:53:05Z

THRIFT-4084: Add a SSL/TLS negotiation check to crosstest to verify SSLv3 
is not active and that TLSv1.0 through 1.2 are accepted.
Client: go, nodejs, perl, python




> Improve SSL security in thrift by adding a make cross client that checks to 
> make sure SSLv3 protocol cannot be negotiated
> -
>
> Key: THRIFT-4084
> URL: https://issues.apache.org/jira/browse/THRIFT-4084
> Project: Thrift
>  Issue Type: Improvement
>  Components: Test Suite
>Affects Versions: 0.10.0
> Environment: Ubuntu Dockerfile
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: cross-validation, security, ssl, tls
>
> Following code review discussions in THRIFT-3369, and seeing THRIFT-3165 in 
> the backlog, I want to add a make cross "language" which isn't a language at 
> all, but a test that checks to see if it is possible to negotiate at various 
> SSL/TLS protocol versions.  This would be a client-only test, likely just a 
> bash script that leverages the openssl client and command line options to 
> connect to a test server and see if it handshakes and negotiates protocol 
> successfully.
> Without THRIFT-3165 implemented, it will ensure:
> * Can handshake using the universal SSLv23 context, however cannot negotiate 
> SSLv3
> * Can negotiate TLSv1.0, TLSv1.1, and TLSv1.2
> With THRIFT-3165 it needs to change to ensure:
> * Can handshake using TLSv1.2 but not any other version
> The solution I came up with was to add a new client called "secure" to make 
> crosstest.  test_secure is a simple bash script that checks the appropriate 
> rules above (the ones without THRIFT-3165, since it is not done), and I added 
> "secure" to the list of cross test "languages" in the top level configure 
> script.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1197: THRIFT-4084: Add a SSL/TLS negotiation check to c...

[jeking3]

GitHub user jeking3 opened a pull request:

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

THRIFT-4084: Add a SSL/TLS negotiation check to crosstest to verify SSLv3 
is not active and that TLSv1.0 through 1.2 are accepted

Fixed the following server implementations to properly disable SSLv3 when 
using default settings:
go, nodejs, perl, py3

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

$ git pull https://github.com/jeking3/thrift THRIFT-4084

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

https://github.com/apache/thrift/pull/1197.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 #1197


commit 608329ba5cba941c67205f6e453b1173b6cd9c21
Author: James E. King, III 
Date:   2017-02-12T22:53:05Z

THRIFT-4084: Add a SSL/TLS negotiation check to crosstest to verify SSLv3 
is not active and that TLSv1.0 through 1.2 are accepted.
Client: go, nodejs, perl, python




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1191
  
I'm not going to merge it until it passes make cross with multiplexed 
protocol.  There's nothing in the pull request and the build system that proves 
it works and will keep working.  I am working on THRIFT-4084 right now, I will 
see if I can get to it soon if you do not.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-3706) There's no support for Multiplexed protocol on c_glib library

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-3706:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1191
  
I'm not going to merge it until it passes make cross with multiplexed 
protocol.  There's nothing in the pull request and the build system that proves 
it works and will keep working.  I am working on THRIFT-4084 right now, I will 
see if I can get to it soon if you do not.


> There's no support for Multiplexed protocol on c_glib library
> -
>
> Key: THRIFT-3706
> URL: https://issues.apache.org/jira/browse/THRIFT-3706
> Project: Thrift
>  Issue Type: Improvement
>  Components: C glib - Library
>Affects Versions: 0.9.3
>Reporter: Gonzalo Aguilar
>Assignee: Gonzalo Aguilar
>
> There's no multiplexed protocol. 
> I will implement the same way it's done in Java an C++



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-3706) There's no support for Multiplexed protocol on c_glib library

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-3706:


Github user gadLinux commented on the issue:

https://github.com/apache/thrift/pull/1191
  
Hi Jeking3, 
Why don't you merge it and add a new issue for others to add multiplexing 
protocol?


> There's no support for Multiplexed protocol on c_glib library
> -
>
> Key: THRIFT-3706
> URL: https://issues.apache.org/jira/browse/THRIFT-3706
> Project: Thrift
>  Issue Type: Improvement
>  Components: C glib - Library
>Affects Versions: 0.9.3
>Reporter: Gonzalo Aguilar
>Assignee: Gonzalo Aguilar
>
> There's no multiplexed protocol. 
> I will implement the same way it's done in Java an C++



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

[gadLinux]

Github user gadLinux commented on the issue:

https://github.com/apache/thrift/pull/1191
  
Hi Jeking3, 
Why don't you merge it and add a new issue for others to add multiplexing 
protocol?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Assigned] (THRIFT-3891) TNonblockingServer configured with more than one IO threads does not always return from serve() upon stop()

[James E. King, III (JIRA)]

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

James E. King, III reassigned THRIFT-3891:
--

Assignee: James E. King, III

> TNonblockingServer configured with more than one IO threads does not always 
> return from serve() upon stop()
> ---
>
> Key: THRIFT-3891
> URL: https://issues.apache.org/jira/browse/THRIFT-3891
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Buğra Gedik
>Assignee: James E. King, III
>Priority: Minor
> Attachments: patch.diff
>
>
> Using {{TNonblockingServer}}, when the number of IO threads is > 1, there is 
> race condition in which {{stop()}} does not properly unblock {{serve()}}. 
> The problem manifests itself when {{stop()}} is called (obviously from a 
> different thread) soon after {{serve()}}. 
> The core issue is that, {{event_base_loopbreak()}} is called within the 
> {{stop()}} sequence without checking whether the IO thread has actually 
> entered its event loop. The documentation of {{event_base_loopbreak()}} says 
> (http://www.wangafu.net/~nickm/libevent-book/Ref3_eventloop.html)
> {quote}
> Note also that event_base_loopexit(base,NULL) and event_base_loopbreak(base) 
> act differently when no event loop is running: loopexit schedules the next 
> instance of the event loop to stop right after the next round of callbacks 
> are run (as if it had been invoked with EVLOOP_ONCE) whereas loopbreak only 
> stops a currently running loop, and has no effect if the event loop isn’t 
> running.
> {quote}
> Attached is a patch (against the released 0.9.3 version of the codebase).



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift issue #1196: THRIFT-3891 TNonblockingServer configured with more than...

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1196
  
Looks like I ran the build and tests outside the dockerfile so it didn't do 
any libevent stuff. :|


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-3891) TNonblockingServer configured with more than one IO threads does not always return from serve() upon stop()

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-3891:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1196
  
Looks like I ran the build and tests outside the dockerfile so it didn't do 
any libevent stuff. :|


> TNonblockingServer configured with more than one IO threads does not always 
> return from serve() upon stop()
> ---
>
> Key: THRIFT-3891
> URL: https://issues.apache.org/jira/browse/THRIFT-3891
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Buğra Gedik
>Priority: Minor
> Attachments: patch.diff
>
>
> Using {{TNonblockingServer}}, when the number of IO threads is > 1, there is 
> race condition in which {{stop()}} does not properly unblock {{serve()}}. 
> The problem manifests itself when {{stop()}} is called (obviously from a 
> different thread) soon after {{serve()}}. 
> The core issue is that, {{event_base_loopbreak()}} is called within the 
> {{stop()}} sequence without checking whether the IO thread has actually 
> entered its event loop. The documentation of {{event_base_loopbreak()}} says 
> (http://www.wangafu.net/~nickm/libevent-book/Ref3_eventloop.html)
> {quote}
> Note also that event_base_loopexit(base,NULL) and event_base_loopbreak(base) 
> act differently when no event loop is running: loopexit schedules the next 
> instance of the event loop to stop right after the next round of callbacks 
> are run (as if it had been invoked with EVLOOP_ONCE) whereas loopbreak only 
> stops a currently running loop, and has no effect if the event loop isn’t 
> running.
> {quote}
> Attached is a patch (against the released 0.9.3 version of the codebase).



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-3891) TNonblockingServer configured with more than one IO threads does not always return from serve() upon stop()

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-3891:


GitHub user jeking3 opened a pull request:

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

THRIFT-3891 TNonblockingServer configured with more than one IO threads 
does not always return from serve() upon stop()

I took the comments from @nsuke and applied them to the PR.  I was able to 
reasonably eliminate the boolean you mentioned however I could not go back to 
loopbreak.  I see no reasonable way to synchronize here.  The run() thread will 
eventually need to release a mutex and THEN call the event loop.  There's no 
way inside breakLoop() to know if it is actually IN the event loop.  Given 
this, the exit variant is the only safe method to break the loop.

The obsoletes PR #1080.

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

$ git pull https://github.com/jeking3/thrift THRIFT-3891

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

https://github.com/apache/thrift/pull/1196.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 #1196


commit 38d3c87aa11f1f4232cb97b01200c01fc578cd9d
Author: Buğra Gedik 
Date:   2016-09-04T08:18:15Z

THRIFT-3891 TNonblockingServer configured with more than one IO threads 
does not always return from serve() upon stop()




> TNonblockingServer configured with more than one IO threads does not always 
> return from serve() upon stop()
> ---
>
> Key: THRIFT-3891
> URL: https://issues.apache.org/jira/browse/THRIFT-3891
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Buğra Gedik
>Priority: Minor
> Attachments: patch.diff
>
>
> Using {{TNonblockingServer}}, when the number of IO threads is > 1, there is 
> race condition in which {{stop()}} does not properly unblock {{serve()}}. 
> The problem manifests itself when {{stop()}} is called (obviously from a 
> different thread) soon after {{serve()}}. 
> The core issue is that, {{event_base_loopbreak()}} is called within the 
> {{stop()}} sequence without checking whether the IO thread has actually 
> entered its event loop. The documentation of {{event_base_loopbreak()}} says 
> (http://www.wangafu.net/~nickm/libevent-book/Ref3_eventloop.html)
> {quote}
> Note also that event_base_loopexit(base,NULL) and event_base_loopbreak(base) 
> act differently when no event loop is running: loopexit schedules the next 
> instance of the event loop to stop right after the next round of callbacks 
> are run (as if it had been invoked with EVLOOP_ONCE) whereas loopbreak only 
> stops a currently running loop, and has no effect if the event loop isn’t 
> running.
> {quote}
> Attached is a patch (against the released 0.9.3 version of the codebase).



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1196: THRIFT-3891 TNonblockingServer configured with mo...

[jeking3]

GitHub user jeking3 opened a pull request:

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

THRIFT-3891 TNonblockingServer configured with more than one IO threads 
does not always return from serve() upon stop()

I took the comments from @nsuke and applied them to the PR.  I was able to 
reasonably eliminate the boolean you mentioned however I could not go back to 
loopbreak.  I see no reasonable way to synchronize here.  The run() thread will 
eventually need to release a mutex and THEN call the event loop.  There's no 
way inside breakLoop() to know if it is actually IN the event loop.  Given 
this, the exit variant is the only safe method to break the loop.

The obsoletes PR #1080.

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

$ git pull https://github.com/jeking3/thrift THRIFT-3891

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

https://github.com/apache/thrift/pull/1196.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 #1196


commit 38d3c87aa11f1f4232cb97b01200c01fc578cd9d
Author: Buğra Gedik 
Date:   2016-09-04T08:18:15Z

THRIFT-3891 TNonblockingServer configured with more than one IO threads 
does not always return from serve() upon stop()




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-3921) C++ code should print enums as strings

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-3921:


Github user asfgit closed the pull request at:

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


> C++ code should print enums as strings
> --
>
> Key: THRIFT-3921
> URL: https://issues.apache.org/jira/browse/THRIFT-3921
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Compiler
>Reporter: Vivek Jain
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> THRIFT-2067 added {{operator<<}} to the C++ generated code, which is great, 
> but enums are printed as numbers rather than their string representation. It 
> would be great if the generated code printed them as a string instead. There 
> might be some backwards-compatibility concerns (if users have already defined 
> their own versions, then thrift's version would cause compile errors), not 
> sure how important you guys think those are.
> Other thoughts/concerns? I am willing to work on a patch if no one else is 
> able to work on it.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-3921) C++ code should print enums as strings

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-3921:


Github user asfgit closed the pull request at:

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


> C++ code should print enums as strings
> --
>
> Key: THRIFT-3921
> URL: https://issues.apache.org/jira/browse/THRIFT-3921
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Compiler
>Reporter: Vivek Jain
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> THRIFT-2067 added {{operator<<}} to the C++ generated code, which is great, 
> but enums are printed as numbers rather than their string representation. It 
> would be great if the generated code printed them as a string instead. There 
> might be some backwards-compatibility concerns (if users have already defined 
> their own versions, then thrift's version would cause compile errors), not 
> sure how important you guys think those are.
> Other thoughts/concerns? I am willing to work on a patch if no one else is 
> able to work on it.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1194: THRIFT-3921 cpp library- optional operator ostrea...

[asfgit]

Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Resolved] (THRIFT-3921) C++ code should print enums as strings

[James E. King, III (JIRA)]

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

James E. King, III resolved THRIFT-3921.

   Resolution: Fixed
Fix Version/s: 0.11.0

> C++ code should print enums as strings
> --
>
> Key: THRIFT-3921
> URL: https://issues.apache.org/jira/browse/THRIFT-3921
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Compiler
>Reporter: Vivek Jain
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> THRIFT-2067 added {{operator<<}} to the C++ generated code, which is great, 
> but enums are printed as numbers rather than their string representation. It 
> would be great if the generated code printed them as a string instead. There 
> might be some backwards-compatibility concerns (if users have already defined 
> their own versions, then thrift's version would cause compile errors), not 
> sure how important you guys think those are.
> Other thoughts/concerns? I am willing to work on a patch if no one else is 
> able to work on it.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1083: THRIFT-3921 Add C++ ostream operator<< functions ...

[asfgit]

Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-2796) Testcase for THRIFT-2793

[James E. King, III (JIRA)]

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

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


[~jensg] Author no longer has a thrift fork; should we complete this or close 
it as won't fix?  All this does is add a .thrift file and doesn't actually use 
the resulting compiled file in any way so it is not a valid test for the issue.

> Testcase for THRIFT-2793
> 
>
> Key: THRIFT-2796
> URL: https://issues.apache.org/jira/browse/THRIFT-2796
> Project: Thrift
>  Issue Type: Test
>  Components: Go - Compiler
>Reporter: Jens Geyer
> Fix For: 1.0
>
>
> Add test for THRIFT-2793 Go compiler produces uncompilable code



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Resolved] (THRIFT-2721) generate more idiomatic erlang code

[James E. King, III (JIRA)]

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

James E. King, III resolved THRIFT-2721.

Resolution: Won't Fix

In comparing this pull request against the current compiler code it looks like 
most of these things were done already so I am resolving this as Won't Fix.

> generate more idiomatic erlang code
> ---
>
> Key: THRIFT-2721
> URL: https://issues.apache.org/jira/browse/THRIFT-2721
> Project: Thrift
>  Issue Type: Improvement
>  Components: Erlang - Compiler
>Reporter: alisdair sullivan
>Assignee: alisdair sullivan
>Priority: Minor
>  Labels: erlang
>
> reduces the amount of code generated by the erlang compiler and increases 
> readability of resulting modules. also adds some more comprehensive tests for 
> the compiler



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift issue #226: THRIFT-2721: reduce complexity of generated erlang code a...

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/226
  
@jfarrell please close this PR per my comments in Jira.  Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-2721) generate more idiomatic erlang code

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-2721:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/226
  
@jfarrell please close this PR per my comments in Jira.  Thanks.


> generate more idiomatic erlang code
> ---
>
> Key: THRIFT-2721
> URL: https://issues.apache.org/jira/browse/THRIFT-2721
> Project: Thrift
>  Issue Type: Improvement
>  Components: Erlang - Compiler
>Reporter: alisdair sullivan
>Assignee: alisdair sullivan
>Priority: Minor
>  Labels: erlang
>
> reduces the amount of code generated by the erlang compiler and increases 
> readability of resulting modules. also adds some more comprehensive tests for 
> the compiler



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-2504) I want a server-side upgrade to MultiplexedProtocol to maintain backwards compatibility with older clients

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-2504:


Github user asfgit closed the pull request at:

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


> I want a server-side upgrade to MultiplexedProtocol to maintain backwards 
> compatibility with older clients
> --
>
> Key: THRIFT-2504
> URL: https://issues.apache.org/jira/browse/THRIFT-2504
> Project: Thrift
>  Issue Type: Improvement
>  Components: Java - Library
>Reporter: Aleksey Pesternikov
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> Multiplexed Protocol provides a number of benefits.  It would be useful for a 
> developer to be able to upgrade a server to use MultiplexedProtocol while 
> still allowing older clients using BinaryProtocol (or others?) to submit 
> their older requests and have them processed as they were before the upgrade, 
> or perhaps at the very least get an exception telling them to upgrade their 
> client.  Right now I believe the behavior of connecting this way is 
> undefined; correct me if I am wrong.
> In THRIFT-66 I handled this by using an unused byte in the VERSION_1 protocol 
> header which was always initialized to zero, so I made "channel zero" 
> something that the server side could implement.  In that solution however 
> both ends continued to use BinaryProtocol so it was easy to maintain 
> backwards compatibility.  In this case we have a server speaking 
> MultiplexedProtocol and a client speaking some other (BinaryProtocol, let's 
> say).  I'm curious what folks who worked on MultiplexedProtocol think about 
> this notion.
> This is one of those changes that would require every language to adopt and 
> be made part of the core requirements of MultiplexedProtocol if people feel 
> it is worth it.  The alternative is that the developer could continue to run 
> an older protocol server on the same port that throws an exception telling 
> the client to upgrade and what port to go to in the message.  This isn't 
> exactly firewall friendly because it needs a new port opened but it is a 
> possible solution.  Thoughts and suggestions welcome as to whether this is 
> worth doing or we should resolve as won't fix.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-2504) I want a server-side upgrade to MultiplexedProtocol to maintain backwards compatibility with older clients

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-2504:


Github user asfgit closed the pull request at:

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


> I want a server-side upgrade to MultiplexedProtocol to maintain backwards 
> compatibility with older clients
> --
>
> Key: THRIFT-2504
> URL: https://issues.apache.org/jira/browse/THRIFT-2504
> Project: Thrift
>  Issue Type: Improvement
>  Components: Java - Library
>Reporter: Aleksey Pesternikov
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> Multiplexed Protocol provides a number of benefits.  It would be useful for a 
> developer to be able to upgrade a server to use MultiplexedProtocol while 
> still allowing older clients using BinaryProtocol (or others?) to submit 
> their older requests and have them processed as they were before the upgrade, 
> or perhaps at the very least get an exception telling them to upgrade their 
> client.  Right now I believe the behavior of connecting this way is 
> undefined; correct me if I am wrong.
> In THRIFT-66 I handled this by using an unused byte in the VERSION_1 protocol 
> header which was always initialized to zero, so I made "channel zero" 
> something that the server side could implement.  In that solution however 
> both ends continued to use BinaryProtocol so it was easy to maintain 
> backwards compatibility.  In this case we have a server speaking 
> MultiplexedProtocol and a client speaking some other (BinaryProtocol, let's 
> say).  I'm curious what folks who worked on MultiplexedProtocol think about 
> this notion.
> This is one of those changes that would require every language to adopt and 
> be made part of the core requirements of MultiplexedProtocol if people feel 
> it is worth it.  The alternative is that the developer could continue to run 
> an older protocol server on the same port that throws an exception telling 
> the client to upgrade and what port to go to in the message.  This isn't 
> exactly firewall friendly because it needs a new port opened but it is a 
> possible solution.  Thoughts and suggestions welcome as to whether this is 
> worth doing or we should resolve as won't fix.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift pull request #1195: THRIFT-2504: Add default processor to java multip...

[asfgit]

Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #114: THRIFT-2504

[asfgit]

Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Resolved] (THRIFT-2504) I want a server-side upgrade to MultiplexedProtocol to maintain backwards compatibility with older clients

[James E. King, III (JIRA)]

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

James E. King, III resolved THRIFT-2504.

   Resolution: Fixed
Fix Version/s: 0.11.0

> I want a server-side upgrade to MultiplexedProtocol to maintain backwards 
> compatibility with older clients
> --
>
> Key: THRIFT-2504
> URL: https://issues.apache.org/jira/browse/THRIFT-2504
> Project: Thrift
>  Issue Type: Improvement
>  Components: Java - Library
>Reporter: Aleksey Pesternikov
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> Multiplexed Protocol provides a number of benefits.  It would be useful for a 
> developer to be able to upgrade a server to use MultiplexedProtocol while 
> still allowing older clients using BinaryProtocol (or others?) to submit 
> their older requests and have them processed as they were before the upgrade, 
> or perhaps at the very least get an exception telling them to upgrade their 
> client.  Right now I believe the behavior of connecting this way is 
> undefined; correct me if I am wrong.
> In THRIFT-66 I handled this by using an unused byte in the VERSION_1 protocol 
> header which was always initialized to zero, so I made "channel zero" 
> something that the server side could implement.  In that solution however 
> both ends continued to use BinaryProtocol so it was easy to maintain 
> backwards compatibility.  In this case we have a server speaking 
> MultiplexedProtocol and a client speaking some other (BinaryProtocol, let's 
> say).  I'm curious what folks who worked on MultiplexedProtocol think about 
> this notion.
> This is one of those changes that would require every language to adopt and 
> be made part of the core requirements of MultiplexedProtocol if people feel 
> it is worth it.  The alternative is that the developer could continue to run 
> an older protocol server on the same port that throws an exception telling 
> the client to upgrade and what port to go to in the message.  This isn't 
> exactly firewall friendly because it needs a new port opened but it is a 
> possible solution.  Thoughts and suggestions welcome as to whether this is 
> worth doing or we should resolve as won't fix.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-3706) There's no support for Multiplexed protocol on c_glib library

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-3706:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1191
  
I added "multiplexed" support to make cross in Travis CI if you rebase on 
master.


> There's no support for Multiplexed protocol on c_glib library
> -
>
> Key: THRIFT-3706
> URL: https://issues.apache.org/jira/browse/THRIFT-3706
> Project: Thrift
>  Issue Type: Improvement
>  Components: C glib - Library
>Affects Versions: 0.9.3
>Reporter: Gonzalo Aguilar
>Assignee: Gonzalo Aguilar
>
> There's no multiplexed protocol. 
> I will implement the same way it's done in Java an C++



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Resolved] (THRIFT-4095) Add multiplexed protocol to Travis CI for make cross

[James E. King, III (JIRA)]

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

James E. King, III resolved THRIFT-4095.

   Resolution: Fixed
Fix Version/s: 0.11.0

> Add multiplexed protocol to Travis CI for make cross
> 
>
> Key: THRIFT-4095
> URL: https://issues.apache.org/jira/browse/THRIFT-4095
> Project: Thrift
>  Issue Type: Improvement
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: Travis CI
>Reporter: James E. King, III
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> In THRIFT-3706 someone wants to contribute our first set of multiplexed cross 
> tests (c_glib client, java server).  These need to be represented in our 
> Travis CI builds, where we are limiting the protocols we test to keep build 
> jobs in a time box.
> Currently we have the following four build jobs running crosstest:
> * #1 uses the ubuntu image and runs "binary|header" on everything except 
> netcore and rust
> * #2 uses the debian image and does the same as #1
> * #3 uses the ubuntu image and runs "compact|json" on everything except 
> netcore and rust
> * #4 uses the debian image and does the same as #3
> The following protocols are not represented in the Travis CI builds and 
> should be added:
> * multiplexed
> There are not many of these.  They could be added to build job #2 which seems 
> to have enough headroom to run the few extra tests this would generate.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1191
  
I added "multiplexed" support to make cross in Travis CI if you rebase on 
master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Assigned] (THRIFT-4095) Add multiplexed protocol to Travis CI for make cross

[James E. King, III (JIRA)]

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

James E. King, III reassigned THRIFT-4095:
--

Assignee: James E. King, III  (was: Jake Farrell)

> Add multiplexed protocol to Travis CI for make cross
> 
>
> Key: THRIFT-4095
> URL: https://issues.apache.org/jira/browse/THRIFT-4095
> Project: Thrift
>  Issue Type: Improvement
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: Travis CI
>Reporter: James E. King, III
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> In THRIFT-3706 someone wants to contribute our first set of multiplexed cross 
> tests (c_glib client, java server).  These need to be represented in our 
> Travis CI builds, where we are limiting the protocols we test to keep build 
> jobs in a time box.
> Currently we have the following four build jobs running crosstest:
> * #1 uses the ubuntu image and runs "binary|header" on everything except 
> netcore and rust
> * #2 uses the debian image and does the same as #1
> * #3 uses the ubuntu image and runs "compact|json" on everything except 
> netcore and rust
> * #4 uses the debian image and does the same as #3
> The following protocols are not represented in the Travis CI builds and 
> should be added:
> * multiplexed
> There are not many of these.  They could be added to build job #2 which seems 
> to have enough headroom to run the few extra tests this would generate.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-4095) Add multiplexed protocol to Travis CI for make cross

[James E. King, III (JIRA)]

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

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


I added "multiplexed" to jobs 1/2 and will have THRIFT-3706 change to use 
"multiplexed" for said tests.

> Add multiplexed protocol to Travis CI for make cross
> 
>
> Key: THRIFT-4095
> URL: https://issues.apache.org/jira/browse/THRIFT-4095
> Project: Thrift
>  Issue Type: Improvement
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: Travis CI
>Reporter: James E. King, III
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> In THRIFT-3706 someone wants to contribute our first set of multiplexed cross 
> tests (c_glib client, java server).  These need to be represented in our 
> Travis CI builds, where we are limiting the protocols we test to keep build 
> jobs in a time box.
> Currently we have the following four build jobs running crosstest:
> * #1 uses the ubuntu image and runs "binary|header" on everything except 
> netcore and rust
> * #2 uses the debian image and does the same as #1
> * #3 uses the ubuntu image and runs "compact|json" on everything except 
> netcore and rust
> * #4 uses the debian image and does the same as #3
> The following protocols are not represented in the Travis CI builds and 
> should be added:
> * multiplexed
> There are not many of these.  They could be added to build job #2 which seems 
> to have enough headroom to run the few extra tests this would generate.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-4095) Add multiplexed and other missing protocols to Travis CI

[James E. King, III (JIRA)]

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

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


Thanks!  I was not aware that they were driven by a file in the distribution.  
Makes good sense though :)

> Add multiplexed and other missing protocols to Travis CI
> 
>
> Key: THRIFT-4095
> URL: https://issues.apache.org/jira/browse/THRIFT-4095
> Project: Thrift
>  Issue Type: Improvement
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: Travis CI
>Reporter: James E. King, III
>Assignee: Jake Farrell
>
> In THRIFT-3706 someone wants to contribute our first set of multiplexed cross 
> tests (c_glib client, java server).  These need to be represented in our 
> Travis CI builds, where we are limiting the protocols we test to keep build 
> jobs in a time box.
> Currently we have the following four build jobs running crosstest:
> * #1 uses the ubuntu image and runs "binary|header" on everything except 
> netcore and rust
> * #2 uses the debian image and does the same as #1
> * #3 uses the ubuntu image and runs "compact|json" on everything except 
> netcore and rust
> * #4 uses the debian image and does the same as #3
> The following protocols are not represented in the Travis CI builds and 
> should be added:
> * multiplexed
> There are not many of these.  They could be added to build job #2 which seems 
> to have enough headroom to run the few extra tests this would generate.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Updated] (THRIFT-4095) Add multiplexed protocol to Travis CI for make cross

[James E. King, III (JIRA)]

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

James E. King, III updated THRIFT-4095:
---
Summary: Add multiplexed protocol to Travis CI for make cross  (was: Add 
multiplexed and other missing protocols to Travis CI)

> Add multiplexed protocol to Travis CI for make cross
> 
>
> Key: THRIFT-4095
> URL: https://issues.apache.org/jira/browse/THRIFT-4095
> Project: Thrift
>  Issue Type: Improvement
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: Travis CI
>Reporter: James E. King, III
>Assignee: Jake Farrell
>
> In THRIFT-3706 someone wants to contribute our first set of multiplexed cross 
> tests (c_glib client, java server).  These need to be represented in our 
> Travis CI builds, where we are limiting the protocols we test to keep build 
> jobs in a time box.
> Currently we have the following four build jobs running crosstest:
> * #1 uses the ubuntu image and runs "binary|header" on everything except 
> netcore and rust
> * #2 uses the debian image and does the same as #1
> * #3 uses the ubuntu image and runs "compact|json" on everything except 
> netcore and rust
> * #4 uses the debian image and does the same as #3
> The following protocols are not represented in the Travis CI builds and 
> should be added:
> * multiplexed
> There are not many of these.  They could be added to build job #2 which seems 
> to have enough headroom to run the few extra tests this would generate.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Updated] (THRIFT-4095) Add multiplexed and other missing protocols to Travis CI

[James E. King, III (JIRA)]

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

James E. King, III updated THRIFT-4095:
---
Description: 
In THRIFT-3706 someone wants to contribute our first set of multiplexed cross 
tests (c_glib client, java server).  These need to be represented in our Travis 
CI builds, where we are limiting the protocols we test to keep build jobs in a 
time box.

Currently we have the following four build jobs running crosstest:

* #1 uses the ubuntu image and runs "binary|header" on everything except 
netcore and rust
* #2 uses the debian image and does the same as #1
* #3 uses the ubuntu image and runs "compact|json" on everything except netcore 
and rust
* #4 uses the debian image and does the same as #3

The following protocols are not represented in the Travis CI builds and should 
be added:

* multiplexed

There are not many of these.  They could be added to build job #2 which seems 
to have enough headroom to run the few extra tests this would generate.

  was:
In THRIFT-3706 someone wants to contribute our first set of multiplexed cross 
tests (c_glib client, java server).  These need to be represented in our Travis 
CI builds, where we are limiting the protocols we test to keep build jobs in a 
time box.

Currently we have the following four build jobs running crosstest:

* #1 uses the ubuntu image and runs "binary|header" on everything except 
netcore and rust
* #2 uses the debian image and does the same as #1
* #3 uses the ubuntu image and runs "compact|json" on everything except netcore 
and rust
* #4 uses the debian image and does the same as #3

The following protocols are not represented in the Travis CI builds and should 
be added:

* header
* multiplexed

There are not many of these.  They could be added to build job #2 which seems 
to have enough headroom to run the few extra tests this would generate.


> Add multiplexed and other missing protocols to Travis CI
> 
>
> Key: THRIFT-4095
> URL: https://issues.apache.org/jira/browse/THRIFT-4095
> Project: Thrift
>  Issue Type: Improvement
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: Travis CI
>Reporter: James E. King, III
>Assignee: Jake Farrell
>
> In THRIFT-3706 someone wants to contribute our first set of multiplexed cross 
> tests (c_glib client, java server).  These need to be represented in our 
> Travis CI builds, where we are limiting the protocols we test to keep build 
> jobs in a time box.
> Currently we have the following four build jobs running crosstest:
> * #1 uses the ubuntu image and runs "binary|header" on everything except 
> netcore and rust
> * #2 uses the debian image and does the same as #1
> * #3 uses the ubuntu image and runs "compact|json" on everything except 
> netcore and rust
> * #4 uses the debian image and does the same as #3
> The following protocols are not represented in the Travis CI builds and 
> should be added:
> * multiplexed
> There are not many of these.  They could be added to build job #2 which seems 
> to have enough headroom to run the few extra tests this would generate.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-4095) Add multiplexed and other missing protocols to Travis CI

[Aki Sukegawa (JIRA)]

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

Aki Sukegawa commented on THRIFT-4095:
--

[~jking3] tests for header servers are included in our CI.
https://travis-ci.org/apache/thrift/jobs/201991012#L7702
They are located in test/features and can be run by test/test.py --features or 
make crossfeature
I'm afraid that the only documentation available is a single sentence in 
test/test.py --help .

To add tests to existing or new jobs, any contributor or committer can edit 
.travis-ci.yml and issue a pull request.

> Add multiplexed and other missing protocols to Travis CI
> 
>
> Key: THRIFT-4095
> URL: https://issues.apache.org/jira/browse/THRIFT-4095
> Project: Thrift
>  Issue Type: Improvement
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: Travis CI
>Reporter: James E. King, III
>Assignee: Jake Farrell
>
> In THRIFT-3706 someone wants to contribute our first set of multiplexed cross 
> tests (c_glib client, java server).  These need to be represented in our 
> Travis CI builds, where we are limiting the protocols we test to keep build 
> jobs in a time box.
> Currently we have the following four build jobs running crosstest:
> * #1 uses the ubuntu image and runs "binary|header" on everything except 
> netcore and rust
> * #2 uses the debian image and does the same as #1
> * #3 uses the ubuntu image and runs "compact|json" on everything except 
> netcore and rust
> * #4 uses the debian image and does the same as #3
> The following protocols are not represented in the Travis CI builds and 
> should be added:
> * header
> * multiplexed
> There are not many of these.  They could be added to build job #2 which seems 
> to have enough headroom to run the few extra tests this would generate.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Created] (THRIFT-4096) Remove unnecessary duplication of build/test effort in Travis CI

[James E. King, III (JIRA)]

James E. King, III created THRIFT-4096:
--

 Summary: Remove unnecessary duplication of build/test effort in 
Travis CI
 Key: THRIFT-4096
 URL: https://issues.apache.org/jira/browse/THRIFT-4096
 Project: Thrift
  Issue Type: Improvement
  Components: Build Process
Affects Versions: 0.11.0
 Environment: Travis CI
Reporter: James E. King, III
Assignee: Jake Farrell


Build jobs #1 and #3 use ubuntu.
Build jobs #2 and #4 use debian.

Both of these are long running jobs that run "make cross" and test the same 
thing twice.  Further, ubuntu is a derivative of debian which makes the testing 
somewhat more duplicated.  There are also build jobs like #23 which build 
everything yet again in ubuntu and produce debian packages, so we build the 
same thing many times in Travis CI.   Recently we had the number of build jobs 
limited because we were using too much resource from Apache's Travis CI slave 
pool so we should be good citizens and try to pare down unnecessary build jobs 
while maintaining quality.

We should discuss removing the ubuntu jobs and docker environment in favor of 
the distribution it originates from and whether that makes sense.  In addition, 
there has been renewed interest from the Fedora project to build thrift and I 
think we would get broader adoption if we had a Fedora docker image that builds 
thrift and runs make cross.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Updated] (THRIFT-4095) Add multiplexed and other missing protocols to Travis CI

[James E. King, III (JIRA)]

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

James E. King, III updated THRIFT-4095:
---
Priority: Major  (was: Critical)

> Add multiplexed and other missing protocols to Travis CI
> 
>
> Key: THRIFT-4095
> URL: https://issues.apache.org/jira/browse/THRIFT-4095
> Project: Thrift
>  Issue Type: Improvement
>  Components: Build Process
>Affects Versions: 0.11.0
> Environment: Travis CI
>Reporter: James E. King, III
>Assignee: Jake Farrell
>
> In THRIFT-3706 someone wants to contribute our first set of multiplexed cross 
> tests (c_glib client, java server).  These need to be represented in our 
> Travis CI builds, where we are limiting the protocols we test to keep build 
> jobs in a time box.
> Currently we have the following four build jobs running crosstest:
> * #1 uses the ubuntu image and runs "binary|header" on everything except 
> netcore and rust
> * #2 uses the debian image and does the same as #1
> * #3 uses the ubuntu image and runs "compact|json" on everything except 
> netcore and rust
> * #4 uses the debian image and does the same as #3
> The following protocols are not represented in the Travis CI builds and 
> should be added:
> * header
> * multiplexed
> There are not many of these.  They could be added to build job #2 which seems 
> to have enough headroom to run the few extra tests this would generate.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Created] (THRIFT-4095) Add multiplexed and other missing protocols to Travis CI

[James E. King, III (JIRA)]

James E. King, III created THRIFT-4095:
--

 Summary: Add multiplexed and other missing protocols to Travis CI
 Key: THRIFT-4095
 URL: https://issues.apache.org/jira/browse/THRIFT-4095
 Project: Thrift
  Issue Type: Improvement
  Components: Build Process
Affects Versions: 0.11.0
 Environment: Travis CI
Reporter: James E. King, III
Assignee: Jake Farrell
Priority: Critical


In THRIFT-3706 someone wants to contribute our first set of multiplexed cross 
tests (c_glib client, java server).  These need to be represented in our Travis 
CI builds, where we are limiting the protocols we test to keep build jobs in a 
time box.

Currently we have the following four build jobs running crosstest:

* #1 uses the ubuntu image and runs "binary|header" on everything except 
netcore and rust
* #2 uses the debian image and does the same as #1
* #3 uses the ubuntu image and runs "compact|json" on everything except netcore 
and rust
* #4 uses the debian image and does the same as #3

The following protocols are not represented in the Travis CI builds and should 
be added:

* header
* multiplexed

There are not many of these.  They could be added to build job #2 which seems 
to have enough headroom to run the few extra tests this would generate.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-2504) I want a server-side upgrade to MultiplexedProtocol to maintain backwards compatibility with older clients

[James E. King, III (JIRA)]

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

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


[~jfarrell] Given mockito is only used during the build process and not 
distributed with the project (the build downloads it on the build user's 
computer, so it is their action that acquires it, not Apache Thrift), does it 
need to be in the LICENSE file?

> I want a server-side upgrade to MultiplexedProtocol to maintain backwards 
> compatibility with older clients
> --
>
> Key: THRIFT-2504
> URL: https://issues.apache.org/jira/browse/THRIFT-2504
> Project: Thrift
>  Issue Type: Improvement
>  Components: Java - Library
>Reporter: Aleksey Pesternikov
>Assignee: James E. King, III
>
> Multiplexed Protocol provides a number of benefits.  It would be useful for a 
> developer to be able to upgrade a server to use MultiplexedProtocol while 
> still allowing older clients using BinaryProtocol (or others?) to submit 
> their older requests and have them processed as they were before the upgrade, 
> or perhaps at the very least get an exception telling them to upgrade their 
> client.  Right now I believe the behavior of connecting this way is 
> undefined; correct me if I am wrong.
> In THRIFT-66 I handled this by using an unused byte in the VERSION_1 protocol 
> header which was always initialized to zero, so I made "channel zero" 
> something that the server side could implement.  In that solution however 
> both ends continued to use BinaryProtocol so it was easy to maintain 
> backwards compatibility.  In this case we have a server speaking 
> MultiplexedProtocol and a client speaking some other (BinaryProtocol, let's 
> say).  I'm curious what folks who worked on MultiplexedProtocol think about 
> this notion.
> This is one of those changes that would require every language to adopt and 
> be made part of the core requirements of MultiplexedProtocol if people feel 
> it is worth it.  The alternative is that the developer could continue to run 
> an older protocol server on the same port that throws an exception telling 
> the client to upgrade and what port to go to in the message.  This isn't 
> exactly firewall friendly because it needs a new port opened but it is a 
> possible solution.  Thoughts and suggestions welcome as to whether this is 
> worth doing or we should resolve as won't fix.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Assigned] (THRIFT-2504) I want a server-side upgrade to MultiplexedProtocol to maintain backwards compatibility with older clients

[James E. King, III (JIRA)]

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

James E. King, III reassigned THRIFT-2504:
--

Assignee: James E. King, III

> I want a server-side upgrade to MultiplexedProtocol to maintain backwards 
> compatibility with older clients
> --
>
> Key: THRIFT-2504
> URL: https://issues.apache.org/jira/browse/THRIFT-2504
> Project: Thrift
>  Issue Type: Improvement
>  Components: Java - Library
>Reporter: Aleksey Pesternikov
>Assignee: James E. King, III
>
> Multiplexed Protocol provides a number of benefits.  It would be useful for a 
> developer to be able to upgrade a server to use MultiplexedProtocol while 
> still allowing older clients using BinaryProtocol (or others?) to submit 
> their older requests and have them processed as they were before the upgrade, 
> or perhaps at the very least get an exception telling them to upgrade their 
> client.  Right now I believe the behavior of connecting this way is 
> undefined; correct me if I am wrong.
> In THRIFT-66 I handled this by using an unused byte in the VERSION_1 protocol 
> header which was always initialized to zero, so I made "channel zero" 
> something that the server side could implement.  In that solution however 
> both ends continued to use BinaryProtocol so it was easy to maintain 
> backwards compatibility.  In this case we have a server speaking 
> MultiplexedProtocol and a client speaking some other (BinaryProtocol, let's 
> say).  I'm curious what folks who worked on MultiplexedProtocol think about 
> this notion.
> This is one of those changes that would require every language to adopt and 
> be made part of the core requirements of MultiplexedProtocol if people feel 
> it is worth it.  The alternative is that the developer could continue to run 
> an older protocol server on the same port that throws an exception telling 
> the client to upgrade and what port to go to in the message.  This isn't 
> exactly firewall friendly because it needs a new port opened but it is a 
> possible solution.  Thoughts and suggestions welcome as to whether this is 
> worth doing or we should resolve as won't fix.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (THRIFT-2471) Make cpp.ref annotation language agnostic

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-2471:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/113
  
Sorry, #1195 has nothing to do with this one.  I was off by one.


> Make cpp.ref annotation language agnostic
> -
>
> Key: THRIFT-2471
> URL: https://issues.apache.org/jira/browse/THRIFT-2471
> Project: Thrift
>  Issue Type: Improvement
>  Components: Compiler (General)
>Reporter: Jens Geyer
>Assignee: Jens Geyer
> Fix For: 0.9.2
>
> Attachments: boost_1.40.0__to__1.54.0-patch
>
>
> The proposal is to make the new {{cpp.ref}} annotation introduced with 
> THRIFT-2421 language agnostic instead of a C++ specialty only. 
> The annotation changes inlined nested structs into pointers to structs. This 
> behaviour is potentially relevant with all languages using value semantics 
> for nested structs etc.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift issue #113: THRIFT-2471

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/113
  
Sorry, #1195 has nothing to do with this one.  I was off by one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-2504) I want a server-side upgrade to MultiplexedProtocol to maintain backwards compatibility with older clients

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-2504:


GitHub user jeking3 opened a pull request:

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

THRIFT-2504: Add default processor to java multiplexed processor to handle 
older clients

Supercedes PR #113 - I fixed the test case so it actually runs and added a 
missing test case.

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

$ git pull https://github.com/jeking3/thrift THRIFT-2504

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

https://github.com/apache/thrift/pull/1195.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 #1195


commit b37938861fb7a6d5d66f7d92f7867469aa8393a7
Author: Aleksey Pesternikov 
Date:   2014-05-01T20:58:18Z

THRIFT-2504: Add default processor to java multiplexed processor to handle 
older clients




> I want a server-side upgrade to MultiplexedProtocol to maintain backwards 
> compatibility with older clients
> --
>
> Key: THRIFT-2504
> URL: https://issues.apache.org/jira/browse/THRIFT-2504
> Project: Thrift
>  Issue Type: Improvement
>  Components: Java - Library
>Reporter: Aleksey Pesternikov
>
> Multiplexed Protocol provides a number of benefits.  It would be useful for a 
> developer to be able to upgrade a server to use MultiplexedProtocol while 
> still allowing older clients using BinaryProtocol (or others?) to submit 
> their older requests and have them processed as they were before the upgrade, 
> or perhaps at the very least get an exception telling them to upgrade their 
> client.  Right now I believe the behavior of connecting this way is 
> undefined; correct me if I am wrong.
> In THRIFT-66 I handled this by using an unused byte in the VERSION_1 protocol 
> header which was always initialized to zero, so I made "channel zero" 
> something that the server side could implement.  In that solution however 
> both ends continued to use BinaryProtocol so it was easy to maintain 
> backwards compatibility.  In this case we have a server speaking 
> MultiplexedProtocol and a client speaking some other (BinaryProtocol, let's 
> say).  I'm curious what folks who worked on MultiplexedProtocol think about 
> this notion.
> This is one of those changes that would require every language to adopt and 
> be made part of the core requirements of MultiplexedProtocol if people feel 
> it is worth it.  The alternative is that the developer could continue to run 
> an older protocol server on the same port that throws an exception telling 
> the client to upgrade and what port to go to in the message.  This isn't 
> exactly firewall friendly because it needs a new port opened but it is a 
> possible solution.  Thoughts and suggestions welcome as to whether this is 
> worth doing or we should resolve as won't fix.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift issue #1156: THRIFT-4011 Use slices for Thrift sets

[dcelasun]

Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1156
  
Whoops, I must have missed that one. Should be fixed now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (THRIFT-4011) Sets of Thrift structs generate Go code that can't be serialized to JSON

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4011:


Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1156
  
```
src/common/clientserver_test.go:61: cannot use handler (type 
*MockThriftTest) as type thrifttest.ThriftTest in argument to GetServerParams:
*MockThriftTest does not implement thrifttest.ThriftTest (wrong type 
for TestSet method)
have TestSet(map[int32]struct {}) (map[int32]struct {}, error)
want TestSet([]int32) ([]int32, error)
src/common/clientserver_test.go:226: cannot use s (type map[int32]struct 
{}) as type []int32 in argument to client.TestSet
FAILcommon [build failed]
make[2]: *** [check] Error 2
make[2]: Leaving directory `/thrift/src/test/go'
make[1]: *** [check-recursive] Error 1
make[1]: Leaving directory `/thrift/src/test'
make: *** [check-recursive] Error 1

```


> Sets of Thrift structs generate Go code that can't be serialized to JSON
> 
>
> Key: THRIFT-4011
> URL: https://issues.apache.org/jira/browse/THRIFT-4011
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Reporter: Can Celasun
>
> Consider the following structs:
> {code}
> struct Foo {
>   1: optional string foo
> }
> struct Bar {
>   1: optional set foos
> }
> {code}
> This compiles into the following Go code:
> {code}
> type Bar struct {
>   Foos map[*Foo]struct{} `thrift:"foos,1" db:"foos" json:"foos,omitempty"`
> }
> {code}
> Even though the generated code has tags for JSON support, Bar can't be 
> serialized to JSON:
> {code}
> json: unsupported type: map[*Foo]struct {}
> {code}
> One solution would be to use slices, not maps, for Thrift sets. Thoughts?



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] thrift issue #1156: THRIFT-4011 Use slices for Thrift sets

[Jens-G]

Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1156
  
```
src/common/clientserver_test.go:61: cannot use handler (type 
*MockThriftTest) as type thrifttest.ThriftTest in argument to GetServerParams:
*MockThriftTest does not implement thrifttest.ThriftTest (wrong type 
for TestSet method)
have TestSet(map[int32]struct {}) (map[int32]struct {}, error)
want TestSet([]int32) ([]int32, error)
src/common/clientserver_test.go:226: cannot use s (type map[int32]struct 
{}) as type []int32 in argument to client.TestSet
FAILcommon [build failed]
make[2]: *** [check] Error 2
make[2]: Leaving directory `/thrift/src/test/go'
make[1]: *** [check-recursive] Error 1
make[1]: Leaving directory `/thrift/src/test'
make: *** [check-recursive] Error 1

```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---