TooTallNate / Java-WebSocket

A barebones WebSocket client and server implementation written in 100% Java.
http://tootallnate.github.io/Java-WebSocket
MIT License
10.47k stars 2.57k forks source link

Bug Report: Violation of Liskov Substitution Principle #1426

Closed 2Dsa3 closed 3 weeks ago

2Dsa3 commented 3 months ago

Description of the Problem In the Java-WebSocket repository, the DefaultExtension class provides a basic implementation of the isFrameValid method, which checks if any reserved bits (RSV1, RSV2, or RSV3) are set. However, the child class CompressionExtension adds additional conditions specific to DataFrame and ControlFrame. When substituting an instance of DefaultExtension with an instance of CompressionExtension, different behavior occurs in frame validation depending on the type of frame received. This violates the Liskov Substitution Principle and can cause confusion for the user.

Steps to Reproduce Open the DefaultExtension.java file located at src/main/java/org/java_websocket/extensions/. Observe the implementation of the isFrameValid method. Open the CompressionExtension.java file in the same directory. Compare the overridden isFrameValid method in CompressionExtension with that of DefaultExtension. Expected Behavior The CompressionExtension should adhere to the contract established by the DefaultExtension class. The isFrameValid method should behave consistently, regardless of whether it is called on an instance of DefaultExtension or CompressionExtension.

Proposed Solution To resolve this violation, the following steps are recommended:

Ensure Consistent Behavior: Modify the CompressionExtension class to align with the behavior of the DefaultExtension class while appropriately extending its functionality. Refactor Code: If the additional conditions are necessary, consider using composition over inheritance to ensure consistent behavior. Code Example DefaultExtension Implementation:

java Copiar código public class DefaultExtension { public boolean isFrameValid(Frame frame) { // Logic to check if any RSV bits are set return !(frame.isRSV1() || frame.isRSV2() || frame.isRSV3()); } } CompressionExtension Implementation (Current):

java Copiar código public class CompressionExtension extends DefaultExtension { @Override public boolean isFrameValid(Frame frame) { if (frame instanceof DataFrame) { // Additional conditions for DataFrame } else if (frame instanceof ControlFrame) { // Additional conditions for ControlFrame } // Existing check for RSV bits return !(frame.isRSV1() || frame.isRSV2() || frame.isRSV3()); } } CompressionExtension Implementation (Proposed):

java Copiar código public class CompressionExtension extends DefaultExtension { @Override public boolean isFrameValid(Frame frame) { if (!super.isFrameValid(frame)) { return false; } if (frame instanceof DataFrame) { // Additional conditions for DataFrame return checkDataFrame(frame); } else if (frame instanceof ControlFrame) { // Additional conditions for ControlFrame return checkControlFrame(frame); } return true; }

private boolean checkDataFrame(Frame frame) {
    // Specific checks for DataFrame
    return true;
}

private boolean checkControlFrame(Frame frame) {
    // Specific checks for ControlFrame
    return true;
}

} By ensuring that the CompressionExtension class adheres to the contract established by the DefaultExtension class, we maintain consistent behavior and comply with the Liskov Substitution Principle.

Additional Information Providing any other details that can help resolve the issue would be beneficial.

This approach should help address the violation of the Liskov Substitution Principle and ensure more maintainable and reliable code within the Java-WebSocket project.