[
https://issues.apache.org/jira/browse/ARTEMIS-4294?focusedWorklogId=863078&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-863078
]
ASF GitHub Bot logged work on ARTEMIS-4294:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 31/May/23 15:37
Start Date: 31/May/23 15:37
Worklog Time Spent: 10m
Work Description: tabish121 commented on code in PR #4493:
URL: https://github.com/apache/activemq-artemis/pull/4493#discussion_r1211906696
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/protocol/websocket/WebSocketFrameEncoderType.java:
##########
@@ -0,0 +1,33 @@
+/**
+ * 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.activemq.artemis.core.server.protocol.websocket;
+
+public enum WebSocketFrameEncoderType {
+ BINARY, TEXT;
+
+ public static WebSocketFrameEncoderType valueOfType(String type) {
+ switch (type) {
+ case "binary":
+ return BINARY;
+ case "text":
+ return TEXT;
+ default:
+ return null;
Review Comment:
Instead of returning null here why not just throw an exception that
indicates that this is an invalid argument here to avoid any chance of NPE later
##########
artemis-server/src/test/java/org/apache/activemq/artemis/core/server/protocol/websocket/WebSocketFrameEncoderTest.java:
##########
@@ -60,11 +61,22 @@ public class WebSocketFrameEncoderTest {
@Before
public void setUp() throws Exception {
- spy = spy(new WebSocketFrameEncoder(maxFramePayloadLength));
+ binarySpy = spy(new WebSocketFrameEncoder(maxFramePayloadLength,
WebSocketFrameEncoderType.BINARY));
+ textSpy = spy(new WebSocketFrameEncoder(maxFramePayloadLength,
WebSocketFrameEncoderType.TEXT));
}
@Test
- public void testWriteNonByteBuf() throws Exception {
+ public void testWriteNonByteBufBinary() throws Exception {
+ testWriteNonByteBuf(binarySpy);
+ }
+
+
Review Comment:
Extra newline
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/ProtocolHandler.java:
##########
@@ -158,7 +160,12 @@ public void channelRead(ChannelHandlerContext ctx, Object
msg) throws Exception
}
stompMaxFramePayloadLength = stompMaxFramePayloadLength != -1 ?
stompMaxFramePayloadLength :
TransportConstants.DEFAULT_WEB_SOCKET_MAX_FRAME_PAYLOAD_LENGTH;
int webSocketMaxFramePayloadLength =
ConfigurationHelper.getIntProperty(TransportConstants.WEB_SOCKET_MAX_FRAME_PAYLOAD_LENGTH,
-1, nettyAcceptor.getConfiguration());
- ctx.pipeline().addLast("websocket-handler", new
WebSocketServerHandler(websocketSubprotocolIds, webSocketMaxFramePayloadLength
!= -1 ? webSocketMaxFramePayloadLength : stompMaxFramePayloadLength));
+ String encoderConfigType =
ConfigurationHelper.getStringProperty(TransportConstants.WEB_SOCKET_ENCODER_TYPE,
TransportConstants.DEFAULT_WEB_SOCKET_ENCODER_TYPE,
nettyAcceptor.getConfiguration());
+ WebSocketFrameEncoderType encoderType =
WebSocketFrameEncoderType.valueOfType(encoderConfigType);
+ if (encoderType == null) {
Review Comment:
As noted below I'd just have the 'valuefType' method validate and throw
instead of needing to check here
Issue Time Tracking
-------------------
Worklog Id: (was: 863078)
Time Spent: 0.5h (was: 20m)
> Support text WebSocket encoding
> -------------------------------
>
> Key: ARTEMIS-4294
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4294
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Reporter: Justin Bertram
> Assignee: Justin Bertram
> Priority: Minor
> Fix For: 2.29.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> WebSocket frames can be encoded as either [binary or
> text|https://datatracker.ietf.org/doc/html/rfc6455#section-11.8]. Currently
> the broker always encodes them as binary. The broker should support an
> acceptor URL parameter to change the encoding to text.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
