Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None

      Description

      Bunch of changes for CloseReason class

      • Add final modifier for fields
      • UTF-8 encoded bytes for reasonPhrase cannot be greater than 123 bytes. Throw IllegalArgumentException in that case.
      • enum constants for status codes need be documented. Otherwise, javadoc would be empty

      Looks like cannot attach the patch. Will send by email.

      If a close frame doesn't include status code, the Endpoint#onClose() is invoked with a CloseReason. What would be the value of CloseReason#getCloseCode() return in that case ? Would it be 1005 ? Good to include that in the spec.

        Activity

        jitu created issue -
        jitu made changes -
        Field Original Value New Value
        Description Bunch of changes for CloseReason class
        * Add final modifier for fields
        * UTF-8 encoded bytes for reasonPhrase cannot be greater than 123 bytes
        * enum constants for status codes need be documented. Otherwise, javadoc would be empty

        Attaching a patch

         
        Bunch of changes for CloseReason class
        * Add final modifier for fields
        * UTF-8 encoded bytes for reasonPhrase cannot be greater than 123 bytes
        * enum constants for status codes need be documented. Otherwise, javadoc would be empty

        Attaching a patch (looks like cannot attach a file)

         
        jitu made changes -
        Comment [ Index: src/main/java/javax/websocket/CloseReason.java
        ===================================================================
        --- src/main/java/javax/websocket/CloseReason.java (revision 105)
        +++ src/main/java/javax/websocket/CloseReason.java (working copy)
        @@ -1,7 +1,7 @@
         /*
          * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
          *
        - * Copyright (c) 2012 Oracle and/or its affiliates. All rights reserved.
        + * Copyright (c) 2013 Oracle and/or its affiliates. All rights reserved.
          *
          * The contents of this file are subject to the terms of either the GNU
          * General Public License Version 2 only ("GPL") or the Common Development
        @@ -39,31 +39,53 @@
          */
         package javax.websocket;
         
        +import java.nio.charset.StandardCharsets;
        +
         /**
        - * A class encapsulating the reason why a web socket has been closed, or why it is being asked to
        - * close. Note the acceptable uses of codes and reason phrase defined in FRC 6455.
        + * A class encapsulating the reason why a web socket has been closed, or why
        + * it is being asked to close. Note the acceptable uses of codes and reason
        + * phrase defined in RFC 6455.
          *
          * @author dannycoward
          * @since DRAFT 001
          */
         public class CloseReason {
         
        - private CloseReason.CloseCode closeCode;
        - private String reasonPhrase;
        + private final CloseReason.CloseCode closeCode;
        + private final String reasonPhrase;
         
             /**
              * Creates a reason for closing a web socket connection with the given
              * code and reason phrase.
              *
              * @param closeCode the close code
        - * @param reasonPhrase the reason phrase
        + * @param reasonPhrase the reason phrase, can be empty string but cannot
        + * be null.
        + * @throws NullPointerException if reasonPhrase is null
        + * @throws IllegalArgumentException if reasonPhrase string is long (UTF-8
        + * encoded reasonPhrase is greater than 123 bytes)
              */
             public CloseReason(CloseReason.CloseCode closeCode, String reasonPhrase) {
                 this.closeCode = closeCode;
                 this.reasonPhrase = reasonPhrase;
        + byte[] reasonBytes = reasonPhrase.getBytes(StandardCharsets.UTF_8);
        + if (reasonBytes.length > 123) {
        + throw new IllegalArgumentException(
        + "Reason phrase's encoded data is more than 123 bytes.");
        + }
             }
         
             /**
        + * Creates a reason for closing a web socket connection with the given
        + * code and empty reason phrase.
        + *
        + * @param closeCode the close code
        + */
        + public CloseReason(CloseReason.CloseCode closeCode) {
        + this(closeCode, "");
        + }
        +
        + /**
              * The Close code associated with this CloseReason.
              *
              * @return the close code.
        @@ -92,7 +114,8 @@
              */
             public interface CloseCode {
                 /**
        - * Returns the code number, for example the integer '1000' for normal closure.
        + * Returns the code number, for example the integer '1000' for normal
        + * closure.
                  *
                  * @return the code number
                  */
        @@ -104,39 +127,104 @@
              * are defined in the specification.
              */
             public enum CloseCodes implements CloseReason.CloseCode {
        - /* 1000 */
        + /**
        + * Indicates a normal closure, meaning that the purpose for
        + * which the connection was established has been fulfilled.
        + */
                 NORMAL_CLOSURE(1000),
        - /* 1001 */
        +
        + /**
        + * Indicates that an endpoint is "going away", such as a server
        + * going down or a browser having navigated away from a page.
        + */
                 GOING_AWAY(1001),
        - /* 1002 */
        +
        + /**
        + * Indicates that an endpoint is terminating the connection due
        + * to a protocol error
        + */
                 PROTOCOL_ERROR(1002),
        - /* 1003 */
        +
        + /**
        + * Indicates that an endpoint is terminating the connection
        + * because it has received a type of data it cannot accept (e.g., an
        + * endpoint that understands only text data MAY send this if it
        + * receives a binary message).
        + */
                 CANNOT_ACCEPT(1003),
        - /* 1004 */
        - RESERVED(1004
        - /* 1005 */),
        +
        + /**
        + * Reserved. The specific meaning might be defined in the future.
        + */
        + RESERVED(1004),
        +
        + /**
        + * It is a reserved value and MUST NOT be set as a status code in a
        + * Close control frame by an endpoint. It is designated for use in
        + * applications expecting a status code to indicate that no status
        + * code was actually present.
        + */
                 NO_STATUS_CODE(1005),
        - /* 1006 */
        +
        + /**
        + * It is a reserved value and MUST NOT be set as a status code in a
        + * Close control frame by an endpoint. It is designated for use in
        + * applications expecting a status code to indicate that the
        + * connection was closed abnormally, e.g., without sending or
        + * receiving a Close control frame.
        + */
                 CLOSED_ABNORMALLY(1006),
        - /* 1007 */
        +
        + /**
        + * Indicates that an endpoint is terminating the connection
        + * because it has received data within a message that was not
        + * consistent with the type of the message (e.g., non-UTF-8 [RFC3629]
        + * data within a text message).
        + */
                 NOT_CONSISTENT(1007),
        - /* 1008 */
        +
        + /**
        + * Indicates that an endpoint is terminating the connection
        + * because it has received a message that violates its policy. This
        + * is a generic status code that can be returned when there is no
        + * other more suitable status code (e.g., 1003 or 1009) or if there
        + * is a need to hide specific details about the policy.
        + */
                 VIOLATED_POLICY(1008),
        - /* 1009 */
        +
        + /**
        + * Indicates that an endpoint is terminating the connection
        + because it has received a message that is too big for it to
        + process.
        + */
                 TOO_BIG(1009),
        - /* 101 */
        +
        + /**
        + * Indicates that an endpoint (client) is terminating the
        + * connection because it has expected the server to negotiate one or
        + * more extension, but the server didn't return them in the response
        + * message of the WebSocket handshake.
        + */
                 NO_EXTENSION(1010),
        - /* 1011 */
        +
        + /**
        + * Indicates that a remote endpoint is terminating the connection
        + * because it encountered an unexpected condition that prevented it from
        + * fulfilling the request.
        + */
                 UNEXPECTED_CONDITION(1011),
        - /* 1012 */
        - SERVICE_RESTART(1012),
        - /* 1013 */
        - TRY_AGAIN_LATER(1013),
        - /* 1015 */
        +
        +
        + /**
        + * It is a reserved value and MUST NOT be set as a status code in a
        + * Close control frame by an endpoint. It is designated for use in
        + * applications expecting a status code to indicate that the
        + * connection was closed due to a failure to perform a TLS handshake
        + * (e.g., the server certificate can't be verified).
        + */
                 TLS_HANDSHAKE_FAILURE(1015);
         
        -
        - CloseCodes(int code) {
        + private CloseCodes(int code) {
                     this.code = code;
                 }
         
        @@ -149,7 +237,7 @@
                     return code;
                 }
         
        - private int code;
        + private final int code;
             }
         }
        ]
        jitu made changes -
        Description Bunch of changes for CloseReason class
        * Add final modifier for fields
        * UTF-8 encoded bytes for reasonPhrase cannot be greater than 123 bytes
        * enum constants for status codes need be documented. Otherwise, javadoc would be empty

        Attaching a patch (looks like cannot attach a file)

         
        Bunch of changes for CloseReason class
        * Add final modifier for fields
        * UTF-8 encoded bytes for reasonPhrase cannot be greater than 123 bytes. Throw IllegalArgumentException in that case.
        * enum constants for status codes need be documented. Otherwise, javadoc would be empty

        Looks like cannot attach the patch. Will send by email.

         
        jitu made changes -
        Description Bunch of changes for CloseReason class
        * Add final modifier for fields
        * UTF-8 encoded bytes for reasonPhrase cannot be greater than 123 bytes. Throw IllegalArgumentException in that case.
        * enum constants for status codes need be documented. Otherwise, javadoc would be empty

        Looks like cannot attach the patch. Will send by email.

         
        Bunch of changes for CloseReason class
        * Add final modifier for fields
        * UTF-8 encoded bytes for reasonPhrase cannot be greater than 123 bytes. Throw IllegalArgumentException in that case.
        * enum constants for status codes need be documented. Otherwise, javadoc would be empty

        Looks like cannot attach the patch. Will send by email.

        If a close frame doesn't include status code, the Endpoint#onClose() is invoked with a CloseReason. What would be the value of CloseReason#getCloseCode() return in that case ? Would it be 1005 ? Good to include that in the spec.

         
        Hide
        Pavel Bucek added a comment -

        also please add usable CloseReason.toString() implementation.

        Show
        Pavel Bucek added a comment - also please add usable CloseReason.toString() implementation.
        dannycoward made changes -
        Priority Major [ 3 ] Minor [ 4 ]
        Hide
        dannycoward added a comment -

        all updated in v012

        Show
        dannycoward added a comment - all updated in v012
        dannycoward made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Assignee dannycoward [ dannycoward ]
        Tags v012
        Resolution Fixed [ 1 ]
        Hide
        Pavel Bucek added a comment -

        please add null checks; constructor and toString method can end with throwing NPE.

        Show
        Pavel Bucek added a comment - please add null checks; constructor and toString method can end with throwing NPE.
        Pavel Bucek made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        dannycoward added a comment -

        done - thanks !

        Show
        dannycoward added a comment - done - thanks !
        dannycoward made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Tags v012 v013
        Resolution Fixed [ 1 ]

          People

          • Assignee:
            dannycoward
            Reporter:
            jitu
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: