[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Hi @jeking3 it was a hard feature to merge... Hope we will do finer next time. Thank you for your continued support. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 I tried to debug the Rust thing. But it seems is not receiving the response from c_glib testNest in my case. I debugged the server and the response is sent, and flushed. So I don't catch why RS is not receiving it. In fact the client stalls waiting a response. I'm more convinced now that it should be looked by a Rust develper. Enable c++ test if you want and I will take a look if they fail against c_glib. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Just curious that The error messages I get are different. In fact they got blocked on the RS (Rust?) client, at least it seems to. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 But I don't know about the languages that are failing. This is why I told you to open other issue and merge this. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Can it be because the SSL server. That's not implemented on c_glib? ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Hi @jeking3 can you merge please? ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Hi @jeking3 I think at the end it was part of the test client... What do you think? It worth it merge it? ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 In fact @jeking3 Some of the failures are not related to this issue. So please don't ask me to fix them as part of this issue. One of them is caused in the Java Server because the exception management. Maybe caused by the c_glib client. But I'm NOT implementing the client on this issue but the server. For example: [java] 2017-11-13 18:35:41,759 ERROR thrift.ProcessFunction (ProcessFunction.java:process(38)) - Internal error processing testException [java] org.apache.thrift.TException: TException [java] at org.apache.thrift.server.ServerTestBase$TestHandler.testException(ServerTestBase.java:237) [java] at thrift.test.ThriftTest$Processor$testException.getResult(ThriftTest.java:2089) [java] at thrift.test.ThriftTest$Processor$testException.getResult(ThriftTest.java:2068) [java] at org.apache.thrift.ProcessFunction.process(ProcessFunction.java:36) [java] at org.apache.thrift.TBaseProcessor.process(TBaseProcessor.java:39) [java] at org.apache.thrift.TMultiplexedProcessor.process(TMultiplexedProcessor.java:134) [java] at org.apache.thrift.server.TThreadPoolServer$WorkerProcess.run(TThreadPoolServer.java:291) [java] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) [java] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) [java] at java.lang.Thread.run(Thread.java:745) ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 What I'm telling you is that I need help if you want to fix everything on this language. I don't have all the time to implement the full stack. This is an implementation of the processor, not all protocols and transports supported. So if we want to have a full c_glib implementation we should break into smaller issues and give them to different developers. The current implementation is tested on the basic protocols and transports with some languages (not all) if something is broken, then another issue has to be raised to implement that part on the right way. I cannot afford do the full stack implementation for c_glib in one shot. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 @jeking3 I don't think is an error, but that all this cross tests are directly not supported. No all protocols are implemented. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Yes but this seems to be in other languages that are not in my scope. So it should be others that fix them. So I think it's good enough for now. Can you merge it? And maybe open a new bug to trac them? ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 4197.6 Fails... I'm done. Don't know what else to do. Please check what is it... ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 @jeking3 It doesn't fail for me, even in the docker container. Please review and merge. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Ok I was unable to run as you said in travis. But the errors the code was giving was because somewhere in build system is configured to compile some binaries as C90. The errors where mostly comments and some initialization C99 style variables. Most of them outside the code I touched on this feature. It now there's a requirement to write code with C90 style we have a problem because generators are not ready. In fact I had to disable parts of the compilation stuff to check where the problem were because the generated code is invalid in C90 style. I think the build system has to be updated or the rules to write code to be updated. So please if this fails again, I will let you correct because you know a lot more about the build system than I do. Thank you for your support. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 With the new image build/docker/scripts/autotools.sh ran ok. I'm running the other... But no error sofar... I will try to add -pendantic to the compilation if it doesn't have it in my host ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Unfortunately it doesn't work for me on the second command. dockerrun ubuntu-xenial Unable to find image 'ubuntu-xenial:latest' locally docker: Error response from daemon: repository ubuntu-xenial not found: does not exist or no pull access. I'm trying to simulate the docker build in my desktop. It seems that some C99 flags are added to the build done by travis. Something is not in the normal builds. This should be resolved at code level not at travis build level. Anyway I will check why in travis it fails. Thank you for pointing out this to me. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 @jeking3 I don't know why some compilations fails. Maybe you could check because I don't know about what travis is doing. For me seems ok. Our tests are now ok. I added some fixes to the tests. I see failures for some languages but I think the problem should be there since some other languages are working properly ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Please merge it or give me a reason to not do it. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 I finally removed the binary protocol from the list of the limits feature tests. I don't know if I did right but it seems so. This bug also contains important bugfixes for the connection management. So I hope we can merge it soon. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Rebased and lots of fixes added. Crossing fingers. I should opened a new issue but since all bug fixing were discovered while doing this incident I took the opportunity to fix everything here. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 @jeking3 Do you think it would be hard to resolve? If you need me to do something. Can you point me the right direction. Remember to remove the binary protocols from the cross tests since they don't support the limi xxx parameter. ---
[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 Jus great, thank you! ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Someone broke the build system or there's a patch that made things worse. Anyway. Check tests manually. I think it's ready to be merged. In fact what I solved in last commit was not related to my changes, anyway I fixed. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 @jeking3 It will fail because binary protocols doesn't use limit string parameter. Can you remove them from tests since it's not supported. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Yes, there are a couple of them on the wild. I will take a look ASAP. I had three of my servers broke at the same time this week and I'm was crazy until today to bring them up again. Give me a couple of days. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Ok. Fixed a bug in the test client. And added the default protocol for the multiplexed. That way the first registered protocol becomes default. This allows you to use the multi without much hassle. But you can always override the default protocol using a property. Nice... ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 What I'm thinking looking at Java implementation is that on c_glib we can set a property in the processor to define what's the default handler to be get in case of a not multi client. Let me see if I can implement it. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1361
@jeking3 I don't think binary and multi server can be compatible. In fact I
see it failing everytime and this is why I didn't set your protocol
recommendations.
Let me explain. When a multiX server is set, the processor allows you to
register several processors. Each of this processors can implement different
services. If you are contacting with a binary client (not multi) when you send
the message it's not prefixed by the service name. It means that at the server
end the server will never know what's the destination processor for that
request.
So when you use a binary client, you must use a binary server. And with the
multi happens the same.
Apart from this, the protocol is implemented, my problem is that I was
confused with the protocols in the json file and I don't know what should I
really enable...
Besides this I ran it manually against binary multi client and ran ok.
test_client:test_client:93: libtool wrapper (GNU libtool) 2.4.6
Debian-2.4.6-1
test_client:test_client:114: newargv[0]:
/home/gaguilar/thrift/thrift/test/c_glib/.libs/test_client
test_client:test_client:104: newargv[1]: --protocol=multi
test_client:test_client:104: newargv[2]: --transport=buffered
test_client:test_client:104: newargv[3]: --port=39347
Connecting (buffered/binary:multi) to: ip/localhost:39347
Test #1, connect localhost:39347
testVoid() = void
testString("Test") = "Test"
testSecondServiceMultiplexSecondTestString("2nd") = "2nd"
testByte(true) = true
testByte(false) = false
testByte(1) = 1
testByte(-1) = -1
testI32(-1) = -1
testI64(-34359738368) = -34359738368
testDouble(-5.2098523) = -5.209852
testBinary(empty) = empty
testBinary([-128..127]) =
{-128,-127,-126,-125,-124,-123,-122,-121,-120,-119,-118,-117,-116,-115,-114,-113,-112,-111,-110,-109,-108,-107,-106,-105,-104,-103,-102,-101,-100,-99,-98,-97,-96,-95,-94,-93,-92,-91,-90,-89,-88,-87,-86,-85,-84,-83,-82,-81,-80,-79,-78,-77,-76,-75,-74,-73,-72,-71,-70,-69,-68,-67,-66,-65,-64,-63,-62,-61,-60,-59,-58,-57,-56,-55,-54,-53,-52,-51,-50,-49,-48,-47,-46,-45,-44,-43,-42,-41,-40,-39,-38,-37,-36,-35,-34,-33,-32,-31,-30,-29,-28,-27,-26,-25,-24,-23,-22,-21,-20,-19,-18,-17,-16,-15,-14,-13,-12,-11,-10,-9,-8,-7,-6,-5,-4,-3,-2,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,108,109,110,111,112,113,114,115,116,117,118,119,120,121,122,123,124,125,126,127}
OK size 256 OK
testStruct({"Zero", 1, -3, -5}) = {"Zero", 1, -3, -5}
testNest({1, {"Zero", 1, -3, -5}), 5} = {1, {"Zero", 1, -3, -5}, 5}
testMap({0 => -10, 1 => -9, 3 => -7, 2 => -8, 4 => -6}) = {0 => -10, 1 =>
-9, 3 => -7, 2 => -8, 4 => -6}
testStringMap({"some" => "thing", "a" => "2", "b" => "blah")} = {"some" =>
"thing", "a" => "2", "b" => "blah"}
testSet({0, 2, -1, -2, 1}) = {1, -1, 0, 2, -2}
testList({-2, -1, 0, 1, 2}) = {-2, -1, 0, 1, 2}
testEnum(ONE) = 1
testEnum(TWO) = 2
testEnum(THREE) = 3
testEnum(FIVE) = 5
testEnum(EIGHT) = 8
testTypedef(309858235082523) = 309858235082523
testMapMap(1) = {-4 => {-4 => -4, -3 => -3, -2 => -2, -1 => -1, }, 4 => {1
=> 1, 2 => 2, 3 => 3, 4 => 4, }, }
testInsanity() = {1 => {2 => {{8 => 8, 5 => 5, }, {{"Goodbye4", 4, 4, 4},
{"Hello2", 2, 2, 2}, }}, 3 => {{8 => 8, 5 => 5, }, {{"Goodbye4", 4, 4, 4},
{"Hello2", 2, 2, 2}, }}, }, 2 => {6 => {{}, {}}, }, }
testClient.testException("Xception") => {1001, "Xception"}
testClient.testException("TException") => Caught TException
testClient.testException("success") => void
testClient.testMultiException("Xception", "test 1") => {1001, "This is an
Xception"}
testClient.testMultiException("Xception2", "test 2") => {2002, {"This is
an Xception2"}}
testClient.testMultiException("success", "test 3") => {{"test 3"}}
testClient.testOneway(1) => success - took 0.03 ms
re-test testI32(-1) = -1
Total time: 25934720 us
All tests done.
Number of failures: 1
Min time: 25934720 us
Max time: 25934720 us
Avg time: 25934720 us
So please check what protocol should be enabled and enable them. If it
fails again I will check.
---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 What's multic and multi. What's the difference? ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Now seems ok. This is another big change. So I will try to do cross tests after. Meanwhile I cross validated with Java. Already in production with this stuff... ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Perfect, now fails on things not related to my changes. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 It seems the protocol decorator was removed, I suppose I copied and pasted wrong... I always forget that the build system is Cmake and forget to test it against it. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 I just went back in history and pushed again after rebase. Now it should be ok ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 rebased ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Ok. I will do it. ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 @jeking3 Can you merge this, please? ---
[GitHub] thrift issue #1361: THRIFT-4329: Implement multiplexed processor that matche...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 @jeking3 a memory leak is getting me crazy maybe we can check together. In the multiplexed protocol, line 169 we should release that protocol but if I do it the code seems to fail with memory dump in other point. I think the chained destructor is making me little crazy. I will still check but for now we should get this out. ---
[GitHub] thrift issue #1361: Implement multiplexed processor that matches CPP and Jav...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 Hi @jeking3 don't worry I will fix the binary protocol memory leak. About the build I think this is failing because foreign problem, not by my changes. Cause it fails for decorator protocol that was already there since long time. Can you check it please? And merge it? ---
[GitHub] thrift issue #1361: Implement multiplexed processor that matches CPP and Jav...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1361 We must also investigate the binary protocol in C since I think is leaking memory everywhere: ==4534== 2,399,952 bytes in 99,998 blocks are definitely lost in loss record 611 of 611 ==4534==at 0x4C2FB25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==4534==by 0x5300580: g_malloc0 (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5306.0) ==4534==by 0x4E49774: thrift_binary_protocol_read_string (thrift_binary_protocol.c:797) ==4534==by 0x4E48E55: thrift_binary_protocol_read_message_begin (thrift_binary_protocol.c:431) ==4534==by 0x4E46013: thrift_multiplexed_processor_process_impl (thrift_multiplexed_processor.c:60) ==4534==by 0x4E51864: thrift_simple_server_serve (thrift_simple_server.c:58) ==4534==by 0x108DAF: test_server_initialize (test_server.c:74) ==4534==by 0x5321459: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5306.0) ==4534==by 0x532138A: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5306.0) ==4534==by 0x5321631: g_test_run_suite (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5306.0) ---
[GitHub] thrift pull request #1361: Implement multiplexed processor that matches CPP ...
GitHub user gadLinux opened a pull request: https://github.com/apache/thrift/pull/1361 Implement multiplexed processor that matches CPP and Java. Tested aga⦠Implement multiplexed processor that matches CPP and Java. Tested against Java Missing Test suite Implement stored message protocol that adds the possibility of reading a header of a message and then process it I don't remember how to do cross tests from protocols. I want to test Java and C multiplexors against a C server. I did in my test units but Don't find the way here. I did it before so I will check. For now it's safe to merge this. You can merge this pull request into a Git repository by running: $ git pull https://github.com/gadLinux/thrift THRIFT-4329 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1361.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 #1361 commit 1d1d9d819d4825ea71649d00ed85307071252bd3 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2017-09-15T10:26:02Z Implement multiplexed processor that matches CPP and Java. Tested against Java Missing Test suite Implement stored message protocol that adds the posibility of reading a header of a message and then process it ---
[GitHub] thrift issue #1272: THRIFT-4205: Set flags for glib+gobject on compilation
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1272 It seems that the THRIFT-4202 is resolved can we merge 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. ---
[GitHub] thrift pull request #1279: THRIFT-4212: Fix flush on ssl socket thrift
GitHub user gadLinux opened a pull request: https://github.com/apache/thrift/pull/1279 THRIFT-4212: Fix flush on ssl socket thrift Fixes the bug in flush on ssl thrift communications transport You can merge this pull request into a Git repository by running: $ git pull https://github.com/gadLinux/thrift THRIFT-4212 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1279.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 #1279 commit 94968a7db10d4b95bec62d33bed66868eebfbc38 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2017-05-23T15:22:44Z THRIFT-4205: Make sure gobject+glib are correctly linked commit fb518bebec7666953fc69a540db9a56973b9bab5 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2017-05-23T15:24:54Z Fix lib for glib commit a813c237fad8997ef96806259b999c5431759caa Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2017-05-25T15:11:38Z Fix logging in thrift library in #THRIFT-4211 commit 9250da1fe208927a09539d335ecf01d07e63d082 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2017-05-25T15:17:37Z Merge branch 'THRIFT-4211' into RELEASE-1.0.0 commit 84d328d93cab5e51587d892c3ecf641130531c3b Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2017-05-25T16:15:30Z Fix flush on invalid socket --- 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 #1278: THRIFT-4211: Fix logging in thrift library
GitHub user gadLinux opened a pull request: https://github.com/apache/thrift/pull/1278 THRIFT-4211: Fix logging in thrift library It seems we were aborting on error. We are now doing properly management. Error is ok and working perfectly. You can merge this pull request into a Git repository by running: $ git pull https://github.com/gadLinux/thrift THRIFT-4211 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1278.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 #1278 commit a813c237fad8997ef96806259b999c5431759caa Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2017-05-25T15:11:38Z Fix logging in thrift library in #THRIFT-4211 --- 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 #1272: THRIFT-4205: Set flags for glib+gobject on compilation
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1272 Good. Wait for a fix. And I will open a new issue, since we detected that thrift crashes when certificate verification not ok. --- 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 #1272: THRIFT-4205: Set flags for glib+gobject on compilation
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1272 Can someone tell why travis fails? --- 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 #1272: THRIFT-4205: Set flags for glib+gobject on compil...
GitHub user gadLinux opened a pull request: https://github.com/apache/thrift/pull/1272 THRIFT-4205: Set flags for glib+gobject on compilation Set flags for glib+gobject on compilation so systems like Android doesn't blow up. You can merge this pull request into a Git repository by running: $ git pull https://github.com/gadLinux/thrift THRIFT-4205 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1272.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 #1272 commit 94968a7db10d4b95bec62d33bed66868eebfbc38 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2017-05-23T15:22:44Z THRIFT-4205: Make sure gobject+glib are correctly linked commit fb518bebec7666953fc69a540db9a56973b9bab5 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2017-05-23T15:24:54Z Fix lib for glib --- 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 #639: Implements ssl sockets on c_glib
GitHub user gadLinux reopened a pull request: https://github.com/apache/thrift/pull/639 Implements ssl sockets on c_glib Adds dependencies to gobject at compile time because it needs it. Server side is not complete so some tests are failing. Looking to fix them all. I moved openssl check outside of the cpp compilation because now it's needed for at least two subprojects at lib. -avoid-version has to be reviewed as this is only for android, and must be enabled only when compiling with android-arm or android-* You can merge this pull request into a Git repository by running: $ git pull https://github.com/gadLinux/thrift thrift-1016 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/639.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 #639 commit 3ad451863e67374829cc99e94fd7b5473effdc3b Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2015-10-09T02:24:47Z Implements ssl sockets on c_glib. Adds dependencies to gobject at compile time because it needs it. commit 782b3767b35d6e564b9a49076ae38b8abdb15bfc Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2015-10-09T02:37:39Z Add missing files. Fix GOBJECT missing in build. commit 3ce56a99313bab6418c9ac40cbbc380183d6a360 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2015-11-11T14:08:44Z Add partially supported certificate pining. It fails the verify result. commit 0e0f534c68e52464946eeab190db5522691ad36b Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2015-11-11T14:14:03Z Use functionality provided by library instead of hacking in --- 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 #639: Implements ssl sockets on c_glib
Github user gadLinux closed the pull request at: https://github.com/apache/thrift/pull/639 --- 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 #639: Implements ssl sockets on c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/639 Yes, quite old sofar. --- 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 #1230: Make a patch to fix #4152 at least for structs
GitHub user gadLinux opened a pull request: https://github.com/apache/thrift/pull/1230 Make a patch to fix #4152 at least for structs Change type Type_ for Struct_ so structs are correctly linked. You can merge this pull request into a Git repository by running: $ git pull https://github.com/gadLinux/thrift THRIFT-4152 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1230.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 #1230 commit 83ea476613868e78da6196e456583128b9454d00 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2017-03-31T09:13:04Z Make a patch to fix #4152 at least for structs --- 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 #1208: THRIFT-4108 : Fix several problems found on library.
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1208 @jeking3 This was closed but not merged? Why? --- 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 #1208: THRIFT-4108 : Fix several problems found on library.
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1208 I added also some documentation and missing include functions. --- 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 #1208: THRIFT-4108 : Fix several problems found on libra...
GitHub user gadLinux opened a pull request: https://github.com/apache/thrift/pull/1208 THRIFT-4108 : Fix several problems found on library. Adjust headers. Format code. Make sure finalization is correct. Fix convenience methods. Remove global ssl context. You can merge this pull request into a Git repository by running: $ git pull https://github.com/gadLinux/thrift THRIFT-4108 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1208.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 #1208 commit 2923765c56e95b1fe93339cc99d8bf7b357f5295 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2017-03-06T17:55:44Z Remove global ssl context. Adjust headers. Format code. Make sure finalization is correct. Fix convenience methods. --- 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 #1200: THRIFT-3706: glib client/java server/crosstest multiplex...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1200 Thank you for answers I learnt a lot --- 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 #1200: THRIFT-3706: glib client/java server/crosstest mu...
Github user gadLinux commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1200#discussion_r102412630
--- Diff: lib/java/test/org/apache/thrift/test/TestServer.java ---
@@ -190,7 +193,9 @@ public static void main(String [] args) {
if (protocol_type.equals("binary")) {
} else if (protocol_type.equals("compact")) {
} else if (protocol_type.equals("json")) {
-} else if (protocol_type.equals("multiplexed")) {
+} else if (protocol_type.equals("multi")) {
--- End diff --
ok
---
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 #1200: THRIFT-3706: glib client/java server/crosstest mu...
Github user gadLinux commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1200#discussion_r102412439
--- Diff: lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c ---
@@ -61,15 +61,15 @@ thrift_protocol_set_property (GObject *object, guint
property_id,
switch (property_id)
{
case PROP_THRIFT_PROTOCOL_TRANSPORT:
- protocol->transport = g_value_get_object (value);
--- End diff --
Ok. As long as the reference count is guaranteed to be well managed. And it
seems because you added the finalize method it should be okay. But I think this
breaks RAII. Since the application can get the resource and free it but the
resource will outlive the requester inside this class. But I come from C/C++
and maybe RAII doesn't apply quite well here because the reference counter. I
will study about it a little bit.
---
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 #1200: THRIFT-3706: glib client/java server/crosstest mu...
Github user gadLinux commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1200#discussion_r102411428
--- Diff:
lib/c_glib/src/thrift/c_glib/protocol/thrift_multiplexed_protocol.c ---
@@ -42,146 +41,119 @@ static GParamSpec
*thrift_multiplexed_protocol_obj_properties[PROP_THRIFT_MULTIP
gint32
thrift_multiplexed_protocol_write_message_begin (ThriftMultiplexedProtocol
*protocol,
- const gchar *name, const ThriftMessageType message_type,
- const gint32 seqid, GError **error)
+const gchar *name, const ThriftMessageType message_type,
+const gint32 seqid, GError **error)
{
- gint32 ret;
- gchar *service_name = NULL;
- g_return_val_if_fail (THRIFT_IS_MULTIPLEXED_PROTOCOL (protocol), -1);
+ gint32 ret;
+ gchar *service_name = NULL;
+ g_return_val_if_fail (THRIFT_IS_MULTIPLEXED_PROTOCOL (protocol), -1);
- ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL
(protocol);
- ThriftMultiplexedProtocolClass *multiplexClass =
THRIFT_MULTIPLEXED_PROTOCOL_GET_CLASS(self);
- ThriftProtocolClass *cls = THRIFT_PROTOCOL_CLASS (multiplexClass);
+ ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL (protocol);
+ ThriftMultiplexedProtocolClass *multiplexClass =
THRIFT_MULTIPLEXED_PROTOCOL_GET_CLASS(self);
+ ThriftProtocolClass *cls = THRIFT_PROTOCOL_CLASS (multiplexClass);
- if( (message_type == T_CALL || message_type == T_ONEWAY) &&
self->service_name != NULL) {
- service_name = g_strdup_printf("%s%s%s", self->service_name,
self->separator, name);
+ if( (message_type == T_CALL || message_type == T_ONEWAY) &&
self->service_name != NULL) {
+service_name = g_strdup_printf("%s%s%s", self->service_name,
THRIFT_MULTIPLEXED_PROTOCOL_DEFAULT_SEPARATOR, name);
+ }else{
+service_name = g_strdup(name);
+ }
- }else{
- service_name = g_strdup(name);
- }
+ // relay to the protocol_decorator
+ ret = thrift_protocol_decorator_write_message_begin(protocol,
service_name, message_type, seqid, error);
- // relay to the protocol_decorator
- ret = thrift_protocol_decorator_write_message_begin(protocol,
service_name, message_type, seqid, error);
+ g_free(service_name);
- g_free(service_name);
-
- return ret;
+ return ret;
}
-
-
static void
thrift_multiplexed_protocol_set_property (GObject *object,
- guint property_id,
- const GValue *value,
- GParamSpec *pspec)
+guint property_id,
+const GValue *value,
+GParamSpec *pspec)
{
- ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL (object);
-
- switch (property_id)
- {
- case PROP_THRIFT_MULTIPLEXED_PROTOCOL_SERVICE_NAME:
- if(self->service_name!=NULL)
- g_free (self->service_name);
- self->service_name= g_value_dup_string (value);
- break;
-
- case PROP_THRIFT_MULTIPLEXED_PROTOCOL_SEPARATOR:
--- End diff --
Ok. Then I agree.
---
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 #1200: THRIFT-3706: glib client/java server multiplexed ...
Github user gadLinux commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1200#discussion_r102329070
--- Diff:
lib/c_glib/src/thrift/c_glib/protocol/thrift_multiplexed_protocol.c ---
@@ -42,146 +41,119 @@ static GParamSpec
*thrift_multiplexed_protocol_obj_properties[PROP_THRIFT_MULTIP
gint32
thrift_multiplexed_protocol_write_message_begin (ThriftMultiplexedProtocol
*protocol,
- const gchar *name, const ThriftMessageType message_type,
- const gint32 seqid, GError **error)
+const gchar *name, const ThriftMessageType message_type,
+const gint32 seqid, GError **error)
{
- gint32 ret;
- gchar *service_name = NULL;
- g_return_val_if_fail (THRIFT_IS_MULTIPLEXED_PROTOCOL (protocol), -1);
+ gint32 ret;
+ gchar *service_name = NULL;
+ g_return_val_if_fail (THRIFT_IS_MULTIPLEXED_PROTOCOL (protocol), -1);
- ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL
(protocol);
- ThriftMultiplexedProtocolClass *multiplexClass =
THRIFT_MULTIPLEXED_PROTOCOL_GET_CLASS(self);
- ThriftProtocolClass *cls = THRIFT_PROTOCOL_CLASS (multiplexClass);
+ ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL (protocol);
+ ThriftMultiplexedProtocolClass *multiplexClass =
THRIFT_MULTIPLEXED_PROTOCOL_GET_CLASS(self);
+ ThriftProtocolClass *cls = THRIFT_PROTOCOL_CLASS (multiplexClass);
- if( (message_type == T_CALL || message_type == T_ONEWAY) &&
self->service_name != NULL) {
- service_name = g_strdup_printf("%s%s%s", self->service_name,
self->separator, name);
+ if( (message_type == T_CALL || message_type == T_ONEWAY) &&
self->service_name != NULL) {
+service_name = g_strdup_printf("%s%s%s", self->service_name,
THRIFT_MULTIPLEXED_PROTOCOL_DEFAULT_SEPARATOR, name);
+ }else{
+service_name = g_strdup(name);
+ }
- }else{
- service_name = g_strdup(name);
- }
+ // relay to the protocol_decorator
+ ret = thrift_protocol_decorator_write_message_begin(protocol,
service_name, message_type, seqid, error);
- // relay to the protocol_decorator
- ret = thrift_protocol_decorator_write_message_begin(protocol,
service_name, message_type, seqid, error);
+ g_free(service_name);
- g_free(service_name);
-
- return ret;
+ return ret;
}
-
-
static void
thrift_multiplexed_protocol_set_property (GObject *object,
- guint property_id,
- const GValue *value,
- GParamSpec *pspec)
+guint property_id,
+const GValue *value,
+GParamSpec *pspec)
{
- ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL (object);
-
- switch (property_id)
- {
- case PROP_THRIFT_MULTIPLEXED_PROTOCOL_SERVICE_NAME:
- if(self->service_name!=NULL)
- g_free (self->service_name);
- self->service_name= g_value_dup_string (value);
- break;
-
- case PROP_THRIFT_MULTIPLEXED_PROTOCOL_SEPARATOR:
--- End diff --
Suppose a protocol or transport that add a separator to the packet to, for
example, add a header of encryption. And the separator is the same. It may be
confused because found twice, once for the protocol and another for the
multiplexed. Changing the multiplexed separator will fix the possible issue.
---
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 #1200: THRIFT-3706: glib client/java server multiplexed ...
Github user gadLinux commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1200#discussion_r102326459
--- Diff: lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c ---
@@ -549,12 +549,27 @@ thrift_protocol_init (ThriftProtocol *protocol)
}
static void
+thrift_protocol_dispose (GObject *gobject)
+{
+ ThriftProtocol *self = THRIFT_PROTOCOL (gobject);
+
+ g_clear_object(>transport);
+
+ /* Always chain up to the parent class; there is no need to check if
+ * the parent class implements the dispose() virtual function: it is
+ * always guaranteed to do so
+ */
+ G_OBJECT_CLASS (thrift_protocol_parent_class)->dispose(gobject);
--- End diff --
Yes I forgot that! Sorry.
---
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 #1200: THRIFT-3706: glib client/java server multiplexed ...
Github user gadLinux commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1200#discussion_r102328202
--- Diff: lib/java/test/org/apache/thrift/test/TestServer.java ---
@@ -190,7 +193,9 @@ public static void main(String [] args) {
if (protocol_type.equals("binary")) {
} else if (protocol_type.equals("compact")) {
} else if (protocol_type.equals("json")) {
-} else if (protocol_type.equals("multiplexed")) {
+} else if (protocol_type.equals("multi")) {
--- End diff --
I don't know much about tests. But you can get crazy here if there's a lot
of procotols.
I suggest what I did. Adding a prefix and sufix. The prefix can be matched
against multiplexed (multi) the suffix is the name of the protocol of the
underlaying implementation.
This convention doesn't add anything to the code. And makes you be ready
for the multiplexed whether a new protocol is added with no effort.
Convention over configuration.
---
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 #1200: THRIFT-3706: glib client/java server multiplexed ...
Github user gadLinux commented on a diff in the pull request: https://github.com/apache/thrift/pull/1200#discussion_r102327046 --- Diff: lib/c_glib/src/thrift/c_glib/transport/thrift_socket.h --- @@ -50,11 +50,8 @@ struct _ThriftSocket /* private */ gchar *hostname; - gshort port; + guint port; int sd; - guint8 *buf; --- End diff -- This is the default implementation of the socket. Are you sure about it? If this is changed I suppose this should be done in it's own ticket. --- 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 #1200: THRIFT-3706: glib client/java server multiplexed ...
Github user gadLinux commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1200#discussion_r102326250
--- Diff: lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c ---
@@ -61,15 +61,15 @@ thrift_protocol_set_property (GObject *object, guint
property_id,
switch (property_id)
{
case PROP_THRIFT_PROTOCOL_TRANSPORT:
- protocol->transport = g_value_get_object (value);
--- End diff --
Why you duplicate it? The underlaying transport should live as long as the
multiplexed one. And must be destroyed after protocol is destroyed. Duplicating
the transport may lead to object references hold and maybe memory freeing
problems.
I think this property must hold a reference to it and not a copy.
The copy can lead to memory freeing problems.
---
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 #1200: THRIFT-3706: glib client/java server multiplexed ...
Github user gadLinux commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1200#discussion_r102326623
--- Diff: lib/c_glib/src/thrift/c_glib/server/thrift_server.c ---
@@ -76,22 +76,22 @@ thrift_server_set_property (GObject *object, guint
property_id,
switch (property_id)
{
case PROP_THRIFT_SERVER_PROCESSOR:
- server->processor = g_value_get_object (value);
--- End diff --
Same here. Are you sure you must duplicate them?
---
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 #1200: THRIFT-3706: glib client/java server multiplexed ...
Github user gadLinux commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1200#discussion_r102325143
--- Diff:
lib/c_glib/src/thrift/c_glib/protocol/thrift_multiplexed_protocol.c ---
@@ -42,146 +41,119 @@ static GParamSpec
*thrift_multiplexed_protocol_obj_properties[PROP_THRIFT_MULTIP
gint32
thrift_multiplexed_protocol_write_message_begin (ThriftMultiplexedProtocol
*protocol,
- const gchar *name, const ThriftMessageType message_type,
- const gint32 seqid, GError **error)
+const gchar *name, const ThriftMessageType message_type,
+const gint32 seqid, GError **error)
{
- gint32 ret;
- gchar *service_name = NULL;
- g_return_val_if_fail (THRIFT_IS_MULTIPLEXED_PROTOCOL (protocol), -1);
+ gint32 ret;
+ gchar *service_name = NULL;
+ g_return_val_if_fail (THRIFT_IS_MULTIPLEXED_PROTOCOL (protocol), -1);
- ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL
(protocol);
- ThriftMultiplexedProtocolClass *multiplexClass =
THRIFT_MULTIPLEXED_PROTOCOL_GET_CLASS(self);
- ThriftProtocolClass *cls = THRIFT_PROTOCOL_CLASS (multiplexClass);
+ ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL (protocol);
+ ThriftMultiplexedProtocolClass *multiplexClass =
THRIFT_MULTIPLEXED_PROTOCOL_GET_CLASS(self);
+ ThriftProtocolClass *cls = THRIFT_PROTOCOL_CLASS (multiplexClass);
- if( (message_type == T_CALL || message_type == T_ONEWAY) &&
self->service_name != NULL) {
- service_name = g_strdup_printf("%s%s%s", self->service_name,
self->separator, name);
+ if( (message_type == T_CALL || message_type == T_ONEWAY) &&
self->service_name != NULL) {
+service_name = g_strdup_printf("%s%s%s", self->service_name,
THRIFT_MULTIPLEXED_PROTOCOL_DEFAULT_SEPARATOR, name);
+ }else{
+service_name = g_strdup(name);
+ }
- }else{
- service_name = g_strdup(name);
- }
+ // relay to the protocol_decorator
+ ret = thrift_protocol_decorator_write_message_begin(protocol,
service_name, message_type, seqid, error);
- // relay to the protocol_decorator
- ret = thrift_protocol_decorator_write_message_begin(protocol,
service_name, message_type, seqid, error);
+ g_free(service_name);
- g_free(service_name);
-
- return ret;
+ return ret;
}
-
-
static void
thrift_multiplexed_protocol_set_property (GObject *object,
- guint property_id,
- const GValue *value,
- GParamSpec *pspec)
+guint property_id,
+const GValue *value,
+GParamSpec *pspec)
{
- ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL (object);
-
- switch (property_id)
- {
- case PROP_THRIFT_MULTIPLEXED_PROTOCOL_SERVICE_NAME:
- if(self->service_name!=NULL)
- g_free (self->service_name);
- self->service_name= g_value_dup_string (value);
- break;
-
- case PROP_THRIFT_MULTIPLEXED_PROTOCOL_SEPARATOR:
--- End diff --
Why you did remove the ability to select the separator. It seems that Java
and maybe others support it. Think that some protocols may required to change
it to something special to make it work. I would leave it.
---
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 #1199: THRIFT-3706 - Implement multiplex protocol and crosstest...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1199 Greta, tell me if you need something from 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 issue #1191: THRIFT-3706 - Implement multiplexor.
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 I cannot do it since this commit is in thrift main outside master. So I cannot see it. You cannot merge because this branch is still not merged so it's still in my fork. You can merge and then apply the patch you want. Otherwise the only way I think is to make a patch. --- 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.
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. ---
[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 I must say that like the SSL patch this is already tested in production. Java vs c_glib. We have to add support for the Server c_glib and Client Java. But I suppose this time it will work without problems. About the multiplex fact, you cannot change the multplexor in runtime while it's already running. You must do it at initialization time. I want to add in the future the ability to add new multiplexed services "on the fly". But I should be add the properties in gobject and make sure I don't break multithreading. So this has to be added at latter step, like always. --- 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.
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 What do I have to do to be a contributor? --- 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.
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 I added some cross tests against Java. Maybe others can add the rest when the protocol is ready for the other languages. --- 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.
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 @jeking3 Please explain how this can happen. Just did a rebase and merged my changes that only adds few files (I think two or three). If master is so controlled that no build failures can go in (you where very strict with my tests)... How can it give a cross test failure? This is not my fault. How others can be merging changes that break the build? --- 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.
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 Hi Jeking3, You are right! I forgot this time to do the proper naming of the merge request. Yesterday I was thinking about the cross test. I will try to add a cross test for Java vs c_glib since it's the one actually tested in our own internal test. Do you have a guideline for the cross test? As you know I'm little lost there. Thank you. --- 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 #1191: Thrift 3706 - Implement multiplexor.
GitHub user gadLinux opened a pull request: https://github.com/apache/thrift/pull/1191 Thrift 3706 - Implement multiplexor. Implement protocol multiplexor. Implement protocol decorator. You can merge this pull request into a Git repository by running: $ git pull https://github.com/gadLinux/thrift thrift-3706 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1191.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 #1191 commit 226710b0a62b9f8a7f32c18ae77132e405996a9a Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2016-03-04T12:16:22Z Implement multiplexed protocol commit 619f54b90bc6a5724400fdb9e71b3d50b3e26e17 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2016-03-05T00:21:27Z Implement protocol multiplexor. For me there are a pair of things missing. Implement protocol decorator. commit c734748f712fb4133e6a031640a9a4be3f361ae2 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2016-03-28T23:08:52Z Fix protocol decorator cause it failed to register list cls->read_list_end on thrift_protocol_decorator_class_init Change log to debug --- 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 #927: THRIFT-3706 : Implement protocol multiplexor in c_...
Github user gadLinux closed the pull request at: https://github.com/apache/thrift/pull/927 --- 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 #927: THRIFT-3706 : Implement protocol multiplexor in c_glib [R...
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/927 I will redo it --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 I cross my fingers so it passes the tests. I want to continue coding but I don't want to do it until we have this issue merged. --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 It seems it still not fine... Do I have to see something? --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 Hi again, As the code is pure c we should be able to support all configurations. It makes me a little surprised to mix mechanism in handshake and negotiation but as far as you know what you are requesting is ok. I'm working passing some certifications PCI, VISA and Master and none of them allows SSL3 to run. Even for handshake. This is the reason why we stick to TLS_1.2. I will check the code a little bit for this way of doing when I have a time slot today. Is there a predefined server that does this way. I suppose that Java is already doing and suppose it's already working since some tests are passing. Can you confirm, please? Thank you a lot for your effort again. Best regards, --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 Seems it's still failing. Do you think I can modify the back the protocol and use SSL2-3 for the tests? --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 Hi, I must say that TLS1.2 is the recommended version for SSL because SSL3 is so buggy and insecure that everyone is filtering it. For testing can be ok. I would not change the default one. I would change just for the test. I suppose I have to write a brief documentation to let people know how to do it. i will do in my blog but I can send the patches for the apache web. One question? Do you sleep? Cause I have responses at all hours! You are so great. Thank you so much. --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 Oh, it failed again. How can this be? Should I rebase again? --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1185#discussion_r100681388
--- Diff: lib/c_glib/src/thrift/c_glib/transport/thrift_ssl_socket.c ---
@@ -0,0 +1,772 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#if defined(WIN32)
+#define MUTEX_TYPEHANDLE
+#define MUTEX_SETUP(x)(x) = CreateMutex(NULL, FALSE, NULL)
+#define MUTEX_CLEANUP(x) CloseHandle(x)
+#define MUTEX_LOCK(x) WaitForSingleObject((x), INFINITE)
+#define MUTEX_UNLOCK(x) ReleaseMutex(x)
+#else
+#define MUTEX_TYPEpthread_mutex_t
+#define MUTEX_SETUP(x)pthread_mutex_init(&(x), NULL)
+#define MUTEX_CLEANUP(x) pthread_mutex_destroy(&(x))
+#define MUTEX_LOCK(x) pthread_mutex_lock(&(x))
+#define MUTEX_UNLOCK(x) pthread_mutex_unlock(&(x))
+#endif
+
+#define OPENSSL_VERSION_NO_THREAD_ID 0x1000L
+
+
+/* object properties */
+enum _ThriftSSLSocketProperties
+{
+ PROP_THRIFT_SSL_SOCKET_CONTEXT = 3,
+ PROP_THRIFT_SSL_SELF_SIGNED
+};
+
+/* To hold a global state management of openssl for all instances */
+static gboolean thrift_ssl_socket_openssl_initialized=FALSE;
+/* Should this be keept at class level? */
+static SSL_CTX* thrift_ssl_socket_global_context=NULL;
+/* This array will store all of the mutexes available to OpenSSL. */
+static MUTEX_TYPE *thrift_ssl_socket_global_mutex_buf=NULL;
+
+
+/**
+ * OpenSSL uniq id function.
+ *
+ * @returnthread id
+ */
+static unsigned long thrift_ssl_socket_static_id_function(void)
+{
+#if defined(WIN32)
+ return GetCurrentThreadId();
+#else
+ return ((unsigned long) pthread_self());
+#endif
+}
+
+static void thrift_ssl_socket_static_locking_callback(int mode, int n,
const char* unk, int id) {
+ if (mode & CRYPTO_LOCK)
+MUTEX_LOCK(thrift_ssl_socket_global_mutex_buf[n]);
+ else
+MUTEX_UNLOCK(thrift_ssl_socket_global_mutex_buf[n]);
+}
+
+static int thrift_ssl_socket_static_thread_setup(void)
+{
+ int i;
+
+ thrift_ssl_socket_global_mutex_buf = malloc(CRYPTO_num_locks() *
sizeof(MUTEX_TYPE));
+ if (!thrift_ssl_socket_global_mutex_buf)
+return 0;
+ for (i = 0; i < CRYPTO_num_locks( ); i++)
+MUTEX_SETUP(thrift_ssl_socket_global_mutex_buf[i]);
+ CRYPTO_set_id_callback(thrift_ssl_socket_static_id_function);
+ CRYPTO_set_locking_callback(thrift_ssl_socket_static_locking_callback);
+ return 1;
+}
+
+static int thrift_ssl_socket_static_thread_cleanup(void)
+{
+ int i;
+ if (!thrift_ssl_socket_global_mutex_buf)
+return 0;
+ CRYPTO_set_id_callback(NULL);
+ CRYPTO_set_locking_callback(NULL);
+ for (i = 0; i < CRYPTO_num_locks( ); i++)
+MUTEX_CLEANUP(thrift_ssl_socket_global_mutex_buf[i]);
+ free(thrift_ssl_socket_global_mutex_buf);
+ thrift_ssl_socket_global_mutex_buf = NULL;
+ return 1;
+}
+
+/*
+static void* thrift_ssl_socket_dyn_lock_create_callback(const char* unk,
int id) {
+ g_print("We should create a lock\n");
+ return NULL;
+}
+
+static void thrift_ssl_socket_dyn_lock_callback(int mode, void* lock,
const char* unk, int id) {
+ if (lock != NULL) {
+if (mode & CRYPTO_LOCK) {
+ g_printf("We should lock thread %d\n");
+} else {
+ g_printf("We should unlock thread %d\n");
+}
+ }
+}
+
+static void thrift_ssl_socket_dyn_lock_destroy_callback(void* lock, const
char* unk, int id) {
+ g_printf("We must destroy the lock\n");
+}
+ */
+
+
+G_DEFINE_T
[GitHub] thrift issue #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 jeking3 Thank you a lot for your patches. They seem to fix the problem you found. But I don't really know why so I will investigate it a little bit after. I want to run my own tests anyway because I'm not sure if pinning will work well like this. About nsuke concerns, the code can load the CA certificate as well. I will try later to see if it helps. For now it seems lot better. --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 Let's fix it latter, please. I'm tired of getting this issue around. I will open a bug report if you want to track it. --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 I forgot to say that also the code was rebased. A curious thing is that everytime I do a rebase the system says that my code diverged... I don't know why since always rebasing the upstream master branch. --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 Okay. I revamped a little bit the system to allow it to run the cross tests. Since you are not using specific functions provided by the API it was causing to not initialize the instance properly. I added sane default values for SSL. It will use TLSv12 by default with dissallowed selfsigned certificates and basic pining function. So I also changed the test unit to allow selfsigned certificates when the client is created. Now the tests run but fails. But there I don't what's the reason. It's because Java, because the test is wrong, or the code in SSL branch is wrong. The latter seems difficult since it works in our environment. So maybe you can help to fix the tests. The important thing is that now it seems to be properly configured even if no API provided is used. Also a couple of straightforward errors on test were corrected. --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 Just a question. Why all CMake stuff. Having two compilation environments seems to me somewhat overloaded... Does it worth it? --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 Hi Jeking3, Yes there's only the client at this time. Cause I had no time to implement server. The tests can be run against Java or any other server. I will add after this commit is approved. The tests I added are rather limited because the lack of c_glib server. So yes I will add them at latter step. About cross checks. It's the first time I see about it. In fact all tests I did where against Java so they are naturally cross language. Again this should be added at a latter step. I need the client to be "in" the next release cause lot of effort has been set securing it. So the patch is big enough to split it in different issues. We can discuss later about a common library between C++ and c_glib, more testing, server and many other improvements. But I think it's a good idea to have this in now that's running. In our case is in production. Tested daily against hundreds of clients. And it's working fine. Can you merge it as is, please? And prepare after in the maillist the next features. --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1185 Thank you for the effort --- 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 #1185: Thrift 3369 - Third try to merge SSL c_glib
GitHub user gadLinux opened a pull request: https://github.com/apache/thrift/pull/1185 Thrift 3369 - Third try to merge SSL c_glib This pull request is the third try to merge against master. It implements SSL on c_glib with certificate pining included. The old pull request is 930. https://github.com/apache/thrift/pull/930 You can merge this pull request into a Git repository by running: $ git pull https://github.com/gadLinux/thrift thrift-3369 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1185.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 #1185 commit b1815ec42f848798422765d955cea0a815fcb525 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2016-03-07T12:42:44Z Implement basic version of the TLS transport commit d4c6207d9dabc138ff06a2b21a4477a5c440187c Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2016-03-07T14:27:22Z Certificate pinning features incorporated commit b691372edfadfc124152645ec527686a960cb84b Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2016-03-07T14:29:10Z Add test for certificate pinning commit f09d8eb55246638d8078857d7ef84459caf84c5d Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2016-03-07T14:30:10Z Add level2 test public key chain commit 2003ea188de1be4a13e8b2e859cc3b3d1a39e541 Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2016-03-07T20:49:42Z Fix dependency of ssl for tests commit 0f8699ddc3e7f5d2b8c8c36283b0c520d30f7ddf Author: Gonzalo Aguilar Delgado <gagui...@seglan.com> Date: 2016-08-04T23:33:29Z Enhace code, fix some issues and plug some bugs. Fix pull request #930 commit d0f2ac1a957912a59131b2e93028e3022d2a9f82 Author: Gonzalo Aguilar Delgado <gagui...@seglan.com> Date: 2016-08-04T23:40:57Z Fix style commit 01dcd95dda883f65ce6baa296815f964ce6f3904 Author: Gonzalo Aguilar Delgado <gagui...@seglan.com> Date: 2016-08-04T23:58:10Z Add file to cmake commit 5d9cb52119872f431f187f7166b374e6488b6229 Author: Gonzalo Aguilar Delgado <gagui...@seglan.com> Date: 2016-08-05T09:09:18Z Fix tests and the bug that produced it. commit d070e6dc6502e4e9cb8ba5ef3a68c0c00a09800f Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2016-12-29T11:42:04Z Fix tests so we don't include tests that require a server until server is ready. Fix compilation with cmake commit 40ec9d1bb9469a59ebfe9fea308cb9cebca8157d Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2016-12-29T11:46:30Z Fix tests in cmake commit cada6b8a8907ac00ed9c6995b1d88efd9f0471cc Author: Gonzalo Aguilar Delgado <gagui...@level2crm.com> Date: 2017-01-26T16:18:16Z Remove the public key in favor of the existing one. Re-add the test that was unintentionally removed. --- 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 #930: THRIFT-3369 : Implement SSL/TLS support on C with ...
Github user gadLinux closed the pull request at: https://github.com/apache/thrift/pull/930 --- 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 #930: THRIFT-3369 : Implement SSL/TLS support on C with c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/930 Re-rebased --- 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 #930: THRIFT-3369 : Implement SSL/TLS support on C with c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/930 I will reopen the merge --- 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 #930: THRIFT-3369 : Implement SSL/TLS support on C with ...
Github user gadLinux commented on a diff in the pull request: https://github.com/apache/thrift/pull/930#discussion_r98028242 --- Diff: lib/c_glib/test/Makefile.am --- @@ -37,16 +37,17 @@ BUILT_SOURCES = \ gen-c_glib/t_test_thrift_test_types.h AM_CPPFLAGS = -I../src -I./gen-c_glib -AM_CFLAGS = -g -Wall -Wextra -pedantic $(GLIB_CFLAGS) $(GOBJECT_CFLAGS) \ +AM_CFLAGS = -g -Wall -Wextra -pedantic $(GLIB_CFLAGS) $(GOBJECT_CFLAGS) $(OPENSSL_INCLUDES) \ @GCOV_CFLAGS@ AM_CXXFLAGS = $(AM_CFLAGS) -AM_LDFLAGS = $(GLIB_LIBS) $(GOBJECT_LIBS) @GCOV_LDFLAGS@ +AM_LDFLAGS = $(GLIB_LIBS) $(GOBJECT_LIBS) $(OPENSSL_LIBS) @GCOV_LDFLAGS@ check_PROGRAMS = \ testserialization \ testapplicationexception \ testcontainertest \ testtransportsocket \ + testtransportsslsocket \ --- End diff -- I think so. But since I don't have yet detection code for ssl there's no opt-out code. I think it will not work without the openssl library. So we have to choices. Require OpenSSL as you recommended above. Or make the code conditional, something is not ready yet. What do you think? --- 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 #930: THRIFT-3369 : Implement SSL/TLS support on C with ...
Github user gadLinux commented on a diff in the pull request:
https://github.com/apache/thrift/pull/930#discussion_r98027875
--- Diff: lib/c_glib/test/CMakeLists.txt ---
@@ -108,13 +108,6 @@ add_test(NAME testoptionalrequired COMMAND
testoptionalrequired)
include_directories("${PROJECT_SOURCE_DIR}/test/c_glib/src"
"${CMAKE_CURRENT_BINARY_DIR}/gen-c_glib")
-add_executable(testthrifttest testthrifttest.c
--- End diff --
Thrift test should be there. Iif I removed it it was because error. I will
readd it.
---
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 #930: THRIFT-3369 : Implement SSL/TLS support on C with ...
Github user gadLinux commented on a diff in the pull request: https://github.com/apache/thrift/pull/930#discussion_r98027723 --- Diff: lib/c_glib/src/thrift/c_glib/transport/thrift_platform_socket.h --- @@ -0,0 +1,120 @@ +/* --- End diff -- I tottally agree. It's a reimplementation of the library for c_glib to not mix things. But this can be seen as an upgrade. And start making room for an common library after merge. Don't you think? --- 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 #930: THRIFT-3369 : Implement SSL/TLS support on C with ...
Github user gadLinux commented on a diff in the pull request: https://github.com/apache/thrift/pull/930#discussion_r98027454 --- Diff: configure.ac --- @@ -199,6 +196,12 @@ if test "$with_c_glib" = "yes"; then fi AM_CONDITIONAL(WITH_C_GLIB, [test "$have_glib2" = "yes" -a "$have_gobject2" = "yes"]) +echo "OpenSSL check" --- End diff -- But this is the latest I fixed... Maybe I lost the changes when doing the rebase. OpenSSL was not used everyplace when I did the patch. In fact it was not even included for c_glib. C++ had this optionally included. I followed the C++ directions. --- 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 #930: THRIFT-3369 : Implement SSL/TLS support on C with c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/930 I'm rebasing again... --- 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 #930: THRIFT-3369 : Implement SSL/TLS support on C with c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/930 Again a problem with code outside the domain area. Can someone help me to fix this? Because it's failing on some places where I never touched. Should I continue doing rebase until everyone fixes their problems? What should I do? --- 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 #930: THRIFT-3369 : Implement SSL/TLS support on C with c_glib
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/930 Hi, I know that it's not a good idea to rewrite history but since this is not merged anyplace I just overwrote the commit. This time I think it's correct. Le'ts see what the tests are saying... --- 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. ---
