[GitHub] qpid-jms pull request: QPIDJMS-31: trivial README change, testing ...
GitHub user gemmellr opened a pull request: https://github.com/apache/qpid-jms/pull/1 QPIDJMS-31: trivial README change, testing GitHub integration You can merge this pull request into a Git repository by running: $ git pull https://github.com/gemmellr/qpid-jms master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-jms/pull/1.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 #1 commit 4b70bd48f14e64120f759cb0d528169581acff2f Author: Robert Gemmell Date: 2015-02-27T17:21:09Z QPIDJMS-31: trivial README change, testing GitHub integration --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request: QPIDJMS-31: trivial README change, testing ...
Github user gemmellr commented on the pull request: https://github.com/apache/qpid-jms/pull/1#issuecomment-76433601 Testing comment via GitHub --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] dispatch pull request: DISPATCH-119: trivial TODO changes, testing...
GitHub user gemmellr opened a pull request: https://github.com/apache/dispatch/pull/5 DISPATCH-119: trivial TODO changes, testing GitHub integration As per subject You can merge this pull request into a Git repository by running: $ git pull https://github.com/gemmellr/dispatch trunk Alternatively you can review and apply these changes as the patch at: https://github.com/apache/dispatch/pull/5.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 #5 commit 1366e6c6ec8bc0e485bdc40d829c892b190ac899 Author: Robert Gemmell Date: 2015-02-27T17:46:00Z DISPATCH-119: trivial TODO changes, testing GitHub integration --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] dispatch pull request: DISPATCH-119: trivial TODO changes, testing...
Github user gemmellr commented on the pull request: https://github.com/apache/dispatch/pull/5#issuecomment-76439860 testing comment via GitHub --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid pull request: QPID-6421: fix typo in README, testing GitHub i...
GitHub user gemmellr opened a pull request: https://github.com/apache/qpid/pull/5 QPID-6421: fix typo in README, testing GitHub integration As per subject You can merge this pull request into a Git repository by running: $ git pull https://github.com/gemmellr/qpid trunk Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid/pull/5.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 #5 commit 22a29128dbe36a5f3877a54d457f00742306553a Author: Robert Gemmell Date: 2015-02-27T18:00:13Z QPID-6421: fix typo in README, testing GitHub integration --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid pull request: QPID-6421: fix typo in README, testing GitHub i...
Github user gemmellr commented on the pull request: https://github.com/apache/qpid/pull/5#issuecomment-76446872 Test comment via GitHub --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid pull request: QPID-6470: FieldValue::getFloatingPointValue() ...
Github user gemmellr commented on the pull request: https://github.com/apache/qpid/pull/6#issuecomment-99921669 The mentioned JIRA has been marked resolved, but the commits didnt include the needed message to automatically close this PR. Can you check if the changes resolve your issue and if so close this PR? (We cant close it directly, only via commit comments, since it is an infra-managed mirror of our upstream repo). --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #1: Memory leak with delayed settlement
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/1 I am in a F2F meeting (or travelling to/from it) all this week so I probably won't get a chance to look closely until next week, but I took a skim of this. I can see from the changes that the issue is more subtle than the JIRA description actually indicates, so it would be useful to update that to be a bit clearer on the actual issue. E.g, settlement is effectively an explicit 'I am forgetting this delivery' step, so it is expected to retain deliveries when as yet unsettled. Retaining deliveries in memory that have been settled (and that fact actually processed by the Transport) would not, and the changes suggest there is scope for that to occur. As an aside, we don't intend to do any 0.16.x releases, 0.17.0 will be the next release (fairly soon) for both proton[-c] and proton-j to cement their new independence, so changes for this will go on master. Robbie --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-java issue #1: upgrading logback to latest 1.1.7
Github user gemmellr commented on the issue: https://github.com/apache/qpid-java/pull/1 Aligns to https://issues.apache.org/jira/browse/QPID-7468 --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #2: Delivery link leak
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/2 I've had a closer look at this now, sorry it took so long. I'm not sure about some of the changes, I'm going to give it a furhter look tomorrow to either establish I'm wrong, or suggest some alternative changes. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j pull request #3: PROTON-1393: fully sever delivery refs during...
GitHub user gemmellr opened a pull request: https://github.com/apache/qpid-proton-j/pull/3 PROTON-1393: fully sever delivery refs during removal Fully sever delivery references for related 'lists' during removal to prevent unexpected retention of old deliveries that arent otherwise reachable. Updated version of #1 / #2. You can merge this pull request into a Git repository by running: $ git pull https://github.com/gemmellr/qpid-proton-j delivery-lists Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton-j/pull/3.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 #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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #2: Delivery link leak
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/2 I've raised #3 with some alternative changes, which are essentially just a subset of yours with a different test. One of the changes here would corrupt the linkPrev/next entries chain, which is essentially part of the problem the changes are really trying to adress, while some others would only make a difference if the entries were already corrupt. The main change required to address the retention issue is the nulling of old refs while removing the delivery from the chain, and the related change to ensure the link free'ing process then doesn't give up early when it iterates the remaining deliveries. If you can give #3 a try out that would be great. Some pointers for future PRs (which I shall add to the site/repo's at some point soon so folks can actually see it in advance, or we can reference it later): - If you raise the PR with the JIRA key in the title, a bot will raise a note against the JIRA for open/close and also relay future comments to the JIRA. - Similarly if you could include the key in the commit log message, that way a bot records it on the JIRA upon eventual commit. - Your PR had some cruft commits/changes from doing merges. In general PRs should be a single commit rebased to sit on the current master without any merge commits. Multiple commits can be useful if its a many-layered change and the steps help evaluate it however. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #4: SSLContext changes
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/4 I've had a look and have some comments: - The changes should inclulde some tests for the new feature. - The setter javadoc should make clear that this overrides any other configuration that might have otherwise been uesd to create a context, and the getter only returns anything set with the setter. - The changes introduce various lines with whitespace errors (lines with only spaces, mixtures of tabs and space indents). Please ensure you use spaces only, and empty lines are in fact empty. - The change in IoHandler to force EXTERNAL is unrelated and not required here, please remove it. Some pointers for any updates and future PRs (info which I'll add to the website/repo's at some point soon, so folks can see it in advance, or we can reference it later): - If you raise the PR with the JIRA key in the title (e.g. see #3), a bot will raise a note against the JIRA for open/close and also relay future comments to the JIRA. - Similarly if you could include the key in the commit log message, that way a bot records it on the JIRA upon eventual commit. - In general PRs should be a single commit rebased to sit at the HEAD of the current master, without any merge commits. Multiple commits can be useful if its a many-layered change and the steps help evaluate it however. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #4: SSLContext changes
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/4 I've pushed an updated set of changes, to address some of my earlier feedback and some issues with the subsequent commit. Give things a try out on master and comment here or on PROTON-1405 if you have any issues. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #4: SSLContext changes
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/4 Actually, if you could also confirm if its just working for you that would be good too, as I can then look to kick off the release process on 0.18.0 some time, perhaps next week. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #7: PROTON-1409 Exposing delivery length
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/7 The javadoc could do with making a little clearer that it returns the currently unread length of the existing payload for the delivery, and is not the length of the delivery (which might not be complete yet) or the buffer containing the contents, which both may differ at any given time. A test would be nice so the new public method is at used and folks can see how it is. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #6: PROTON-1410 Moving ThreadLocal logic for Decoders on...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/6 Can you elaborate a little on what you are doing to need this? Are you just subclassing the MessageImpl? If so would protected getter(s) work for you? Creating a separate new 'non impl' utility class purely to access a thread local of a specific implementation from within a third bit of code stikes me as ugly, especially if it is already a subclass of the starting 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #6: PROTON-1410 Moving ThreadLocal logic for Decoders on...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/6 I'm aware of what qpid-jms does, it does that while avoiding using the Message[Impl] from proton entirely, meaning it isn't using the thread local from there either, but just happens to have its own (though it could actually do the same without that thread local if it wanted, given how the rest of the client works.) I wouldnt be all that inclined to make qpid-jms use the proposed TLSEncoder class even if it already existed in proton, it seems nicer to keep that in the client. As an aside, TLS seems a weird acronym to use in the name, given its typical meaning/usage. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #7: PROTON-1409 Exposing delivery length
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/7 I've pushed a change to expose the currently available bytes length. I went with 'available()' since it felt natural for what its indicating, and applying to both send/recv cases. In hindsight, 'remaining' could also be good, as thats what proton uses for actual buffers, though I think I still prefer 'available' here. I've just left the existing method unchanged for now, keeping it aligned with the various other related impl methods and as a bonus delays screwing with anyone who had been reflecting access to it thus far, we can consolidate them as and when the impl changes otherwise. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #8: 0.16.x
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/8 Did you raise this in error? If so please close 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #102: NO-JIRA: revert temporary workaround for PROTON-1453
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/102 Looks like the change was applied, but was no longer the head commit at the time, and didn't have "This closes #102" in the message of the commit itself or a merge commit. Can you close 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms issue #6: Added two new example classes, Client and Server.
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/6 Hi Austin, I think this is a good start. There are some changes that I think would improve the new example classes, in general I think the classes would benefit from some simplification in terms of a making a clear messaging example. They effectively combine to give a request-response example, so for me the main things are to demonstrate the use of a [Temporary]Queue for responses and using JMSReplyTo to pass that destination information to the server and for it to use that in sending responses back. For example, consider the equivalent Client and Server examples for Proton's C++ and Python bindings: https://github.com/apache/qpid-proton/blob/master/examples/python/client.py https://github.com/apache/qpid-proton/blob/master/examples/python/server.py https://github.com/apache/qpid-proton/blob/master/examples/cpp/client.cpp https://github.com/apache/qpid-proton/blob/master/examples/cpp/server.cpp These look (I haven't actually run them) to just send some fixed messages with well-known request text, and capitalize them on the server before sending the responses. I think doing the same here too would make for a clearer example, showing just the bits people need to know about the client config etc to get started and the messaging pattern in general, without the added complexity of commands and reading input expanding the scope. If we strip the example back to just send/receive basic text messages like the Proton ones, it should even be possible to run the different examples in combination fairly easily. That might take a little more work, so I'm not suggeting you need to do that here, just commenting on it perhaps being a 'nice to have' some time, and aligning the behaviour more here would make sense as a starting point to the end. The main complication I can think of in actually doing it would be that right now the JMS client examples use a queue named "queue" by default whereas the Proton ones use "examples" by default. The Proton ones do let you specify the details on the command line, so that would be one way to align them (the other being updating the JMS clients jndi.properties file before compiling them). I don't see a need for adding that complexity here, but I would be happy to change the JMS examples default queue name config in jndi.properties to match. Again, I'm not suggesting you need to d o that here though, we can always do that separately later. Beyond the above, I noted some very small issues looking this over which would apply even with the simplifications, I'll go put some comments against the code now. Robbie --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/6#discussion_r115540862
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Server.java ---
@@ -0,0 +1,119 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import javax.jms.ObjectMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import java.util.ArrayList;
+import java.util.Collections;
+
+public class Server {
+
+public static void main(String[] args) throws Exception {
+ System.out.println("[SERVER] Awaiting Message...");
+
+ try {
+ // The configuration for the Qpid InitialContextFactory has
been supplied in
+ // a jndi.properties file in the classpath, which results
in it being picked
+ // up automatically by the InitialContext constructor.
+ Context context = new InitialContext();
+
+ ConnectionFactory factory = (ConnectionFactory)
context.lookup("myFactoryLookup");
+ Destination queue = (Destination)
context.lookup("myQueueLookup");
+
+ Connection connection =
factory.createConnection(System.getProperty("USER"),
System.getProperty("PASSWORD"));
+ connection.setExceptionListener(new MyExceptionListener());
+ connection.start();
+
+ Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
+
+
+ MessageConsumer messageConsumer =
session.createConsumer(queue);
+ while (true) {
+
+ long start = System.currentTimeMillis();
+
+ //Receive a message
+ Message receivedMessage =
messageConsumer.receive(0);
--- End diff --
Removing the 0 might be clearer, using the plain receive() call.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/6#discussion_r115537215 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java --- @@ -0,0 +1,204 @@ + --- End diff -- This new file needs a licence header added. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/6#discussion_r115539336
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java ---
@@ -0,0 +1,204 @@
+
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TemporaryQueue;
+import javax.jms.TextMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Scanner;
+
+public class Client {
+private static final int DELIVERY_MODE = DeliveryMode.NON_PERSISTENT;
+public static void main(String[] args) throws Exception {
+try {
+// The configuration for the Qpid InitialContextFactory has
been supplied in
+// a jndi.properties file in the classpath, which results in
it being picked
+// up automatically by the InitialContext constructor.
+Context context = new InitialContext();
+
+ConnectionFactory factory = (ConnectionFactory)
context.lookup("myFactoryLookup");
+Destination queue = (Destination)
context.lookup("myQueueLookup");
+
+Connection connection =
factory.createConnection(System.getProperty("USER"),
System.getProperty("PASSWORD"));
+connection.setExceptionListener(new MyExceptionListener());
+connection.start();
+
+Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
+
+//Create message and temporary queue to send to and from.
+Message messageToBeSent = userInterface(session);
+TemporaryQueue tempQ = session.createTemporaryQueue();
+messageToBeSent.setJMSReplyTo(tempQ);
+
+MessageProducer messageProducer =
session.createProducer(queue);
+
+//Send the message
+messageProducer.send(messageToBeSent, DELIVERY_MODE,
Message.DEFAULT_PRIORITY, Message.DEFAULT_TIME_TO_LIVE);
+System.out.println("[CLIENT] The message has been sent.");
+
+
+MessageConsumer messageConsumer =
session.createConsumer(tempQ);
+long start = System.currentTimeMillis();
+boolean deductTimeout = false;
+int timeout = 1000;
+
+//Receive the server response
+TextMessage newMessage = (TextMessage)
messageConsumer.receive(timeout);
+if (newMessage != null) {
+ System.out.print("[CLIENT] Response from server
received in ");
+} else {
+System.out.println("[CLIENT] Response not received within
timeout, stopping.");
+deductTimeout = true;
+}
+
+long finish = System.currentTimeMillis();
+long taken = finish - start;
+if (deductTimeout) {
+taken -= timeout;
+}
+if (newMessage != null) {
+ System.out.println(taken + "ms");
--- End diff --
I would probably just remove the timing stuff. The Sender+Reciever examples
only have that as they can serve as simple throughput checks, it isnt likely to
be that interesting in this case.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/6#discussion_r115537587
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java ---
@@ -0,0 +1,204 @@
+
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TemporaryQueue;
+import javax.jms.TextMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Scanner;
+
+public class Client {
+private static final int DELIVERY_MODE = DeliveryMode.NON_PERSISTENT;
+public static void main(String[] args) throws Exception {
--- End diff --
Nice to have a line between variables and 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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/6#discussion_r115538757
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java ---
@@ -0,0 +1,204 @@
+
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TemporaryQueue;
+import javax.jms.TextMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Scanner;
+
+public class Client {
+private static final int DELIVERY_MODE = DeliveryMode.NON_PERSISTENT;
+public static void main(String[] args) throws Exception {
+try {
+// The configuration for the Qpid InitialContextFactory has
been supplied in
+// a jndi.properties file in the classpath, which results in
it being picked
+// up automatically by the InitialContext constructor.
+Context context = new InitialContext();
+
+ConnectionFactory factory = (ConnectionFactory)
context.lookup("myFactoryLookup");
+Destination queue = (Destination)
context.lookup("myQueueLookup");
+
+Connection connection =
factory.createConnection(System.getProperty("USER"),
System.getProperty("PASSWORD"));
+connection.setExceptionListener(new MyExceptionListener());
+connection.start();
+
+Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
+
--- End diff --
This line and most of the other 'empty' lines below actually have leading
whitespace that should be removed. The 'git diff' command can be configured to
show this whitespace if it isnt already, as can most IDE's (which can also
typically format the code and remove them, either on request or automatically).
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/6#discussion_r115541734
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Server.java ---
@@ -0,0 +1,119 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import javax.jms.ObjectMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import java.util.ArrayList;
+import java.util.Collections;
+
+public class Server {
+
+public static void main(String[] args) throws Exception {
+ System.out.println("[SERVER] Awaiting Message...");
+
+ try {
+ // The configuration for the Qpid InitialContextFactory has
been supplied in
+ // a jndi.properties file in the classpath, which results
in it being picked
+ // up automatically by the InitialContext constructor.
+ Context context = new InitialContext();
+
+ ConnectionFactory factory = (ConnectionFactory)
context.lookup("myFactoryLookup");
+ Destination queue = (Destination)
context.lookup("myQueueLookup");
+
+ Connection connection =
factory.createConnection(System.getProperty("USER"),
System.getProperty("PASSWORD"));
+ connection.setExceptionListener(new MyExceptionListener());
+ connection.start();
+
+ Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
+
+
+ MessageConsumer messageConsumer =
session.createConsumer(queue);
+ while (true) {
+
+ long start = System.currentTimeMillis();
+
+ //Receive a message
+ Message receivedMessage =
messageConsumer.receive(0);
+
+ long finish = System.currentTimeMillis();
+ long taken = finish - start;
+
+ System.out.println("[SERVER] Received the message
in " + taken + "ms");
+
+ //Create new message to return to client
+ Destination replyDestination =
receivedMessage.getJMSReplyTo();
+ TextMessage newMessage =
session.createTextMessage(interpretMessage(receivedMessage));
+ MessageProducer messageProducer =
session.createProducer(null);
--- End diff --
MessageProducer instances need to be closed when you are finished using
them, this would currently be orphaning some resources each time. You also
typically create a long-lived anoynmous MessageProducers, so as to re-use them.
If you move the creation outside the while loop, you can then reuse a single
producer for sending all the subsequent responses.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/6#discussion_r115540310
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Server.java ---
@@ -0,0 +1,119 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import javax.jms.ObjectMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import java.util.ArrayList;
+import java.util.Collections;
+
+public class Server {
+
+public static void main(String[] args) throws Exception {
+ System.out.println("[SERVER] Awaiting Message...");
--- End diff --
If this line is left, it would probably be better in the while loop before
the receive call that might block indefinitely.
There are also some tabs here and in many of the other lines below, which
should be replaced with spaces. As before, git diff and IDEs can help 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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/6#discussion_r115542360
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Server.java ---
@@ -0,0 +1,119 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import javax.jms.ObjectMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import java.util.ArrayList;
+import java.util.Collections;
+
+public class Server {
+
+public static void main(String[] args) throws Exception {
+ System.out.println("[SERVER] Awaiting Message...");
+
+ try {
+ // The configuration for the Qpid InitialContextFactory has
been supplied in
+ // a jndi.properties file in the classpath, which results
in it being picked
+ // up automatically by the InitialContext constructor.
+ Context context = new InitialContext();
+
+ ConnectionFactory factory = (ConnectionFactory)
context.lookup("myFactoryLookup");
+ Destination queue = (Destination)
context.lookup("myQueueLookup");
+
+ Connection connection =
factory.createConnection(System.getProperty("USER"),
System.getProperty("PASSWORD"));
+ connection.setExceptionListener(new MyExceptionListener());
+ connection.start();
+
+ Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
+
+
+ MessageConsumer messageConsumer =
session.createConsumer(queue);
+ while (true) {
+
+ long start = System.currentTimeMillis();
+
+ //Receive a message
+ Message receivedMessage =
messageConsumer.receive(0);
+
+ long finish = System.currentTimeMillis();
+ long taken = finish - start;
+
+ System.out.println("[SERVER] Received the message
in " + taken + "ms");
+
+ //Create new message to return to client
+ Destination replyDestination =
receivedMessage.getJMSReplyTo();
+ TextMessage newMessage =
session.createTextMessage(interpretMessage(receivedMessage));
+ MessageProducer messageProducer =
session.createProducer(null);
+ newMessage.setJMSDestination(replyDestination);
--- End diff --
This line should be removed. It doesnt actually have any effect on where
the message goes, and is not intended for application usage, but is rather an
annoying and confusing element of JMS, where many setter methods on the Message
exist only for the client itself to use during the send call when supplied with
another JMS providers messages.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #7: Client+Server class additions - updated
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/7#discussion_r116053494
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java ---
@@ -0,0 +1,106 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TemporaryQueue;
+import javax.jms.TextMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+
+public class Client {
+private static final int DELIVERY_MODE = DeliveryMode.NON_PERSISTENT;
+
+public static void main(String[] args) throws Exception {
+try {
+// The configuration for the Qpid InitialContextFactory has
been supplied in
+// a jndi.properties file in the classpath, which results in
it being picked
+// up automatically by the InitialContext constructor.
+Context context = new InitialContext();
+
+ConnectionFactory factory = (ConnectionFactory)
context.lookup("myFactoryLookup");
+Destination queue = (Destination)
context.lookup("myQueueLookup");
+
+Connection connection =
factory.createConnection(System.getProperty("USER"),
System.getProperty("PASSWORD"));
+connection.setExceptionListener(new MyExceptionListener());
+connection.start();
+
+Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
+
+//Creates a message and temporary queue to send to and from.
+int random = (int) (Math.random()*3);
+TextMessage messageToBeSent;
+if (random == 0) {
+messageToBeSent = session.createTextMessage("first example
message");
+} else if (random == 1) {
+messageToBeSent = session.createTextMessage("second
example message");
+} else {
+messageToBeSent = session.createTextMessage("third example
message");
+}
+
+TemporaryQueue tempQueue = session.createTemporaryQueue();
+messageToBeSent.setJMSReplyTo(tempQueue);
+MessageProducer messageProducer =
session.createProducer(queue);
+
+//Send the message
+messageProducer.send(messageToBeSent, DELIVERY_MODE,
Message.DEFAULT_PRIORITY, Message.DEFAULT_TIME_TO_LIVE);
+System.out.println("[CLIENT] The message with text \"" +
messageToBeSent.getText() +"\" has been sent.");
+
+MessageConsumer messageConsumer =
session.createConsumer(tempQueue);
--- End diff --
It would be better to move creation of the consumer on the response queue
up to before sending any request, to ensure there is a listener ready for the
message. It often wont matter, but there are some select cases it does so its
best to have the examples show that.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #7: Client+Server class additions - updated
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/7#discussion_r116057472
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java ---
@@ -0,0 +1,106 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TemporaryQueue;
+import javax.jms.TextMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+
+public class Client {
+private static final int DELIVERY_MODE = DeliveryMode.NON_PERSISTENT;
+
+public static void main(String[] args) throws Exception {
+try {
+// The configuration for the Qpid InitialContextFactory has
been supplied in
+// a jndi.properties file in the classpath, which results in
it being picked
+// up automatically by the InitialContext constructor.
+Context context = new InitialContext();
+
+ConnectionFactory factory = (ConnectionFactory)
context.lookup("myFactoryLookup");
+Destination queue = (Destination)
context.lookup("myQueueLookup");
+
+Connection connection =
factory.createConnection(System.getProperty("USER"),
System.getProperty("PASSWORD"));
+connection.setExceptionListener(new MyExceptionListener());
+connection.start();
+
+Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
+
+//Creates a message and temporary queue to send to and from.
--- End diff --
Following the other examples would be better here, increasing consistency
between them and serving to demonstrate sending multiple requests with the same
producer as would typically be done. For example:
```
String[] requests = new String[] { "Twas brillig, and the slithy toves",
"Did gire and gymble in the wabe.",
"All mimsy were the borogroves,",
"And the mome raths outgrabe." };
for (String request : requests) {
TextMessage requestMessage = session.createTextMessage(request);
requestMessage.setJMSReplyTo(replyToQueue);
messageProducer.send(requestMessage, DeliveryMode.NON_PERSISTENT,
Message.DEFAULT_PRIORITY, Message.DEFAULT_TIME_TO_LIVE);
}
```
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #7: Client+Server class additions - updated
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/7#discussion_r116069936
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Server.java ---
@@ -0,0 +1,94 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import javax.jms.ObjectMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import java.util.ArrayList;
+import java.util.Collections;
+
+public class Server {
+public static void main(String[] args) throws Exception {
+ try {
+ // The configuration for the Qpid InitialContextFactory has
been supplied in
+ // a jndi.properties file in the classpath, which results
in it being picked
+ // up automatically by the InitialContext constructor.
+ Context context = new InitialContext();
+
+ ConnectionFactory factory = (ConnectionFactory)
context.lookup("myFactoryLookup");
+ Destination queue = (Destination)
context.lookup("myQueueLookup");
+
+ Connection connection =
factory.createConnection(System.getProperty("USER"),
System.getProperty("PASSWORD"));
+ connection.setExceptionListener(new MyExceptionListener());
+ connection.start();
+
+ Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
+
+ MessageConsumer messageConsumer =
session.createConsumer(queue);
+ MessageProducer messageProducer =
session.createProducer(null);
+
+ while (true) {
+ System.out.println("[SERVER] Awaiting Message...");
--- End diff --
The lines below this are indented an extra level, they should all be at
this level. I also think we can simplify this section and improve the output a
little by doing something like:
```
while (true) {
TextMessage receivedMessage = (TextMessage) messageConsumer.receive();
System.out.println("[SERVER] Received: " + receivedMessage.getText());
TextMessage responseMessage =
session.createTextMessage(receivedMessage.getText().toUpperCase());
messageProducer.send(receivedMessage.getJMSReplyTo(), responseMessage,
DeliveryMode.NON_PERSISTENT, Message.DEFAULT_PRIORITY,
Message.DEFAULT_TIME_TO_LIVE);
}
```
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #7: Client+Server class additions - updated
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/7#discussion_r116064745
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Server.java ---
@@ -0,0 +1,94 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import javax.jms.ObjectMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import java.util.ArrayList;
+import java.util.Collections;
+
+public class Server {
+public static void main(String[] args) throws Exception {
+ try {
--- End diff --
The try block and its contents are indented an extra level than it should
be. Your editor should be able to fix the indent. Feel free to grab me on IRC
for assistance if needed.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #7: Client+Server class additions - updated
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/7#discussion_r116048356
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java ---
@@ -0,0 +1,106 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TemporaryQueue;
+import javax.jms.TextMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+
+public class Client {
+private static final int DELIVERY_MODE = DeliveryMode.NON_PERSISTENT;
+
+public static void main(String[] args) throws Exception {
+try {
+// The configuration for the Qpid InitialContextFactory has
been supplied in
+// a jndi.properties file in the classpath, which results in
it being picked
+// up automatically by the InitialContext constructor.
+Context context = new InitialContext();
+
+ConnectionFactory factory = (ConnectionFactory)
context.lookup("myFactoryLookup");
+Destination queue = (Destination)
context.lookup("myQueueLookup");
+
+Connection connection =
factory.createConnection(System.getProperty("USER"),
System.getProperty("PASSWORD"));
+connection.setExceptionListener(new MyExceptionListener());
+connection.start();
+
+Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
+
+//Creates a message and temporary queue to send to and from.
+int random = (int) (Math.random()*3);
+TextMessage messageToBeSent;
+if (random == 0) {
+messageToBeSent = session.createTextMessage("first example
message");
+} else if (random == 1) {
+messageToBeSent = session.createTextMessage("second
example message");
+} else {
+messageToBeSent = session.createTextMessage("third example
message");
+}
+
+TemporaryQueue tempQueue = session.createTemporaryQueue();
+messageToBeSent.setJMSReplyTo(tempQueue);
+MessageProducer messageProducer =
session.createProducer(queue);
+
+//Send the message
+messageProducer.send(messageToBeSent, DELIVERY_MODE,
Message.DEFAULT_PRIORITY, Message.DEFAULT_TIME_TO_LIVE);
+System.out.println("[CLIENT] The message with text \"" +
messageToBeSent.getText() +"\" has been sent.");
+
+MessageConsumer messageConsumer =
session.createConsumer(tempQueue);
+
+//Receive the server response
+TextMessage receivedMessage = (TextMessage)
messageConsumer.receive(1000);
+if (receivedMessage != null) {
+System.out.println("[CLIENT] Response from server
received.");
+} else {
+System.out.println("[CLIENT] Response not received within
timeout, stopping.");
+}
+
+//Display response and close client.
+System.out.println("[CLIENT] Here is the interpreted
message:\n" + receivedMessage.getText() + "\n[CLIENT] Quitting Client.");
--- End diff --
I'd probably just print a single line showing the response text, and
optionally the request text. E.g. other exampels show a line with "\ --> \" style output.
I sugge
[GitHub] qpid-jms pull request #7: Client+Server class additions - updated
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/7#discussion_r116052679
--- Diff:
qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java ---
@@ -0,0 +1,106 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.example;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.Destination;
+import javax.jms.ExceptionListener;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TemporaryQueue;
+import javax.jms.TextMessage;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+
+public class Client {
+private static final int DELIVERY_MODE = DeliveryMode.NON_PERSISTENT;
+
+public static void main(String[] args) throws Exception {
+try {
+// The configuration for the Qpid InitialContextFactory has
been supplied in
+// a jndi.properties file in the classpath, which results in
it being picked
+// up automatically by the InitialContext constructor.
+Context context = new InitialContext();
+
+ConnectionFactory factory = (ConnectionFactory)
context.lookup("myFactoryLookup");
+Destination queue = (Destination)
context.lookup("myQueueLookup");
+
+Connection connection =
factory.createConnection(System.getProperty("USER"),
System.getProperty("PASSWORD"));
+connection.setExceptionListener(new MyExceptionListener());
+connection.start();
+
+Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
+
+//Creates a message and temporary queue to send to and from.
+int random = (int) (Math.random()*3);
+TextMessage messageToBeSent;
+if (random == 0) {
+messageToBeSent = session.createTextMessage("first example
message");
+} else if (random == 1) {
+messageToBeSent = session.createTextMessage("second
example message");
+} else {
+messageToBeSent = session.createTextMessage("third example
message");
+}
+
+TemporaryQueue tempQueue = session.createTemporaryQueue();
+messageToBeSent.setJMSReplyTo(tempQueue);
+MessageProducer messageProducer =
session.createProducer(queue);
+
+//Send the message
+messageProducer.send(messageToBeSent, DELIVERY_MODE,
Message.DEFAULT_PRIORITY, Message.DEFAULT_TIME_TO_LIVE);
+System.out.println("[CLIENT] The message with text \"" +
messageToBeSent.getText() +"\" has been sent.");
+
+MessageConsumer messageConsumer =
session.createConsumer(tempQueue);
+
+//Receive the server response
+TextMessage receivedMessage = (TextMessage)
messageConsumer.receive(1000);
+if (receivedMessage != null) {
+System.out.println("[CLIENT] Response from server
received.");
+} else {
+System.out.println("[CLIENT] Response not received within
timeout, stopping.");
+}
+
+//Display response and close client.
+System.out.println("[CLIENT] Here is the interpreted
message:\n" + receivedMessage.getText() + "\n[CLIENT] Quitting Client.");
+connection.close();
+System.exit(1);
--- End diff --
The exit call should be removed. It is signalling an error return code when
everything actually went ok. The example will e
[GitHub] qpid-jms issue #7: Client+Server class additions - updated 5/15/17
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/7 Hi Austin, I've squashed your chagnes into a single commit (https://github.com/apache/qpid-jms/commit/72c7bb0880b5cc7dd332c0220091f93d0de9a5a2) and put it in along with further commit (https://github.com/apache/qpid-jms/commit/8a1a498c6c8307da1e3079c09c3d4d2e0aff5776) to fix up some remaining indentation+tab issues and make some small simplification+clarification changes. Thanks for the contribution! --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms issue #9: QPIDJMS-294: Ensure that SASL mechanism has completed bef...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/9 Looks good. I gave things a try with the changes from PROTON-1486 (now pushed) against Dispatch and the C++ broker, which continue to send the explicit challange before the outcome rather than use the additional-data field, all seemed well. I assumed you used the java broker and its related changes on QPID-7787 so I skipped that. On the unit testing, extracting a mechanism finder seems reasonable, can probably do something simple with lambdas and/or method references to essentially pass what its doing now. I'll wait for proton-j 0.20.0 / PROTON-1486 to actually be available before merging this since it isnt yet pressing and it saves master failing to compile later once the snapshots aren't around. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128747005
--- Diff:
qpid-jms-client/src/main/java/org/apache/qpid/jms/sasl/GssapiMechanism.java ---
@@ -0,0 +1,163 @@
+/*
+ * 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.
+ */
+package org.apache.qpid.jms.sasl;
+
+import javax.security.auth.Subject;
+import javax.security.auth.login.AppConfigurationEntry;
+import javax.security.auth.login.Configuration;
+import javax.security.auth.login.LoginContext;
+import javax.security.sasl.Sasl;
+import javax.security.sasl.SaslClient;
+import javax.security.sasl.SaslException;
+import java.security.Principal;
+import java.security.PrivilegedActionException;
+import java.security.PrivilegedExceptionAction;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Implements the GSSAPI sasl authentication Mechanism.
+ */
+public class GssapiMechanism extends AbstractMechanism {
+
+public static final String NAME = "GSSAPI";
+private Subject subject;
+private SaslClient saslClient;
+private String protocol = "amqp";
+private String server = null;
+private String configScope = null;
+
+// a gss/sasl service name, x@y, morphs to a krbPrincipal a/y@REALM
+
+@Override
+public int getPriority() {
+return PRIORITY.LOW.getValue();
+}
+
+@Override
+public String getName() {
+return NAME;
+}
+
+@Override
+public byte[] getInitialResponse() throws SaslException {
+try {
+LoginContext loginContext = null;
+if (configScope != null) {
+loginContext = new LoginContext(configScope);
+} else {
+// inline keytab config using user as principal
+loginContext = new LoginContext("", null, null,
+kerb5InlineConfig(getUsername(), true));
+}
+loginContext.login();
+subject = loginContext.getSubject();
+
+return Subject.doAs(subject, new
PrivilegedExceptionAction() {
+
+@Override
+public byte[] run() throws Exception {
+saslClient = Sasl.createSaslClient(new
String[]{getName()}, null, protocol, server, null, null);
--- End diff --
Referencing the constant rather than using getName() might be clearer.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128749066
--- Diff:
qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SaslIntegrationTest.java
---
@@ -54,6 +54,7 @@
private static final Symbol SCRAM_SHA_1 =
Symbol.valueOf("SCRAM-SHA-1");
private static final Symbol SCRAM_SHA_256 =
Symbol.valueOf("SCRAM-SHA-256");
private static final Symbol EXTERNAL = Symbol.valueOf("EXTERNAL");
+private static final Symbol GSSAPI = Symbol.valueOf("GSSAPI");
--- End diff --
This isn't used and can be 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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128748789
--- Diff:
qpid-jms-client/src/main/java/org/apache/qpid/jms/sasl/GssapiMechanism.java ---
@@ -0,0 +1,163 @@
+/*
+ * 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.
+ */
+package org.apache.qpid.jms.sasl;
+
+import javax.security.auth.Subject;
+import javax.security.auth.login.AppConfigurationEntry;
+import javax.security.auth.login.Configuration;
+import javax.security.auth.login.LoginContext;
+import javax.security.sasl.Sasl;
+import javax.security.sasl.SaslClient;
+import javax.security.sasl.SaslException;
+import java.security.Principal;
+import java.security.PrivilegedActionException;
+import java.security.PrivilegedExceptionAction;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Implements the GSSAPI sasl authentication Mechanism.
+ */
+public class GssapiMechanism extends AbstractMechanism {
+
+public static final String NAME = "GSSAPI";
+private Subject subject;
+private SaslClient saslClient;
+private String protocol = "amqp";
+private String server = null;
+private String configScope = null;
+
+// a gss/sasl service name, x@y, morphs to a krbPrincipal a/y@REALM
+
+@Override
+public int getPriority() {
+return PRIORITY.LOW.getValue();
+}
+
+@Override
+public String getName() {
+return NAME;
+}
+
+@Override
+public byte[] getInitialResponse() throws SaslException {
+try {
+LoginContext loginContext = null;
+if (configScope != null) {
+loginContext = new LoginContext(configScope);
+} else {
+// inline keytab config using user as principal
+loginContext = new LoginContext("", null, null,
+kerb5InlineConfig(getUsername(), true));
+}
+loginContext.login();
+subject = loginContext.getSubject();
+
+return Subject.doAs(subject, new
PrivilegedExceptionAction() {
+
+@Override
+public byte[] run() throws Exception {
+saslClient = Sasl.createSaslClient(new
String[]{getName()}, null, protocol, server, null, null);
+if (saslClient.hasInitialResponse()) {
+return saslClient.evaluateChallenge(new byte[0]);
+}
+return null;
+}
+});
+} catch (Exception e) {
+throw new SaslException(e.toString(), e);
+}
+}
+
+@Override
+public byte[] getChallengeResponse(final byte[] challenge) throws
SaslException {
+try {
+return Subject.doAs(subject, new
PrivilegedExceptionAction() {
+@Override
+public byte[] run() throws Exception {
+return saslClient.evaluateChallenge(challenge);
+}
+});
+} catch (PrivilegedActionException e) {
+throw new SaslException(e.toString(), e);
+}
+}
+
+@Override
+public void verifyCompletion() throws SaslException {
+boolean result = saslClient.isComplete();
+saslClient.dispose();
+if (!result) {
+throw new SaslException("not complete");
+}
+}
+
+
+@Override
+public boolean isApplicable(String username, String password,
Principal localPrincipal) {
+return true;
+}
+
+public static Configuration kerb5InlineConfig(String principal,
boolean initiator) {
+final Map krb5LoginModuleOptions = new HashMap<>();
+k
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128752547
--- Diff: qpid-jms-client/src/test/resources/minikdc-krb5.conf ---
@@ -0,0 +1,26 @@
+#
+# 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.
+#
+[libdefaults]
+default_realm = {0}
+udp_preference_limit = 1
+default_keytab_name = FILE:target/test.krb5.keytab
+
+[realms]
+{0} = '{'
+kdc = {1}:{2}
+'}'
--- End diff --
As with the login config, naming it more specifically for the test its used
in would be good.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128726699
--- Diff:
qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticator.java
---
@@ -137,7 +142,9 @@ private void handleSaslStep() throws
JMSSecurityException {
byte[] challenge = new byte[sasl.pending()];
sasl.recv(challenge, 0, challenge.length);
byte[] response =
mechanism.getChallengeResponse(challenge);
-sasl.send(response, 0, response.length);
+if (response != null) {
--- End diff --
Did you need this null check? The SASL process is likely to hang if there
is no response to a challenge. I think its reasonable to require one be given,
or fail if there isn't and an exception isn't thrown.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128727012
--- Diff: qpid-jms-client/pom.xml ---
@@ -93,6 +93,19 @@
hamcrest-all
test
+
+ org.apache.hadoop
+ hadoop-minikdc
+ ${hadoop-minikdc-version}
--- End diff --
The versions should be controlled in the parent dependencyManagement
section along with the others.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128754720
--- Diff:
qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SaslGssApiIntegrationTest.java
---
@@ -0,0 +1,185 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.integration;
+
+import org.apache.directory.server.kerberos.shared.keytab.Keytab;
+import org.apache.directory.server.kerberos.shared.keytab.KeytabEntry;
+import org.apache.hadoop.minikdc.MiniKdc;
+import org.apache.qpid.jms.JmsConnectionFactory;
+import org.apache.qpid.jms.test.QpidJmsTestCase;
+import org.apache.qpid.jms.test.testpeer.TestAmqpPeer;
+import org.apache.qpid.proton.amqp.Symbol;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.JMSSecurityException;
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import static junit.framework.TestCase.assertTrue;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+
+public class SaslGssApiIntegrationTest extends QpidJmsTestCase {
+
+private static final Logger LOG =
LoggerFactory.getLogger(SaslGssApiIntegrationTest.class);
+
+private static final Symbol GSSAPI = Symbol.valueOf("GSSAPI");
+private static final String serviceName = "amqp/localhost";
+
+private MiniKdc kdc;
+
+@Before
+public void setUpKerberso() throws Exception {
+Path tempDirectory = Files.createTempDirectory("junit.test.");
+File root = tempDirectory.toFile();
+root.deleteOnExit();
+kdc = new MiniKdc(MiniKdc.createConf(), new File(root, "kdc"));
+kdc.start();
+
+// hard coded match, default_keytab_name in minikdc-krb5.conf
template
+File userKeyTab = new File("target/test.krb5.keytab");
+kdc.createPrincipal(userKeyTab, "client", serviceName);
+
+Keytab kt = Keytab.read(userKeyTab);
+for (KeytabEntry entry : kt.getEntries()) {
+LOG.info("KeyTab Kerb PrincipalNames:" +
entry.getPrincipalName());
+}
+
+java.util.logging.Logger logger =
java.util.logging.Logger.getLogger("javax.security.sasl");
--- End diff --
Is this just for debug? Might be good to make it optional, rather than
leave it changing the config for all subsequent 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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128749340
--- Diff:
qpid-jms-client/src/test/java/org/apache/qpid/jms/test/testpeer/TestAmqpPeer.java
---
@@ -495,6 +512,169 @@ public void run()
}
}
+public void expectGSSAPIFail(Symbol mech) throws Exception {
+SaslMechanismsFrame saslMechanismsFrame = new
SaslMechanismsFrame().setSaslServerMechanisms(mech);
+
+addHandler(new HeaderHandlerImpl(AmqpHeader.SASL_HEADER,
AmqpHeader.SASL_HEADER,
+new FrameSender(
+this, FrameType.SASL, 0,
+saslMechanismsFrame, null)));
+
+addHandler(new SaslInitMatcher().withMechanism(equalTo(mech)));
+
+}
+
+public void expectGSSAPI(Symbol mech, String serviceName) throws
Exception {
+
+SaslMechanismsFrame saslMechanismsFrame = new
SaslMechanismsFrame().setSaslServerMechanisms(mech);
+
+addHandler(new HeaderHandlerImpl(AmqpHeader.SASL_HEADER,
AmqpHeader.SASL_HEADER,
+new FrameSender(
+this, FrameType.SASL, 0,
+saslMechanismsFrame, null)));
+
+// setup server gss context
+LoginContext loginContext = new LoginContext("", null, null,
+kerb5InlineConfig(serviceName, false));
--- End diff --
Using this presumably means this test will fail on the IBM JDK since it
references the Sun login class? That probably needs addressed in some way.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128754756
--- Diff:
qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SaslGssApiIntegrationTest.java
---
@@ -0,0 +1,185 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.integration;
+
+import org.apache.directory.server.kerberos.shared.keytab.Keytab;
+import org.apache.directory.server.kerberos.shared.keytab.KeytabEntry;
+import org.apache.hadoop.minikdc.MiniKdc;
+import org.apache.qpid.jms.JmsConnectionFactory;
+import org.apache.qpid.jms.test.QpidJmsTestCase;
+import org.apache.qpid.jms.test.testpeer.TestAmqpPeer;
+import org.apache.qpid.proton.amqp.Symbol;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.JMSSecurityException;
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import static junit.framework.TestCase.assertTrue;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+
+public class SaslGssApiIntegrationTest extends QpidJmsTestCase {
+
+private static final Logger LOG =
LoggerFactory.getLogger(SaslGssApiIntegrationTest.class);
+
+private static final Symbol GSSAPI = Symbol.valueOf("GSSAPI");
+private static final String serviceName = "amqp/localhost";
+
+private MiniKdc kdc;
+
+@Before
+public void setUpKerberso() throws Exception {
--- End diff --
Typo in name.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128747741
--- Diff: qpid-jms-client/src/main/resources/login.config ---
@@ -0,0 +1,23 @@
+/*
+ * 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.
+ */
+
+KRB5-CLIENT {
+com.sun.security.auth.module.Krb5LoginModule required
+useKeyTab=true
+principal="client"
+keytab="target/test.krb5.keytab";
+};
--- End diff --
Should this file be in the src/test tree rather than in src/main?
Naming the file more specifically for the test it is used in, e.g
-login.config, would be good for later. Perhaps similarly for the
keytab.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128750540
--- Diff:
qpid-jms-client/src/test/java/org/apache/qpid/jms/test/testpeer/TestAmqpPeer.java
---
@@ -495,6 +512,169 @@ public void run()
}
}
+public void expectGSSAPIFail(Symbol mech) throws Exception {
+SaslMechanismsFrame saslMechanismsFrame = new
SaslMechanismsFrame().setSaslServerMechanisms(mech);
+
+addHandler(new HeaderHandlerImpl(AmqpHeader.SASL_HEADER,
AmqpHeader.SASL_HEADER,
+new FrameSender(
+this, FrameType.SASL, 0,
+saslMechanismsFrame, null)));
+
+addHandler(new SaslInitMatcher().withMechanism(equalTo(mech)));
+
+}
+
+public void expectGSSAPI(Symbol mech, String serviceName) throws
Exception {
--- End diff --
Rename to expectSaslGSSAPI for consistency with the other sasl verification
methods. Passing the mechanism name is perhaps also a bit redundant and could
be removed. Similarly for the expectGSSAPIFail.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128755403
--- Diff:
qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SaslGssApiIntegrationTest.java
---
@@ -0,0 +1,185 @@
+/*
+ *
+ * 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.
+ *
+ */
+package org.apache.qpid.jms.integration;
+
+import org.apache.directory.server.kerberos.shared.keytab.Keytab;
+import org.apache.directory.server.kerberos.shared.keytab.KeytabEntry;
+import org.apache.hadoop.minikdc.MiniKdc;
+import org.apache.qpid.jms.JmsConnectionFactory;
+import org.apache.qpid.jms.test.QpidJmsTestCase;
+import org.apache.qpid.jms.test.testpeer.TestAmqpPeer;
+import org.apache.qpid.proton.amqp.Symbol;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.JMSSecurityException;
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import static junit.framework.TestCase.assertTrue;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+
+public class SaslGssApiIntegrationTest extends QpidJmsTestCase {
+
+private static final Logger LOG =
LoggerFactory.getLogger(SaslGssApiIntegrationTest.class);
+
+private static final Symbol GSSAPI = Symbol.valueOf("GSSAPI");
+private static final String serviceName = "amqp/localhost";
+
+private MiniKdc kdc;
+
+@Before
+public void setUpKerberso() throws Exception {
+Path tempDirectory = Files.createTempDirectory("junit.test.");
--- End diff --
Including the test name would also be good to help trace any issues back
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128754495
--- Diff:
qpid-jms-client/src/test/java/org/apache/qpid/jms/test/testpeer/TestAmqpPeer.java
---
@@ -495,6 +512,169 @@ public void run()
}
}
+public void expectGSSAPIFail(Symbol mech) throws Exception {
+SaslMechanismsFrame saslMechanismsFrame = new
SaslMechanismsFrame().setSaslServerMechanisms(mech);
+
+addHandler(new HeaderHandlerImpl(AmqpHeader.SASL_HEADER,
AmqpHeader.SASL_HEADER,
+new FrameSender(
+this, FrameType.SASL, 0,
+saslMechanismsFrame, null)));
+
+addHandler(new SaslInitMatcher().withMechanism(equalTo(mech)));
+
+}
+
+public void expectGSSAPI(Symbol mech, String serviceName) throws
Exception {
+
+SaslMechanismsFrame saslMechanismsFrame = new
SaslMechanismsFrame().setSaslServerMechanisms(mech);
+
+addHandler(new HeaderHandlerImpl(AmqpHeader.SASL_HEADER,
AmqpHeader.SASL_HEADER,
+new FrameSender(
+this, FrameType.SASL, 0,
+saslMechanismsFrame, null)));
+
+// setup server gss context
+LoginContext loginContext = new LoginContext("", null, null,
+kerb5InlineConfig(serviceName, false));
+loginContext.login();
+final Subject serverSubject =loginContext.getSubject();
+
+LOGGER.info("saslServer subject:" +
serverSubject.getPrivateCredentials());
+
+Map config = new HashMap();
+final CallbackHandler handler = new CallbackHandler() {
+@Override
+public void handle(Callback[] callbacks) throws IOException,
UnsupportedCallbackException {
+LOGGER.info("Here with: " + Arrays.asList(callbacks));
+for (Callback callback :callbacks) {
+if (callback instanceof AuthorizeCallback) {
+AuthorizeCallback authorizeCallback =
(AuthorizeCallback) callback;
+
authorizeCallback.setAuthorized(authorizeCallback.getAuthenticationID().equals(authorizeCallback.getAuthorizationID()));
+}
+}
+}
+};
+final SaslServer saslServer = Subject.doAs(serverSubject, new
PrivilegedExceptionAction() {
+@Override
+public SaslServer run() throws Exception {
+return Sasl.createSaslServer(mech.toString(), null, null,
config, handler);
+}
+});
+
+final SaslChallengeFrame challengeFrame = new SaslChallengeFrame();
+
+SaslInitMatcher saslInitMatcher = new SaslInitMatcher()
+.withMechanism(equalTo(mech))
+.withInitialResponse(new BaseMatcher() {
+
+@Override
+public void describeTo(Description description) {}
+
+@Override
+public boolean matches(Object o) {
+if (o == null) {
+LOGGER.error("Got null initial response!");
+return false;
+}
+final Binary binary = (Binary) o;
+// validate via sasl
+byte[] token = null;
+try {
+token = Subject.doAs(serverSubject, new
PrivilegedExceptionAction() {
+@Override
+public byte[] run() throws Exception {
+LOGGER.info("Evaluate Response..
size:" + binary.getLength());
+return
saslServer.evaluateResponse(binary.getArray());
+}
+});
+} catch (PrivilegedActionException e) {
+e.printStackTrace();
+}
+LOGGER.info("Complete:" + saslServer.isComplete());
+
+if (token != null) {
+// fling it back in on complete
+challengeFrame.setChallenge(new Binary(token));
+}
+return true;
+}
+}).onCompletion(new AmqpPeerRunnable() {
+@Override
+public void run() {
+TestAmqpPeer.this.se
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128807526
--- Diff:
qpid-jms-client/src/main/java/org/apache/qpid/jms/sasl/GssapiMechanism.java ---
@@ -0,0 +1,190 @@
+/*
+ * 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.
+ */
+package org.apache.qpid.jms.sasl;
+
+import javax.security.auth.Subject;
+import javax.security.auth.login.AppConfigurationEntry;
+import javax.security.auth.login.Configuration;
+import javax.security.auth.login.LoginContext;
+import javax.security.sasl.Sasl;
+import javax.security.sasl.SaslClient;
+import javax.security.sasl.SaslException;
+import java.security.Principal;
+import java.security.PrivilegedActionException;
+import java.security.PrivilegedExceptionAction;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Implements the GSSAPI sasl authentication Mechanism.
+ */
+public class GssapiMechanism extends AbstractMechanism {
+
+public static final String NAME = "GSSAPI";
+private Subject subject;
+private SaslClient saslClient;
+private String protocol = "amqp";
+private String serverName = null;
+private String configScope = null;
+
+// a gss/sasl service name, x@y, morphs to a krbPrincipal a/y@REALM
+
+@Override
+public int getPriority() {
+return PRIORITY.LOW.getValue();
+}
+
+@Override
+public String getName() {
+return NAME;
+}
+
+@Override
+public byte[] getInitialResponse() throws SaslException {
+try {
+LoginContext loginContext = null;
+if (configScope != null) {
+loginContext = new LoginContext(configScope);
+} else {
+// inline keytab config using user as principal
+loginContext = new LoginContext("", null, null,
+kerb5InlineConfig(getUsername(), true));
+}
+loginContext.login();
+subject = loginContext.getSubject();
+
+return Subject.doAs(subject, new
PrivilegedExceptionAction() {
+
+@Override
+public byte[] run() throws Exception {
+saslClient = Sasl.createSaslClient(new String[]{NAME},
null, protocol, serverName, null, null);
+if (saslClient.hasInitialResponse()) {
+return saslClient.evaluateChallenge(new byte[0]);
+}
+return null;
+}
+});
+} catch (Exception e) {
+throw new SaslException(e.toString(), e);
+}
+}
+
+@Override
+public byte[] getChallengeResponse(final byte[] challenge) throws
SaslException {
+try {
+return Subject.doAs(subject, new
PrivilegedExceptionAction() {
+@Override
+public byte[] run() throws Exception {
+return saslClient.evaluateChallenge(challenge);
+}
+});
+} catch (PrivilegedActionException e) {
+throw new SaslException(e.toString(), e);
+}
+}
+
+@Override
+public void verifyCompletion() throws SaslException {
+boolean result = saslClient.isComplete();
+saslClient.dispose();
+if (!result) {
+throw new SaslException("not complete");
+}
+}
+
+
+@Override
+public boolean isApplicable(String username, String password,
Principal localPrincipal) {
+return true;
+}
+
+private static final boolean IBM_JAVA =
System.getProperty("java.vendor").contains("IBM");
+public static Configuration kerb5InlineConfig(Stri
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128808271
--- Diff:
qpid-jms-client/src/main/java/org/apache/qpid/jms/sasl/GssapiMechanism.java ---
@@ -0,0 +1,190 @@
+/*
+ * 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.
+ */
+package org.apache.qpid.jms.sasl;
+
+import javax.security.auth.Subject;
+import javax.security.auth.login.AppConfigurationEntry;
+import javax.security.auth.login.Configuration;
+import javax.security.auth.login.LoginContext;
+import javax.security.sasl.Sasl;
+import javax.security.sasl.SaslClient;
+import javax.security.sasl.SaslException;
+import java.security.Principal;
+import java.security.PrivilegedActionException;
+import java.security.PrivilegedExceptionAction;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Implements the GSSAPI sasl authentication Mechanism.
+ */
+public class GssapiMechanism extends AbstractMechanism {
+
+public static final String NAME = "GSSAPI";
+private Subject subject;
+private SaslClient saslClient;
+private String protocol = "amqp";
+private String serverName = null;
+private String configScope = null;
+
+// a gss/sasl service name, x@y, morphs to a krbPrincipal a/y@REALM
+
+@Override
+public int getPriority() {
+return PRIORITY.LOW.getValue();
+}
+
+@Override
+public String getName() {
+return NAME;
+}
+
+@Override
+public byte[] getInitialResponse() throws SaslException {
+try {
+LoginContext loginContext = null;
+if (configScope != null) {
+loginContext = new LoginContext(configScope);
+} else {
+// inline keytab config using user as principal
+loginContext = new LoginContext("", null, null,
+kerb5InlineConfig(getUsername(), true));
+}
+loginContext.login();
+subject = loginContext.getSubject();
+
+return Subject.doAs(subject, new
PrivilegedExceptionAction() {
+
+@Override
+public byte[] run() throws Exception {
+saslClient = Sasl.createSaslClient(new String[]{NAME},
null, protocol, serverName, null, null);
+if (saslClient.hasInitialResponse()) {
+return saslClient.evaluateChallenge(new byte[0]);
+}
+return null;
+}
+});
+} catch (Exception e) {
+throw new SaslException(e.toString(), e);
+}
+}
+
+@Override
+public byte[] getChallengeResponse(final byte[] challenge) throws
SaslException {
+try {
+return Subject.doAs(subject, new
PrivilegedExceptionAction() {
+@Override
+public byte[] run() throws Exception {
+return saslClient.evaluateChallenge(challenge);
+}
+});
+} catch (PrivilegedActionException e) {
+throw new SaslException(e.toString(), e);
+}
+}
+
+@Override
+public void verifyCompletion() throws SaslException {
+boolean result = saslClient.isComplete();
+saslClient.dispose();
+if (!result) {
+throw new SaslException("not complete");
+}
+}
+
+
+@Override
+public boolean isApplicable(String username, String password,
Principal localPrincipal) {
+return true;
+}
+
+private static final boolean IBM_JAVA =
System.getProperty("java.vendor").contains("IBM");
+public static Configuration kerb5InlineConfig(Stri
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128810019
--- Diff:
qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticator.java
---
@@ -137,7 +142,9 @@ private void handleSaslStep() throws
JMSSecurityException {
byte[] challenge = new byte[sasl.pending()];
sasl.recv(challenge, 0, challenge.length);
byte[] response =
mechanism.getChallengeResponse(challenge);
-sasl.send(response, 0, response.length);
+if (response != null) {
--- End diff --
Good point. Interesting, I guess it is allowing for the 'additional-data'
style information at the end of the exchange where there isn't a response.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-jms/pull/10#discussion_r128809457
--- Diff:
qpid-jms-client/src/test/java/org/apache/qpid/jms/test/testpeer/TestAmqpPeer.java
---
@@ -495,6 +512,169 @@ public void run()
}
}
+public void expectGSSAPIFail(Symbol mech) throws Exception {
+SaslMechanismsFrame saslMechanismsFrame = new
SaslMechanismsFrame().setSaslServerMechanisms(mech);
+
+addHandler(new HeaderHandlerImpl(AmqpHeader.SASL_HEADER,
AmqpHeader.SASL_HEADER,
+new FrameSender(
+this, FrameType.SASL, 0,
+saslMechanismsFrame, null)));
+
+addHandler(new SaslInitMatcher().withMechanism(equalTo(mech)));
+
+}
+
+public void expectGSSAPI(Symbol mech, String serviceName) throws
Exception {
+
+SaslMechanismsFrame saslMechanismsFrame = new
SaslMechanismsFrame().setSaslServerMechanisms(mech);
+
+addHandler(new HeaderHandlerImpl(AmqpHeader.SASL_HEADER,
AmqpHeader.SASL_HEADER,
+new FrameSender(
+this, FrameType.SASL, 0,
+saslMechanismsFrame, null)));
+
+// setup server gss context
+LoginContext loginContext = new LoginContext("", null, null,
+kerb5InlineConfig(serviceName, false));
+loginContext.login();
+final Subject serverSubject =loginContext.getSubject();
+
+LOGGER.info("saslServer subject:" +
serverSubject.getPrivateCredentials());
+
+Map config = new HashMap();
+final CallbackHandler handler = new CallbackHandler() {
+@Override
+public void handle(Callback[] callbacks) throws IOException,
UnsupportedCallbackException {
+LOGGER.info("Here with: " + Arrays.asList(callbacks));
+for (Callback callback :callbacks) {
+if (callback instanceof AuthorizeCallback) {
+AuthorizeCallback authorizeCallback =
(AuthorizeCallback) callback;
+
authorizeCallback.setAuthorized(authorizeCallback.getAuthenticationID().equals(authorizeCallback.getAuthorizationID()));
+}
+}
+}
+};
+final SaslServer saslServer = Subject.doAs(serverSubject, new
PrivilegedExceptionAction() {
+@Override
+public SaslServer run() throws Exception {
+return Sasl.createSaslServer(mech.toString(), null, null,
config, handler);
+}
+});
+
+final SaslChallengeFrame challengeFrame = new SaslChallengeFrame();
+
+SaslInitMatcher saslInitMatcher = new SaslInitMatcher()
+.withMechanism(equalTo(mech))
+.withInitialResponse(new BaseMatcher() {
+
+@Override
+public void describeTo(Description description) {}
+
+@Override
+public boolean matches(Object o) {
+if (o == null) {
+LOGGER.error("Got null initial response!");
+return false;
+}
+final Binary binary = (Binary) o;
+// validate via sasl
+byte[] token = null;
+try {
+token = Subject.doAs(serverSubject, new
PrivilegedExceptionAction() {
+@Override
+public byte[] run() throws Exception {
+LOGGER.info("Evaluate Response..
size:" + binary.getLength());
+return
saslServer.evaluateResponse(binary.getArray());
+}
+});
+} catch (PrivilegedActionException e) {
+e.printStackTrace();
+}
+LOGGER.info("Complete:" + saslServer.isComplete());
+
+if (token != null) {
+// fling it back in on complete
+challengeFrame.setChallenge(new Binary(token));
+}
+return true;
+}
+}).onCompletion(new AmqpPeerRunnable() {
+@Override
+public void run() {
+TestAmqpPeer.this.se
[GitHub] qpid-dispatch issue #190: DISPATCH-802 - Additional fixes - 1. Modified erro...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-dispatch/pull/190 Tested the latest version and the attach target is now null as needed to indicate refusal, and the updated error details look good. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/12 I don't think the various environment etc checking being done really belongs in the client, it seems like that stuff belongs in Netty if the isAvailable() checks it offers aren't actually sufficient. Similarly it feels a little odd making it check for being on a Mac when KQueue is really for BSD rather than OSX specific, and they might open up support for others later, again it feels like Netty is better placed to be checking that type of thing. I also think that if added the KQueue bit would be better set off by default for now and flipped later on. It's very new in Netty itself, whereas the epoll stuff had time to mature before it was added. Linux is also probably the primary platform we test and use the client on, whereas we don't really test on OS X at all (though I shall change that shortly), so sticking to the NIO bits by default there seems wiser for 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/12 > Agreed its not ideal we have to do this, but this was found on epoll in artemis (which you already have epoll implemented and defaulted to true) , as such until netty improve the check then this check is worth while for the interim unless you default epoll to false currently the client has the same issue with epoll as found - https://issues.apache.org/jira/browse/ARTEMIS-1099 Given it seems it only happens if you use a 32bit JVM and only emits a warning, I don't think its serious enough to introduce all that not-ideal code for the existing epoll bits when noone has yet complained about it with the client. This is however an opportunity to get it addressed in Netty instead where it seems like it belongs. > The classifier for the dependency for kqueue implementation is for osx build, there isn't one currently for freeBSD or other derivaitves (osx being bsd based), as such the check for the platform being osx, to be safe. E.g. our case is we do have java clients that are on macOS. I realise that the existing artifact is targetting OSX, but that could change and its just another ugly check for us to fall afoul of later. Netty better knows what the restrictions on its usage are than we do, it seems like its availability check should be handling such things. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #10: PROTON-1551: Fix the problem with binary content ov...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/10 Can you please squash the changes into a single commit and make the commit message a little more specific, e.g "PROTON-1551: fix length encoding for binary over 255 bytes in length when using DataImpl". --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-cpp issue #4: QPID-7702: Fix some minor memory leaks detected by Coveri...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-cpp/pull/4 @kgiusti Can you close the PR? This looks to have been applied in https://git-wip-us.apache.org/repos/asf?p=qpid-cpp.git;h=f91a23c. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-cpp issue #6: QPID-7875 qpid-route fetches links multiple times when de...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-cpp/pull/6 @chrisrichardson77 A change for this looks to have been made in https://git-wip-us.apache.org/repos/asf?p=qpid-cpp.git;a=commitdiff;h=a8d392efe30ef763d2e93c6ce733976ac786c0f0, can you close the PR please? (The commit log message that can do it wasnt used when the fix was committed, and as its a read-only mirror we cant do it directly) --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-cpp issue #7: QPID-7876 qpid-route does not properly consider src-local...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-cpp/pull/7 @chrisrichardson77 A change for this looks to have been made in https://git-wip-us.apache.org/repos/asf?p=qpid-cpp.git;a=commitdiff;h=edccbc9e1737603a1d1f66f0df8499dbba07e93b, can you close the PR please? (The commit log message that can do it wasnt used when the fix was committed, and as its a read-only mirror we cant do it directly) --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-cpp issue #8: QPID-7874 Use qpid-route quiet option to suppress extra l...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-cpp/pull/8 @chrisrichardson77 A change for this looks to have been made in https://git-wip-us.apache.org/repos/asf?p=qpid-cpp.git;h=55d4171, can you close the PR please? (The commit log message that can do it wasnt used when the fix was committed, and as its a read-only mirror we cant do it directly) --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-dispatch issue #212: DISPATCH-858 - Moved third party text into files a...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-dispatch/pull/212 @ganeshmurthy looks good --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #128: PROTON-1342: CI on OS X
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/128 I agree on reducing the number of builds. The /apache org has a shared job capacity for all the foundations projects, so we dont want to be overloading things, and tripling the number just to add OSX seems like its getting there. I'd probably do what Andrew said and pick the latest plus something earlier. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #129: PROTON-522: Apache Qpid Proton on Mac/OSX - C/Object...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/129 @astitcher seems to have made various changes related to these ones in commits such as: d82bbfab037c97e1c403ae701f1b3fe272813ff7 , b6ad8a996faa34aeb8e475902a6f151c5476d45f, 43e49f69cda68afc948a8d9081084dea70cb960c , e9480cb3cd46e3f1437a0b03462cdf2bcd52bb25 , 6dddff298ed220ec4621e61eabb4b4cb8ab908fd , and f07322d9c596e334f4eef8f14d0c41ff3b3e8704. You might want to check what the current state is on master and either rebase this or close it and raise specific new JIRAs+PRs for any further changes needed. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #12: PROTON-1690 JMH Benchmarks for baseline performance...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/12 Hi Franz Thanks for the PR. I have some feedback for changes that would be good and/or necessary. The JMH dependencies are not ALv2 compatible and so I think we should make it an optional module, which doesn't deploy anything. I think it will often be true people wont want to run those tests regularly enough to warrant including them with every build, so I think it also makes sense to make it optional for that reason anyway. Making it so unfortunately adds some issues around versioning during the release, as we will still want the module versions to get updated, so we need to adjust the process slightly to handle that. I'd also suggest that we move them under the tests directory. I've had a stab at making the build/release work with this all in mind, going with tests/performance-jmh as the new directory. I've pasted a patch at https://paste.apache.org/lgCc with some of the changes. The changes for the root pom.xml are the diff from master, instead of those here rather than on top, while the tests/performance-jmh/pom.xml is the full new file I ended up with rather than a diff from here. With the updated poms, the module can be enabled as part of the overall build by adding -Pperformance-jmh, or it can just be built serparately using its own pom directly. Theres some trickery in there to ensure the release is performed in a way that updates the versions, or else fails. The LICENSE file being added here can be removed, its covered by the one in the root dir already. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #13: Added API to Transport interface to allow custom sa...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/13 My immediate reaction is that this isn't acceptable. It exposes various APIs that are considered part of the implementation only, and then further exposes additional implementation detail presumably to subclass it, which it similarly isn't intended for. Given the general nature of the engines Sasl object/api, would I be right to assume the reason you want this ability is mainly just due to limitations imposed from use within the Reactor? If so perhaps theres a nicer reactor-only approach that can be explored, or alternatively perhaps theres a neat way to tie it in with the existing layering exposed by TransportInternal for similar reactor related layering reasons. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #13: Added API to Transport interface to allow custom sa...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/13 For completeness, a different change was made via https://issues.apache.org/jira/browse/PROTON-1736 in commit 17cef9ace9a7c75901d517f951ae1d4610819436. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-dispatch issue #252: Tross dispatch 845 1
Github user gemmellr commented on the issue: https://github.com/apache/qpid-dispatch/pull/252 Couple of questions. Probably a thing to consider even with the existing config method, but how are routes treated when more than one connection says it matches something? How does this integrate with the policy stuff, e.g. is it possible to control who can set up routes this way and which addresses they can do it for? --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #140: PROTON-1638, PROTON-1728: Reorganize the source tree
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/140 I gave things a skim over, changes seemed reasonable from my doesnt-do-cmake-etc perspective. Noting for later reference: this builds on / incorporates WIP from #136 and #138. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-cpp issue #12: WIP - A batch of C++ updates
Github user gemmellr commented on the issue: https://github.com/apache/qpid-cpp/pull/12 Docs, removing 'untested proton version' warning, etc, changes look good to me. I'll trust Gordon about the code :) --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #142: PROTON-1831: Incorrect open/close sequence for same ...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/142 The use of "invalid-field" for the error feels like it is maybe a little off. Its possible/likely there is nothing wrong with the information in the fields of the attach, as none of it is really actually checked (e.g whether its the same remote handle, itself a session:handle-in-use error) other than someone got there previously with the name. Perhaps "illegal-state" might be more applicable given the actual check being done? That said though, its not actually 100% clear it is an illegal state. The spec covers attaching an active link a second time as resulting in the first attachment being detached with the "stolen" link error. Its described for handling across different connections (something proton thus cant actually do on its own) e.g during reconnect, but its not clear that precludes it being the case on a single session or connection. However, it does seem far more likely to be in result of an error for the situation to ever occur on a single session/connection so perhaps treating it as such is the way to go in the end (plus any alternative would I guess be much harder to achieve, so maybe its the only choice for now). --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #142: PROTON-1831: Incorrect open/close sequence for same ...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/142 > Thanks - I didn't know about stolen. I think we could implement what the spec says on the server side - close the old link/handle with "stolen" and create a new link under the new handle. In the same place its doing it now, or for the whole connection? (Still leaving other connections as something for the using server, e.g dispatch, to handle on its own). Its worth saying that it doesnt currently check the handle is appropriate as it chokes on the name first. It'd need to check the handles more fully too, e.g two attaches with the same name and the same remote handle would still be an session:handle-in-use error. > That would fix the crash, which was because of multiple handles to the same link object - we'd have a seprate link object for each handle, all but one of which are closed with "stolen". One further annoyance is how to inform the stuff already using the link that this all happened. As I mentioned elsewhere before, this also starts getting into issues with limits checking e.g maxHandles (as it may still be 'in use' until the engine is processed later). >I think on the client side its always an error to try to attach if you already have a link with same name an a handle. After disconnect all handles are cleared, so it is OK an error to re-attach during reconnect. I want to fix the client to refuse to create a link at all in this case. That would prevent this happening on the wire and give more immediate feedback to code that is mistakenly trying to double-attach. I think thats fair personally, always have. > The fix is not trivial due to proton's batch-processing madness or I'd have done it first. I feel your pain from previously looking at similar stuff :) --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #142: PROTON-1831: Incorrect open/close sequence for same ...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/142 It seems you already committed this before Gordon and I commented, but neglected to close the PR with the commit or otherwise. It either needs closing, or the commit reverted, depending what your intent is after the comments. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms issue #20: Issue QPIDJMS-400: Add missing OSGI resolution:=optional...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/20 I agree the Travis CI OSX failure is unrelated. There isn't a way to trigger a new build on the github mirror without updating the PR. I don't think this change is actually quite the way to go. The 'optional' flag was not missing for the epoll bits since the codebase actually needs the epoll classes by default, its used by default and the 'is epoll functionality available currently' checking exists in that code. The kqueue bits are not used by default and so perhaps that could be marked optional. Regardless, there are actually two variants of the jars published, a classified platform-specific one, and a non-classified neutral one that doesn't actually contain the native libs but still provides the Java code. Which were you trying to use? --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms issue #20: Issue QPIDJMS-400: Add missing OSGI resolution:=optional...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/20 > FYI, attempting to exclude the transitive dependency on the kqueue jar so only the epoll jar is available in the bundle does not fix the problem, the OSGI container again complains about the missing kqueue dependency because the OSGI manifest "requires" the bundle to be present. Which makes sense. Rather than exclude it entirely, did you try using the platform-neutral non-classified dependency instead? i.e try: http://repo2.maven.org/maven2/io/netty/netty-transport-native-kqueue/4.1.25.Final/netty-transport-native-kqueue-4.1.25.Final.jar (rather than: http://repo2.maven.org/maven2/io/netty/netty-transport-native-kqueue/4.1.25.Final/netty-transport-native-kqueue-4.1.25.Final-osx-x86_64.jar) --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms issue #20: Issue QPIDJMS-400: Add missing OSGI resolution:=optional...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/20 That wont actually work since the epoll bits are required by default, as the classes are used by default, in determining whether the functionality is currently available. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #133: PROTON-1796 branch for automated periodic Coverity S...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/133 I don't think we have ability to configure periodic builds in Travis on this mirror, over which we have limited control in which case this doesn't really make sense. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #133: PROTON-1796 branch for automated periodic Coverity S...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/133 I meant the PR probably doesnt make sense, i.e we wouldnt have a branch here, which it seems you agree with. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #14: PROTON-1911: Improve performance of EncoderImpl#wri...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/14 After some investigation and testing, a different approach (https://github.com/apache/qpid-proton-j/commit/7eac8b945c8ce90f091126d34cf174e8792fdfc0) for improvement was taken as discussed on https://issues.apache.org/jira/browse/PROTON-1911. Could you close this PR? We can only do so via commit messages since this repo is a mirror with limited controls. Thanks, Robbie. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #15: PROTON-1916: Makes StringsBenchmark::encodeStringMe...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/15 The JIRA, PR, and commit say they are primarily changing the benchmark/test but most of the change has nothing to do with that. Even though the perf change is tiny and most things wont see it (due to them implementing their own WritableBuffer and not using the method updated at all) its really the main change here so things should reflect that. The benchmark updated was doing what was originally desired, but the updated version is more specifically targeting the String encoding side of things, so no issue with changing that. The commenting-out of the decode benchmark however seems like it should be removed though. The main updates change the bracing style of half the class, making it a lot harder to see what you actually changed than it needs to be, and then becoming inconsistent with the rest of the class. Please restore things to the existing format to minimise the diff. You have introduced a full new method that nothing apparently ever uses, since a non-array buffer isn't passed to it. I suppose there was a line untested before for this to be the case, but it was far simpler and there would now be a 50 line method in addition, so I think adding some basic testing is needed for that. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms issue #22: QPIDJMS-417 Reduce GC pressure while using BytesMessage
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/22 Can you elaborate on the benefits you measured here? I'd like to understand the extent to consider against the downside of exposing dep impl types throughout the code base. The existing bits worked the way it did at least a small part because BytesMessage specified thats where it gets its behaviours from. Are we sure ByteBuf streams behave the same? --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j pull request #18: NO-JIRA fix use of AtomicInteger and other v...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-proton-j/pull/18#discussion_r225223601
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/engine/impl/ssl/DefaultSslEngineFacade.java
---
@@ -36,7 +36,7 @@
/**
* Our testing has shown that application buffers need to be a bit
larger
* than that provided by {@link SSLSession#getApplicationBufferSize()}
otherwise
- * {@link Status#BUFFER_OVERFLOW} will result on {@link
SSLEngine#unwrap()}.
+ * {@link Status#BUFFER_OVERFLOW} will result on {@link
SSLEngine#unwrap)}.
--- End diff --
Seems to me like this change breaks the javadoc rather than fixing anything.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j pull request #18: NO-JIRA fix use of AtomicInteger and other v...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-proton-j/pull/18#discussion_r225224724
--- Diff:
examples/reactor/src/main/java/org/apache/qpid/proton/example/reactor/EchoInputStreamWrapper.java
---
@@ -31,11 +31,12 @@
public class EchoInputStreamWrapper extends Thread {
+private static final AtomicInteger idCounter = new AtomicInteger();
--- End diff --
I don't think this really needed fixed and would have just left it as is.
Its only an example, likely to count to 1. Typically having a static shared
between reactors would often be incorrect.
If it were to change the variable name would be of the wrong syntax.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j pull request #18: NO-JIRA fix use of AtomicInteger and other v...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-proton-j/pull/18#discussion_r225236948
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/TaskImpl.java ---
@@ -29,10 +29,12 @@
import org.apache.qpid.proton.reactor.Task;
public class TaskImpl implements Task, Comparable {
+
+private static final AtomicInteger count = new AtomicInteger();
--- End diff --
This seems like an actual bug, so it deserves a JIRA. Variable name syntax
is now wrong.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #19: PROTON-1958 Fix use of AtomicInteger for instance I...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/19 I'll merge this later, could perhaps do with a test. Note that the example change should really have been kept separate as it has nothing to do with the defect (and as noted before, wont really make any difference to the example since it will typically only have 1 instance) --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j pull request #18: NO-JIRA Minor code improvements (e.g. semico...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-proton-j/pull/18#discussion_r225562577
--- Diff:
examples/reactor/src/main/java/org/apache/qpid/proton/example/reactor/EchoInputStreamWrapper.java
---
@@ -31,11 +31,12 @@
public class EchoInputStreamWrapper extends Thread {
+private static final AtomicInteger idCounter = new AtomicInteger();
--- End diff --
I'd actually be more inclined to delete the example (since its crap) than
think about it any more than I have already hehe. Either way, the change should
not have been moved to the other PR since it isn't part of that issue.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #18: NO-JIRA Minor code improvements (e.g. semicolons an...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/18 If the changes aren't related enough to be in the same commit, they probably shouldn't be in the same PR to begin with :) (My view is that nothing but the most complex layered changes that are fully only understandable step by step should have multiple commits on the same PR.) --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #16: PROTON-1938: Fix error propagation in TransportImpl
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/16 As noted on the JIRA, an alternate change was made in 19bf22f7e9b88e73db4b195f674d1527dbd713f3. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j pull request #18: NO-JIRA Minor code improvements (e.g. semico...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-proton-j/pull/18#discussion_r231104760
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/message/impl/MessageImpl.java ---
@@ -49,11 +49,7 @@
}
}
-private static final ThreadLocal tlsCodec = new
ThreadLocal() {
- @Override protected EncoderDecoderPair initialValue() {
-return new EncoderDecoderPair();
- }
- };
+private static final ThreadLocal tlsCodec =
ThreadLocal.withInitial(EncoderDecoderPair::new);
--- End diff --
I spend about half of yesterday chasing down some sporadic test failures I
was seeing in the python test suite, that seemed like it must be from local
changes I had in that area. Instead, it eventually turned out to be this
change, as I also had this commit in my tree to rebase it. For whatever reason,
altering this seems to cause a behaviour shift that the Jython based tests
don't like (at least in my env), leading to sporadic failure to run any of the
tests. I'll be committing an updated version without this change.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #15: PROTON-1916: Makes StringsBenchmark::encodeStringMe...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/15 We don't get notified of updated pushes for the mirrors, so I didn't know you had modified this since my comment. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #19: PROTON-1958 Fix use of AtomicInteger for instance I...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/19 Can you please close out this PR? I went with a different approach, committed in 83609c4752fbdaa1ddfa032285e5caa09b61f480, of making each reactor timer maintain a counter for its tasks, rather than a shared counter across the JVM for all reactors. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-jms issue #25: [QPIDJMS-426] Update to proton-j 0.30.0
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/25 Sorry, I didn't notice you raised this. I had the change sitting ready since preparing 0.30.0 last week, awaiting the vote and then completing the announcement work. It never occurred to me anyone might have raised this. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #20: PROTON-1965 Optimize CompositeReadableBuffer::equal...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/20 The reset is a good catch. Its a noteworthy bug in its own right and so should be fixed separately, and also tested. I have done that now via PROTON-1966. Sorry for the hassle @franz1981 but can you please rebase your PR again. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #20: PROTON-1965 Optimize CompositeReadableBuffer::equal...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/20 It seems the github mirror is not up to date, and I've since prodded a re-sync and it still isn't up to date. I'll possibly need to ask infra about it after lunch. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #20: PROTON-1965 Optimize CompositeReadableBuffer::equal...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/20 Checked in with infra who looked into it and found the issue, mirror is now back up to date. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #20: PROTON-1965 Optimize CompositeReadableBuffer::equal...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/20 Can you unwind the 'revert' of my currentPos -> origPos variable name change from earlier? :) --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j pull request #20: PROTON-1965 Optimize CompositeReadableBuffer...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-proton-j/pull/20#discussion_r234714952
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/codec/CompositeReadableBuffer.java
---
@@ -834,22 +834,39 @@ public boolean equals(Object other) {
return false;
}
-ReadableBuffer buffer = (ReadableBuffer)other;
-if (this.remaining() != buffer.remaining()) {
+ReadableBuffer buffer = (ReadableBuffer) other;
+final int remaining = remaining();
+if (remaining != buffer.remaining()) {
return false;
}
+if (hasArray()) {
+return equals(currentArray, position, remaining, buffer);
--- End diff --
This bit is a little broken as it uses position without account for any
array offset in the current buffer. Its position in the buffer may not be the
same as its position within the array, e.g. if it were previously sliced from a
larger buffer. Calling equals with slices from two equal buffers with different
current positions will fail.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #20: PROTON-1965 Optimize CompositeReadableBuffer::equal...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/20 I've pushed an updated version of the change with a fix for the offset issue I noticed yesterday, which Tim added a test to cover. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
