From 2965fa4cc52f02a7ba397365f7298060fbc81552 Mon Sep 17 00:00:00 2001 From: kasemir Date: Tue, 12 Aug 2025 12:59:18 -0400 Subject: [PATCH 1/7] PVA: Introduce CMD_ACL_CHANGE message to server and client --- .../pva/client/AccessRightsChangeHandler.java | 46 ++++++++++ .../epics/pva/client/ClientTCPHandler.java | 1 + .../org/epics/pva/client/PVAClientMain.java | 12 +-- .../epics/pva/common/AccessRightsChange.java | 85 +++++++++++++++++++ .../java/org/epics/pva/common/PVAHeader.java | 3 + .../pva/server/CreateChannelHandler.java | 13 ++- .../java/org/epics/pva/server/ServerDemo.java | 5 +- 7 files changed, 156 insertions(+), 9 deletions(-) create mode 100644 core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java create mode 100644 core/pva/src/main/java/org/epics/pva/common/AccessRightsChange.java diff --git a/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java b/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java new file mode 100644 index 0000000000..4c56721a7c --- /dev/null +++ b/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java @@ -0,0 +1,46 @@ +/******************************************************************************* + * Copyright (c) 2025 Oak Ridge National Laboratory. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + ******************************************************************************/ +package org.epics.pva.client; + +import static org.epics.pva.PVASettings.logger; + +import java.nio.ByteBuffer; +import java.util.logging.Level; + +import org.epics.pva.common.AccessRightsChange; +import org.epics.pva.common.CommandHandler; +import org.epics.pva.common.PVAHeader; + +/** Handle a server's CMD_ACL_CHANGE message + * @author Kay Kasemir + */ +class AccessRightsChangeHandler implements CommandHandler +{ + @Override + public byte getCommand() + { + return PVAHeader.CMD_ACL_CHANGE; + } + + @Override + public void handleCommand(final ClientTCPHandler tcp, final ByteBuffer buffer) throws Exception + { + final AccessRightsChange acl = AccessRightsChange.decode(tcp.getRemoteAddress(), buffer.remaining(), buffer); + if (acl == null) + return; + final PVAChannel channel = tcp.getClient().getChannel(acl.cid); + if (channel == null) + { + logger.log(Level.WARNING, this + " received CMD_ACL_CHANGE for unknown channel ID " + acl.cid); + return; + } + + logger.log(Level.FINE, () -> "Received '" + channel.getName() + "' " + acl); + // TODO Update channel, ... + } +} diff --git a/core/pva/src/main/java/org/epics/pva/client/ClientTCPHandler.java b/core/pva/src/main/java/org/epics/pva/client/ClientTCPHandler.java index b94a9e29ce..10e1eb1c11 100644 --- a/core/pva/src/main/java/org/epics/pva/client/ClientTCPHandler.java +++ b/core/pva/src/main/java/org/epics/pva/client/ClientTCPHandler.java @@ -48,6 +48,7 @@ class ClientTCPHandler extends TCPHandler new ValidatedHandler(), new EchoHandler(), new CreateChannelHandler(), + new AccessRightsChangeHandler(), new DestroyChannelHandler(), new GetHandler(), new PutHandler(), diff --git a/core/pva/src/main/java/org/epics/pva/client/PVAClientMain.java b/core/pva/src/main/java/org/epics/pva/client/PVAClientMain.java index cebbede602..5244407ed9 100644 --- a/core/pva/src/main/java/org/epics/pva/client/PVAClientMain.java +++ b/core/pva/src/main/java/org/epics/pva/client/PVAClientMain.java @@ -97,8 +97,8 @@ private static void info(final List names) throws Exception final PVAChannel pv = iter.next(); if (pv.getState() == ClientChannelState.CONNECTED) { - PVASettings.logger.log(Level.INFO, "Server: " + pv.getTCP().getServerX509Name()); - PVASettings.logger.log(Level.INFO, "Client: " + pv.getTCP().getClientX509Name()); + PVASettings.logger.log(Level.INFO, "Server X509 Name: " + pv.getTCP().getServerX509Name()); + PVASettings.logger.log(Level.INFO, "Client X509 Name: " + pv.getTCP().getClientX509Name()); final PVAData data = pv.info(request).get(timeout_ms, TimeUnit.MILLISECONDS); System.out.println(pv.getName() + " = " + data.formatType()); @@ -142,8 +142,8 @@ private static void get(final List names) throws Exception final PVAChannel pv = iter.next(); if (pv.getState() == ClientChannelState.CONNECTED) { - PVASettings.logger.log(Level.INFO, "Server: " + pv.getTCP().getServerX509Name()); - PVASettings.logger.log(Level.INFO, "Client: " + pv.getTCP().getClientX509Name()); + PVASettings.logger.log(Level.INFO, "Server X509 Name: " + pv.getTCP().getServerX509Name()); + PVASettings.logger.log(Level.INFO, "Client X509 Name: " + pv.getTCP().getClientX509Name()); final PVAData data = pv.read(request).get(timeout_ms, TimeUnit.MILLISECONDS); System.out.println(pv.getName() + " = " + data); @@ -188,8 +188,8 @@ private static void monitor(final List names) throws Exception { try { - PVASettings.logger.log(Level.INFO, "Server: " + ch.getTCP().getServerX509Name()); - PVASettings.logger.log(Level.INFO, "Client: " + ch.getTCP().getClientX509Name()); + PVASettings.logger.log(Level.INFO, "Server X509 Name: " + ch.getTCP().getServerX509Name()); + PVASettings.logger.log(Level.INFO, "Client X509 Name: " + ch.getTCP().getClientX509Name()); ch.subscribe(request, listener); } catch (Exception ex) diff --git a/core/pva/src/main/java/org/epics/pva/common/AccessRightsChange.java b/core/pva/src/main/java/org/epics/pva/common/AccessRightsChange.java new file mode 100644 index 0000000000..4d77a907c8 --- /dev/null +++ b/core/pva/src/main/java/org/epics/pva/common/AccessRightsChange.java @@ -0,0 +1,85 @@ +/******************************************************************************* + * Copyright (c) 2025 Oak Ridge National Laboratory. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + ******************************************************************************/ +package org.epics.pva.common; + +import static org.epics.pva.PVASettings.logger; + +import java.net.InetSocketAddress; +import java.nio.ByteBuffer; +import java.util.logging.Level; + +/** Helper for CMD_ACL_CHANGE + * @author Kay Kasemir + */ +public class AccessRightsChange +{ + /** Size of payload */ + public static final int PAYLOAD_SIZE = Integer.BYTES + 1; + + /** Client channel ID */ + public int cid; + + /** Access rights bits */ + public byte access_rights; + + /** Access rights bit definitions */ + public static final byte READ_ONLY = 0x00, + WRITE_ACCESS = 0x01; + + private AccessRightsChange(final int cid, final byte access_rights) + { + this.cid = cid; + this.access_rights = access_rights; + } + + /** @return Do the access rights include write access? */ + public boolean mayWrite() + { + return (access_rights & WRITE_ACCESS) == WRITE_ACCESS; + } + + /** Encode access rights change + * @param buffer Buffer into which to encode + * @param cid Client channel ID + * @param b Access rights + */ + public static void encode(final ByteBuffer buffer, final int cid, final boolean writable) + { + PVAHeader.encodeMessageHeader(buffer, PVAHeader.FLAG_SERVER, PVAHeader.CMD_ACL_CHANGE, PAYLOAD_SIZE); + buffer.putInt(cid); + buffer.put(writable ? WRITE_ACCESS : READ_ONLY); + } + + /** Decode access rights change + * @param from Peer address + * @param payload Payload size + * @param buffer Buffer positioned on payload + * @return Decoded access rights change or null if not a valid + */ + public static AccessRightsChange decode(final InetSocketAddress from, + final int payload, final ByteBuffer buffer) + { + if (payload < PAYLOAD_SIZE) + { + logger.log(Level.WARNING, "PVA client " + from + " sent only " + payload + " bytes for access rights change"); + return null; + } + final AccessRightsChange acl = new AccessRightsChange(buffer.getInt(), buffer.get()); + logger.log(Level.FINER, () -> "PVA client " + from + " sent " + acl); + return acl; + } + + @Override + public String toString() + { + return String.format("CID %d access rights %s (0x%02X)", + cid, + mayWrite() ? "writeable" : "read-only", + access_rights); + } +} diff --git a/core/pva/src/main/java/org/epics/pva/common/PVAHeader.java b/core/pva/src/main/java/org/epics/pva/common/PVAHeader.java index 5f11e90375..7a868ff1be 100644 --- a/core/pva/src/main/java/org/epics/pva/common/PVAHeader.java +++ b/core/pva/src/main/java/org/epics/pva/common/PVAHeader.java @@ -72,6 +72,9 @@ public class PVAHeader /** Application command: Reply to search */ public static final byte CMD_SEARCH_RESPONSE = 0x04; + /** Application command: Access Control (List) Channel */ + public static final byte CMD_ACL_CHANGE = 0x06; + /** Application command: Create Channel */ public static final byte CMD_CREATE_CHANNEL = 0x07; diff --git a/core/pva/src/main/java/org/epics/pva/server/CreateChannelHandler.java b/core/pva/src/main/java/org/epics/pva/server/CreateChannelHandler.java index 3e2642be64..dad5479844 100644 --- a/core/pva/src/main/java/org/epics/pva/server/CreateChannelHandler.java +++ b/core/pva/src/main/java/org/epics/pva/server/CreateChannelHandler.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2019-2020 Oak Ridge National Laboratory. + * Copyright (c) 2019-2025 Oak Ridge National Laboratory. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -12,6 +12,7 @@ import java.nio.ByteBuffer; import java.util.logging.Level; +import org.epics.pva.common.AccessRightsChange; import org.epics.pva.common.CommandHandler; import org.epics.pva.common.PVAHeader; import org.epics.pva.data.PVAStatus; @@ -20,7 +21,6 @@ /** Handle response to client's CREATE CHANNEL command * @author Kay Kasemir */ -@SuppressWarnings("nls") class CreateChannelHandler implements CommandHandler { @Override @@ -55,6 +55,7 @@ private void sendChannelCreated(final ServerTCPHandler tcp, final ServerPV pv, i { tcp.submit((version, buffer) -> { + // Confirm channel creation logger.log(Level.FINE, () -> "Confirm channel creation " + pv + " [CID " + cid + "]"); PVAHeader.encodeMessageHeader(buffer, PVAHeader.FLAG_SERVER, @@ -66,6 +67,14 @@ private void sendChannelCreated(final ServerTCPHandler tcp, final ServerPV pv, i buffer.putInt(pv.getSID()); // status PVAStatus.StatusOK.encode(buffer); + + // Send initial access rights with the channel confirmation. + // The client may have read the channel creation confirmation + // and send for example a Get-Init before it processes the access rights info, + // but at least it's in the TCP pipeline + final boolean writable = pv.isWritable(); + logger.log(Level.FINE, () -> "Send ACL " + pv + " [CID " + cid + "]" + (writable ? " writable" : " read-only")); + AccessRightsChange.encode(buffer, cid, writable); }); } } diff --git a/core/pva/src/main/java/org/epics/pva/server/ServerDemo.java b/core/pva/src/main/java/org/epics/pva/server/ServerDemo.java index 5f9c43f6eb..2f4eaa3a7d 100644 --- a/core/pva/src/main/java/org/epics/pva/server/ServerDemo.java +++ b/core/pva/src/main/java/org/epics/pva/server/ServerDemo.java @@ -26,7 +26,6 @@ * * @author Kay Kasemir */ -@SuppressWarnings("nls") public class ServerDemo { public static void main(String[] args) throws Exception @@ -97,6 +96,10 @@ public static void main(String[] args) throws Exception if (! ex.getMessage().toLowerCase().contains("incompatible")) throw ex; } + + write_pv.close(); + pv2.close(); + pv1.close(); } } } From 26dec60edf3cad5f3c7a5898b0fe346d724fbd61 Mon Sep 17 00:00:00 2001 From: kasemir Date: Tue, 12 Aug 2025 14:22:02 -0400 Subject: [PATCH 2/7] PVA: Notify on changed access permissions --- .../main/java/org/phoebus/pv/pva/PVA_PV.java | 11 +++++-- .../pva/client/AccessRightsChangeHandler.java | 2 +- .../client/ClientAccessRightsListener.java | 27 +++++++++++++++++ .../pva/client/ClientChannelListener.java | 6 ++-- .../java/org/epics/pva/client/PVAChannel.java | 29 +++++++++++++++++-- .../java/org/epics/pva/client/PVAClient.java | 24 +++++++++++++-- 6 files changed, 87 insertions(+), 12 deletions(-) create mode 100644 core/pva/src/main/java/org/epics/pva/client/ClientAccessRightsListener.java diff --git a/core/pv-pva/src/main/java/org/phoebus/pv/pva/PVA_PV.java b/core/pv-pva/src/main/java/org/phoebus/pv/pva/PVA_PV.java index a34180c2b3..712c27cf26 100644 --- a/core/pv-pva/src/main/java/org/phoebus/pv/pva/PVA_PV.java +++ b/core/pv-pva/src/main/java/org/phoebus/pv/pva/PVA_PV.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2019-2022 Oak Ridge National Laboratory. + * Copyright (c) 2019-2025 Oak Ridge National Laboratory. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -44,7 +44,9 @@ public PVA_PV(final String name, final String base_name) throws Exception // Analyze base_name, determine channel and request name_helper = PVNameHelper.forName(base_name); logger.log(Level.FINE, () -> "PVA '" + base_name + "' -> " + name_helper); - channel = PVA_Context.getInstance().getClient().getChannel(name_helper.getChannel(), this::channelStateChanged); + channel = PVA_Context.getInstance().getClient().getChannel(name_helper.getChannel(), + this::channelStateChanged, + this::accessRightsChanged); } private void channelStateChanged(final PVAChannel channel, final ClientChannelState state) @@ -67,6 +69,11 @@ else if (! isDisconnected(super.read())) } } + private void accessRightsChanged(final PVAChannel channel, final boolean is_writable) + { + notifyListenersOfPermissions(! is_writable); + } + private void handleMonitor(final PVAChannel channel, final BitSet changes, final BitSet overruns, diff --git a/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java b/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java index 4c56721a7c..e0cd81542e 100644 --- a/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java +++ b/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java @@ -41,6 +41,6 @@ public void handleCommand(final ClientTCPHandler tcp, final ByteBuffer buffer) t } logger.log(Level.FINE, () -> "Received '" + channel.getName() + "' " + acl); - // TODO Update channel, ... + channel.updateAccessRights(acl.mayWrite()); } } diff --git a/core/pva/src/main/java/org/epics/pva/client/ClientAccessRightsListener.java b/core/pva/src/main/java/org/epics/pva/client/ClientAccessRightsListener.java new file mode 100644 index 0000000000..9c6546679d --- /dev/null +++ b/core/pva/src/main/java/org/epics/pva/client/ClientAccessRightsListener.java @@ -0,0 +1,27 @@ +/******************************************************************************* + * Copyright (c) 2025 Oak Ridge National Laboratory. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + ******************************************************************************/ +package org.epics.pva.client; + +/** Listener to a {@link PVAChannel} access rights + * @author Kay Kasemir + */ +@FunctionalInterface +public interface ClientAccessRightsListener +{ + /** Invoked when the channel access rights change + * + *

Will be called as soon as possible, i.e. within + * the thread that handles the network communication. + * + *

Client code must not block. + * + * @param channel Channel with updated permissions + * @param is_writable May we write to the channel? + */ + public void channelAccessRightsChanged(PVAChannel channel, boolean is_writable); +} diff --git a/core/pva/src/main/java/org/epics/pva/client/ClientChannelListener.java b/core/pva/src/main/java/org/epics/pva/client/ClientChannelListener.java index 6614b783c2..3bca80b887 100644 --- a/core/pva/src/main/java/org/epics/pva/client/ClientChannelListener.java +++ b/core/pva/src/main/java/org/epics/pva/client/ClientChannelListener.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2019 Oak Ridge National Laboratory. + * Copyright (c) 2019-2025 Oak Ridge National Laboratory. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,10 +7,10 @@ ******************************************************************************/ package org.epics.pva.client; -/** Listener to a {@link PVAChannel} - * +/** Listener to a {@link PVAChannel} state * @author Kay Kasemir */ +@FunctionalInterface public interface ClientChannelListener { /** Invoked when the channel state changes diff --git a/core/pva/src/main/java/org/epics/pva/client/PVAChannel.java b/core/pva/src/main/java/org/epics/pva/client/PVAChannel.java index 610ab5fc5b..e79bae9ca0 100644 --- a/core/pva/src/main/java/org/epics/pva/client/PVAChannel.java +++ b/core/pva/src/main/java/org/epics/pva/client/PVAChannel.java @@ -12,6 +12,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; @@ -38,13 +39,13 @@ * *

When no longer in use, the channel should be {@link #close()}d. * - * + * * Note that several methods return a CompletableFuture. * This has been done because at this time the Futures used internally are indeed CompletableFutures * and this type offers an extensive API for composition and chaining of futures. * But note that user code must never call 'complete(..)' nor 'completeExceptionally()' * on the provided CompletableFutures. - * + * * @author Kay Kasemir */ @SuppressWarnings("nls") @@ -63,7 +64,11 @@ public class PVAChannel extends SearchRequest.Channel implements AutoCloseable private final PVAClient client; private final ClientChannelListener listener; + private final ClientAccessRightsListener access_rights_listener; private volatile int sid = -1; + // For compatibility with earlier implementation, + // assume channels are writable until access_rights_listener tells otherwise + private AtomicBoolean is_writable = new AtomicBoolean(true); /** State * @@ -79,11 +84,14 @@ public class PVAChannel extends SearchRequest.Channel implements AutoCloseable private final CopyOnWriteArrayList subscriptions = new CopyOnWriteArrayList<>(); - PVAChannel(final PVAClient client, final String name, final ClientChannelListener listener) + PVAChannel(final PVAClient client, final String name, + final ClientChannelListener listener, + final ClientAccessRightsListener access_rights_listener) { super(CID_Provider.incrementAndGet(), name); this.client = client; this.listener = listener; + this.access_rights_listener = access_rights_listener; } PVAClient getClient() @@ -121,6 +129,21 @@ public boolean isConnected() return getState() == ClientChannelState.CONNECTED; } + /** @return true if channel has write permissions */ + public boolean isWritable() + { + return is_writable.get(); + } + + /** Called by AccessRightsChangeHandler + * @param may_write Is channel writable? + */ + void updateAccessRights(final boolean may_write) + { + if (is_writable.getAndSet(may_write) != may_write) + access_rights_listener.channelAccessRightsChanged(this, may_write); + } + /** Wait for channel to connect * @return {@link CompletableFuture} to await connection. * true on success, diff --git a/core/pva/src/main/java/org/epics/pva/client/PVAClient.java b/core/pva/src/main/java/org/epics/pva/client/PVAClient.java index a51a711342..517528bfbc 100644 --- a/core/pva/src/main/java/org/epics/pva/client/PVAClient.java +++ b/core/pva/src/main/java/org/epics/pva/client/PVAClient.java @@ -40,12 +40,14 @@ * * @author Kay Kasemir */ -@SuppressWarnings("nls") public class PVAClient implements AutoCloseable { - /** Default channel listener logs state changes */ + /** Default channel state listener logs state changes */ private static final ClientChannelListener DEFAULT_CHANNEL_LISTENER = (ch, state) -> logger.log(Level.INFO, ch.toString()); + /** Default channel access rights listener does nothing */ + private static final ClientAccessRightsListener DEFAULT_ACCESS_RIGHTS_LISTENER = (ch, write) -> {}; + private final ClientUDPHandler udp; private final BeaconTracker beacons = new BeaconTracker(); @@ -166,7 +168,23 @@ public PVAChannel getChannel(final String channel_name) */ public PVAChannel getChannel(final String channel_name, final ClientChannelListener listener) { - final PVAChannel channel = new PVAChannel(this, channel_name, listener); + return getChannel(channel_name, listener, DEFAULT_ACCESS_RIGHTS_LISTENER); + } + + /** Create channel by name + * + *

Starts search. + * + * @param channel_name PVA channel name + * @param state_listener {@link ClientChannelListener} that will be invoked with connection state updates + * @param access_rights_listener {@link ClientAccessRightsListener} that will be invoked with access rights updates + * @return {@link PVAChannel} + */ + public PVAChannel getChannel(final String channel_name, + final ClientChannelListener state_listener, + final ClientAccessRightsListener access_rights_listener) + { + final PVAChannel channel = new PVAChannel(this, channel_name, state_listener, access_rights_listener); channels_by_id.putIfAbsent(channel.getCID(), channel); // Register with search From 7db1af9cf3856b15c65b858e8621d1263dec5595 Mon Sep 17 00:00:00 2001 From: kasemir Date: Tue, 12 Aug 2025 14:40:34 -0400 Subject: [PATCH 3/7] PVA: More consistent "is writable" --- .../java/org/epics/pva/client/AccessRightsChangeHandler.java | 2 +- .../main/java/org/epics/pva/common/AccessRightsChange.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java b/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java index e0cd81542e..f8a6798635 100644 --- a/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java +++ b/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java @@ -41,6 +41,6 @@ public void handleCommand(final ClientTCPHandler tcp, final ByteBuffer buffer) t } logger.log(Level.FINE, () -> "Received '" + channel.getName() + "' " + acl); - channel.updateAccessRights(acl.mayWrite()); + channel.updateAccessRights(acl.isWritable()); } } diff --git a/core/pva/src/main/java/org/epics/pva/common/AccessRightsChange.java b/core/pva/src/main/java/org/epics/pva/common/AccessRightsChange.java index 4d77a907c8..ef3eee07f1 100644 --- a/core/pva/src/main/java/org/epics/pva/common/AccessRightsChange.java +++ b/core/pva/src/main/java/org/epics/pva/common/AccessRightsChange.java @@ -38,7 +38,7 @@ private AccessRightsChange(final int cid, final byte access_rights) } /** @return Do the access rights include write access? */ - public boolean mayWrite() + public boolean isWritable() { return (access_rights & WRITE_ACCESS) == WRITE_ACCESS; } @@ -79,7 +79,7 @@ public String toString() { return String.format("CID %d access rights %s (0x%02X)", cid, - mayWrite() ? "writeable" : "read-only", + isWritable() ? "writeable" : "read-only", access_rights); } } From 09ddcb4399a1377e38741715b7095646a64177eb Mon Sep 17 00:00:00 2001 From: kasemir Date: Thu, 14 Aug 2025 14:50:42 -0400 Subject: [PATCH 4/7] PVA: Send initial access rights before channel create confirmation --- .../org/epics/pva/server/CreateChannelHandler.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/core/pva/src/main/java/org/epics/pva/server/CreateChannelHandler.java b/core/pva/src/main/java/org/epics/pva/server/CreateChannelHandler.java index dad5479844..f1ccdd9f52 100644 --- a/core/pva/src/main/java/org/epics/pva/server/CreateChannelHandler.java +++ b/core/pva/src/main/java/org/epics/pva/server/CreateChannelHandler.java @@ -55,6 +55,12 @@ private void sendChannelCreated(final ServerTCPHandler tcp, final ServerPV pv, i { tcp.submit((version, buffer) -> { + // Send initial access rights with (before) the channel confirmation, + // so client knows permissions when channel is confirmed + final boolean writable = pv.isWritable(); + logger.log(Level.FINE, () -> "Send ACL " + pv + " [CID " + cid + "]" + (writable ? " writable" : " read-only")); + AccessRightsChange.encode(buffer, cid, writable); + // Confirm channel creation logger.log(Level.FINE, () -> "Confirm channel creation " + pv + " [CID " + cid + "]"); PVAHeader.encodeMessageHeader(buffer, @@ -67,14 +73,6 @@ private void sendChannelCreated(final ServerTCPHandler tcp, final ServerPV pv, i buffer.putInt(pv.getSID()); // status PVAStatus.StatusOK.encode(buffer); - - // Send initial access rights with the channel confirmation. - // The client may have read the channel creation confirmation - // and send for example a Get-Init before it processes the access rights info, - // but at least it's in the TCP pipeline - final boolean writable = pv.isWritable(); - logger.log(Level.FINE, () -> "Send ACL " + pv + " [CID " + cid + "]" + (writable ? " writable" : " read-only")); - AccessRightsChange.encode(buffer, cid, writable); }); } } From d96c6a8e1351c9f6d3cf66c490d107e4a3173cd0 Mon Sep 17 00:00:00 2001 From: kasemir Date: Fri, 15 Aug 2025 09:49:49 -0400 Subject: [PATCH 5/7] PVA: Mention ACL bits for PUT-GET and RPC --- .../pva/client/AccessRightsChangeHandler.java | 2 +- .../epics/pva/common/AccessRightsChange.java | 26 +++++++++++++------ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java b/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java index f8a6798635..f5e6e7f8fc 100644 --- a/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java +++ b/core/pva/src/main/java/org/epics/pva/client/AccessRightsChangeHandler.java @@ -41,6 +41,6 @@ public void handleCommand(final ClientTCPHandler tcp, final ByteBuffer buffer) t } logger.log(Level.FINE, () -> "Received '" + channel.getName() + "' " + acl); - channel.updateAccessRights(acl.isWritable()); + channel.updateAccessRights(acl.havePUTaccess()); } } diff --git a/core/pva/src/main/java/org/epics/pva/common/AccessRightsChange.java b/core/pva/src/main/java/org/epics/pva/common/AccessRightsChange.java index ef3eee07f1..699956dd9b 100644 --- a/core/pva/src/main/java/org/epics/pva/common/AccessRightsChange.java +++ b/core/pva/src/main/java/org/epics/pva/common/AccessRightsChange.java @@ -27,9 +27,17 @@ public class AccessRightsChange /** Access rights bits */ public byte access_rights; - /** Access rights bit definitions */ - public static final byte READ_ONLY = 0x00, - WRITE_ACCESS = 0x01; + /** Access rights bit definitions + * + * May client write (PUT), + * perform a write with read-back (PUT-GET) + * call a remote procedure (RPC)? + */ + public static final byte READ_ONLY = 0x00, + PUT_ACCESS = (1 << 0), + PUT_GET_ACCESS = (1 << 1), + RPC_ACCESS = (1 << 2); + private AccessRightsChange(final int cid, final byte access_rights) { @@ -37,10 +45,12 @@ private AccessRightsChange(final int cid, final byte access_rights) this.access_rights = access_rights; } - /** @return Do the access rights include write access? */ - public boolean isWritable() + // TODO Add API for PUT_GET and RPC once PVXS has a reference implementation + + /** @return Do the access rights include write ('PUT') access? */ + public boolean havePUTaccess() { - return (access_rights & WRITE_ACCESS) == WRITE_ACCESS; + return (access_rights & PUT_ACCESS) == PUT_ACCESS; } /** Encode access rights change @@ -52,7 +62,7 @@ public static void encode(final ByteBuffer buffer, final int cid, final boolean { PVAHeader.encodeMessageHeader(buffer, PVAHeader.FLAG_SERVER, PVAHeader.CMD_ACL_CHANGE, PAYLOAD_SIZE); buffer.putInt(cid); - buffer.put(writable ? WRITE_ACCESS : READ_ONLY); + buffer.put(writable ? PUT_ACCESS : READ_ONLY); } /** Decode access rights change @@ -79,7 +89,7 @@ public String toString() { return String.format("CID %d access rights %s (0x%02X)", cid, - isWritable() ? "writeable" : "read-only", + havePUTaccess() ? "writeable" : "read-only", access_rights); } } From 9b005896a81d6c7364e7ad4537c097bf7095427c Mon Sep 17 00:00:00 2001 From: kasemir Date: Thu, 28 Aug 2025 15:23:11 -0400 Subject: [PATCH 6/7] PVA client: Fix ConcurrentModificationException in search `ChannelSearch.runSearches` used to re-use one `to_search` array list. By the time the submitted searches encode the channels from that list, it might get cleared and re-populated. Need to create a new one in each `runSearches` call. At same time removed some unnecessary casts. --- .../java/org/epics/pva/client/ChannelSearch.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/core/pva/src/main/java/org/epics/pva/client/ChannelSearch.java b/core/pva/src/main/java/org/epics/pva/client/ChannelSearch.java index 35ee2ba03b..59e1dfc6a5 100644 --- a/core/pva/src/main/java/org/epics/pva/client/ChannelSearch.java +++ b/core/pva/src/main/java/org/epics/pva/client/ChannelSearch.java @@ -342,9 +342,6 @@ public synchronized void boost() } } - /** List of channels to search, re-used within runSearches */ - private final ArrayList to_search = new ArrayList<>(); - /** Invoked by timer: Check searched channels for the next one to handle */ @SuppressWarnings("unchecked") private void runSearches() @@ -352,7 +349,7 @@ private void runSearches() // Determine current search bucket final int current = current_search_bucket.getAndUpdate(i -> (i + 1) % search_buckets.size()); // Collect channels to be searched while sync'ed - to_search.clear(); + final ArrayList to_search = new ArrayList<>(); synchronized (this) { final Set bucket = search_buckets.get(current); @@ -406,7 +403,7 @@ private void runSearches() int count = 0; while (start + count < to_search.size() && count < Short.MAX_VALUE-1) { - final PVAChannel channel = to_search.get(start + count); + final SearchRequest.Channel channel = to_search.get(start + count); int size = 4 + PVAString.getEncodedSize(channel.getName()); if (payload + size < MAX_SEARCH_PAYLOAD) { @@ -428,9 +425,9 @@ else if (count == 0) if (count == 0) break; - final List batch = to_search.subList(start, start + count); - // PVAChannel extends SearchRequest.Channel, so use List as Collection - search((Collection) (List)batch); + // Submit one batch from 'to_search' + final List batch = to_search.subList(start, start + count); + search(batch); start += count; } } From 28c9df6dcee43e9b8c7c1b8215c9c359b7ff10b0 Mon Sep 17 00:00:00 2001 From: kasemir Date: Thu, 28 Aug 2025 15:59:18 -0400 Subject: [PATCH 7/7] PVA: "unchecked" cast has been removed --- core/pva/src/main/java/org/epics/pva/client/ChannelSearch.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/pva/src/main/java/org/epics/pva/client/ChannelSearch.java b/core/pva/src/main/java/org/epics/pva/client/ChannelSearch.java index 59e1dfc6a5..7d434e69ce 100644 --- a/core/pva/src/main/java/org/epics/pva/client/ChannelSearch.java +++ b/core/pva/src/main/java/org/epics/pva/client/ChannelSearch.java @@ -343,7 +343,6 @@ public synchronized void boost() } /** Invoked by timer: Check searched channels for the next one to handle */ - @SuppressWarnings("unchecked") private void runSearches() { // Determine current search bucket