You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2020/11/19 10:44:52 UTC
[GitHub] [qpid-broker-j] Dedeepya-T opened a new pull request #71: QPID-8482:[Broker-J]Derived a UUID for session name based on attach name
Dedeepya-T opened a new pull request #71:
URL: https://github.com/apache/qpid-broker-j/pull/71
Derived a UUID for session name based on attach name and it can be used to log the session name.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-broker-j] Dedeepya-T commented on a change in pull request #71: QPID-8482:[Broker-J]Derived a UUID for session name based on attach name
Posted by GitBox <gi...@apache.org>.
Dedeepya-T commented on a change in pull request #71:
URL: https://github.com/apache/qpid-broker-j/pull/71#discussion_r529243728
##########
File path: broker-core/src/main/java/org/apache/qpid/server/model/Session.java
##########
@@ -51,6 +51,9 @@
@DerivedAttribute
int getChannelId();
+ @DerivedAttribute
+ String getPeerSessionName();
Review comment:
Moved it to the 0-10 session interface
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-broker-j] Dedeepya-T commented on a change in pull request #71: QPID-8482:[Broker-J]Derived a UUID for session name based on attach name
Posted by GitBox <gi...@apache.org>.
Dedeepya-T commented on a change in pull request #71:
URL: https://github.com/apache/qpid-broker-j/pull/71#discussion_r529243472
##########
File path: broker-core/src/test/java/org/apache/qpid/server/session/AbstractAMQPSessionTest.java
##########
@@ -136,7 +136,7 @@ public void testRegisterTransactedMessageReceived()
protected MockAMQPSession(final Connection parent, final int sessionId)
{
- super(parent, sessionId);
+ super(parent, sessionId,"mockAMQPSession");
Review comment:
Done!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-broker-j] alex-rufous commented on a change in pull request #71: QPID-8482:[Broker-J]Derived a UUID for session name based on attach name
Posted by GitBox <gi...@apache.org>.
alex-rufous commented on a change in pull request #71:
URL: https://github.com/apache/qpid-broker-j/pull/71#discussion_r528711621
##########
File path: broker-core/src/main/java/org/apache/qpid/server/session/AMQPSession.java
##########
@@ -42,6 +42,9 @@
@Override
int getChannelId();
+ @Override
Review comment:
There is no need for duplication. Thought, after making this attribute 0-10 specific, it needs to be removed from AQMPSession
##########
File path: broker-core/src/main/java/org/apache/qpid/server/model/Session.java
##########
@@ -51,6 +51,9 @@
@DerivedAttribute
int getChannelId();
+ @DerivedAttribute
+ String getPeerSessionName();
Review comment:
Let's move this derived attribute into 0-10 session as it is 0-10 specific attribute
##########
File path: broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnectionDelegate.java
##########
@@ -427,6 +432,54 @@ public void sessionAttach(final ServerConnection serverConnection, final Session
}
}
+ private String getPeerSessionName(final byte[] attachName)
+ {
+ String peerSessionName = "";
+ try
+ {
+ peerSessionName = UUID.nameUUIDFromBytes(attachName).toString();
+ }
+ catch (Exception e)
+ {
+ peerSessionName = getPeerSessionNameUsingSha(attachName);
+ }
+ return peerSessionName;
+ }
+
+ private String getPeerSessionNameUsingSha(final byte[] attachName)
+ {
+ final String peerSessionName;
+ final MessageDigest hasher;
+ try
+ {
+ hasher = MessageDigest.getInstance(MESSAGE_DIGEST_SHA256);
+ // Split the hash into two parts: MSB and LSB
+ long msb = toNumber(attachName, 0, 8);
+ long lsb = toNumber(attachName, 8, 16);
+
+ // Apply version and variant bits (required for RFC-4122 compliance)
+ msb = (msb & 0xffffffffffff0fffL) | (5 & 0x0f) << 12;
+ lsb = (lsb & 0x3fffffffffffffffL) | 0x8000000000000000L;
+
+ peerSessionName = new UUID(msb, lsb).toString();
Review comment:
Let's return Base64 representation of the digest bytes. I think that giving UUID might create an impression that the UUID was set on the client side whilst in fact it wasn't.
##########
File path: broker-core/src/test/java/org/apache/qpid/server/session/AbstractAMQPSessionTest.java
##########
@@ -136,7 +136,7 @@ public void testRegisterTransactedMessageReceived()
protected MockAMQPSession(final Connection parent, final int sessionId)
{
- super(parent, sessionId);
+ super(parent, sessionId,"mockAMQPSession");
Review comment:
space is missing between sessionId and "mockAMQPSession"
##########
File path: broker-core/src/main/java/org/apache/qpid/server/session/AbstractAMQPSession.java
##########
@@ -152,6 +155,11 @@ public int getChannelId()
return _sessionId;
}
+ @Override
+ public String getPeerSessionName(){
Review comment:
Bracket is supposed to be on the next line. Though, after making peerSessionName 0-10 specific, the changes need to be removed
##########
File path: broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnectionDelegate.java
##########
@@ -409,8 +413,9 @@ public void sessionAttach(final ServerConnection serverConnection, final Session
final ServerSession serverSession =
new ServerSession(serverConnection, serverSessionDelegate, new Binary(atc.getName()), 0);
+ String peerSessionName = getPeerSessionName(serverSession.getName().getBytes());
Review comment:
Variable 'peerSessionName' can be in-lined
##########
File path: broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnectionDelegate.java
##########
@@ -427,6 +432,54 @@ public void sessionAttach(final ServerConnection serverConnection, final Session
}
}
+ private String getPeerSessionName(final byte[] attachName)
+ {
+ String peerSessionName = "";
+ try
+ {
+ peerSessionName = UUID.nameUUIDFromBytes(attachName).toString();
Review comment:
The client can set UUID string bytes as session name. We need to capture original UUID as set on the client. Thus, we can do that as in the code below
`try
{
return UUID.fromString(new String(attachName, UTF_8))
}
catch(RuntimeException e)
{
//capture UUID#fromString and new String() exceptions
return getPeerSessionNameUsingSha(attachName);
}`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-broker-j] Dedeepya-T commented on a change in pull request #71: QPID-8482:[Broker-J]Derived a UUID for session name based on attach name
Posted by GitBox <gi...@apache.org>.
Dedeepya-T commented on a change in pull request #71:
URL: https://github.com/apache/qpid-broker-j/pull/71#discussion_r529296446
##########
File path: broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnectionDelegate.java
##########
@@ -427,6 +432,54 @@ public void sessionAttach(final ServerConnection serverConnection, final Session
}
}
+ private String getPeerSessionName(final byte[] attachName)
+ {
+ String peerSessionName = "";
+ try
+ {
+ peerSessionName = UUID.nameUUIDFromBytes(attachName).toString();
+ }
+ catch (Exception e)
+ {
+ peerSessionName = getPeerSessionNameUsingSha(attachName);
+ }
+ return peerSessionName;
+ }
+
+ private String getPeerSessionNameUsingSha(final byte[] attachName)
+ {
+ final String peerSessionName;
+ final MessageDigest hasher;
+ try
+ {
+ hasher = MessageDigest.getInstance(MESSAGE_DIGEST_SHA256);
+ // Split the hash into two parts: MSB and LSB
+ long msb = toNumber(attachName, 0, 8);
+ long lsb = toNumber(attachName, 8, 16);
+
+ // Apply version and variant bits (required for RFC-4122 compliance)
+ msb = (msb & 0xffffffffffff0fffL) | (5 & 0x0f) << 12;
+ lsb = (lsb & 0x3fffffffffffffffL) | 0x8000000000000000L;
+
+ peerSessionName = new UUID(msb, lsb).toString();
Review comment:
Done!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-broker-j] Dedeepya-T commented on a change in pull request #71: QPID-8482:[Broker-J]Derived a UUID for session name based on attach name
Posted by GitBox <gi...@apache.org>.
Dedeepya-T commented on a change in pull request #71:
URL: https://github.com/apache/qpid-broker-j/pull/71#discussion_r529244724
##########
File path: broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnectionDelegate.java
##########
@@ -409,8 +413,9 @@ public void sessionAttach(final ServerConnection serverConnection, final Session
final ServerSession serverSession =
new ServerSession(serverConnection, serverSessionDelegate, new Binary(atc.getName()), 0);
+ String peerSessionName = getPeerSessionName(serverSession.getName().getBytes());
Review comment:
Inlined!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-broker-j] Dedeepya-T commented on a change in pull request #71: QPID-8482:[Broker-J]Derived a UUID for session name based on attach name
Posted by GitBox <gi...@apache.org>.
Dedeepya-T commented on a change in pull request #71:
URL: https://github.com/apache/qpid-broker-j/pull/71#discussion_r529243567
##########
File path: broker-core/src/main/java/org/apache/qpid/server/session/AMQPSession.java
##########
@@ -42,6 +42,9 @@
@Override
int getChannelId();
+ @Override
Review comment:
Removed!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-broker-j] asfgit closed pull request #71: QPID-8482:[Broker-J]Derived a UUID for session name based on attach name
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #71:
URL: https://github.com/apache/qpid-broker-j/pull/71
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-broker-j] Dedeepya-T commented on a change in pull request #71: QPID-8482:[Broker-J]Derived a UUID for session name based on attach name
Posted by GitBox <gi...@apache.org>.
Dedeepya-T commented on a change in pull request #71:
URL: https://github.com/apache/qpid-broker-j/pull/71#discussion_r529244191
##########
File path: broker-core/src/main/java/org/apache/qpid/server/session/AbstractAMQPSession.java
##########
@@ -152,6 +155,11 @@ public int getChannelId()
return _sessionId;
}
+ @Override
+ public String getPeerSessionName(){
Review comment:
Removed the method!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-broker-j] Dedeepya-T commented on a change in pull request #71: QPID-8482:[Broker-J]Derived a UUID for session name based on attach name
Posted by GitBox <gi...@apache.org>.
Dedeepya-T commented on a change in pull request #71:
URL: https://github.com/apache/qpid-broker-j/pull/71#discussion_r529246624
##########
File path: broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnectionDelegate.java
##########
@@ -427,6 +432,54 @@ public void sessionAttach(final ServerConnection serverConnection, final Session
}
}
+ private String getPeerSessionName(final byte[] attachName)
+ {
+ String peerSessionName = "";
+ try
+ {
+ peerSessionName = UUID.nameUUIDFromBytes(attachName).toString();
Review comment:
Done!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org