fungover / thunder Goto Github PK

Describe the bug

When connecting multiple clients even if they have different clientId the second client will be disconnected when coming from the same ip number.

To Reproduce

Steps to reproduce the behavior:

1. Start the server
2. Connect first client
3. Connect second client from same host
4. Second client will be disconnected and not recive a connect ack.

Expected behavior

Multiple clients with same ip number should be able to connect and only clients connecting using same clientid, part of the connect message header sent from the client when connecting, should be denied.

Environment (please complete the following information):

• Project Version 0.1.0

Since we are many people working on this project some of us might have there own personal settings for formatting of files. This might be problematic since files that are updated by several people might change just because of formatting rules.
We can create an .editorconfig file with common settings that will be applied automatically in all IDEs that support this when opening this project. https://editorconfig.org/

I suggest taking a look in project https://github.com/fungover/haze as inspiration. Some settings might be unnecessary since they might have a default value?

Describe the bug

Dependency for Mockito missing from pom.xml, tests using mockito does not work.

To Reproduce

Steps to reproduce the behavior:

1. Run tests

Expected behavior

Tests using mockito should work.

Problem Description

When a client no longer want to receive messages to a topic we can unsubscribe.
https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718072

In response the broker should send a UNSUBACK
https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718077

Problem Description

Implementing logging in our program. Using either built in logging tools with java.util.logging, or using a logging framework like Log4j

https://www.loggly.com/ultimate-guide/java-logging-basics/

Introduce a client handler to store additional information about clients alongside the socket reference such as
ClientId - each client likely has a unique identifier
Subscribed topics - clients can subscribe to different channels, keeping track of these subscriptions
Status information - info about clients, connection, status, activity, and permissions etc

Add instructions to README.md with docker commands to pull and run the application.
Use code markup with 3 ` to make it easy to copy the instructions one by one and run them in a terminal.

example instruction that can be copied.

Problem Description

A connected client can subscribe to a topic with a SUBSCRIBE message.
buffer[0] == (byte) 0x82
https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718063

After storing any needed information as what topic we subscribe to we send a SUBACK
https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718068

Describe the bug

Duplicate dependencies tags in pom.xml introduced from merge #5

To Reproduce

Steps to reproduce the behavior:

1. Go to pom.xml
2. See error

Problem Description

When a new client connects #6 to serversocket we need to handle that connection and wait for a CONNECT request.
Broker should then return CONNACK message.

We need inputStream and outputStream from client socket and after reading some data we can check if the incoming request is a CONNECT request.
https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718028

`InputStream inputStream = socket.getInputStream();
OutputStream outputStream = socket.getOutputStream();
byte[] buffer = new byte[1024];

            int bytesRead = inputStream.read(buffer);
            if (bytesRead > 0 && buffer[0] == (byte) 0x10) {
                System.out.println("Received MQTT CONNECT message from client");
                byte[] connackMessage = new byte[]{(byte) 0x20, (byte) 0x02, (byte) 0x00, (byte) 0x00};
                outputStream.write(connackMessage);
                outputStream.flush();
                System.out.println("Sent MQTT CONNACK message to client");`

• After a Network Connection is established by a Client to a Server, the first Packet sent from the Client to the Server MUST be a CONNECT Packet
• The Server MUST process a second CONNECT Packet sent from a Client as a protocol violation and disconnect the Client
• In a connect message the first byte is hexadecimal value 0x10

Response in form of CONNACK
https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718033

To prevent denial of service attacks against our server we could add a limit time and disconnect the client if inputStream.read(buffer); hasn't returned in for example 30 seconds.
Java supports timeout on sockets read using the setSoTimeout method.

Add dependencies for unit tests in the pom.xml file.

Describe the bug

Pull requests contain unnecessary formating that has nothing to do with issue.

To Reproduce

Steps to reproduce the behavior:

1. Open IntelliJ.
2. Go to project.
3. Select multiple files that you'd like to format.
4. Press CTRL + ALT + L to trigger the format operation.
5. Done.

Expected behavior

I expect that when I press CTRL + ALT + L, the selected files would be automatically formatted according to the project's coding style or formatting rules.

Environment:

• OS: [Windows, Linux]

When creating a new pull request we can have SonarCloud linting of our code and calculate code coverage from tests. For the test coverage to work we have to add jacoco-maven-plugin and also call sonarlint with a key from our build pipeline.

Problem Description

All MQTT brokers opens a server on port 1883 by default. So should our program also do and wait for incoming connections.

Describe the bug

When trying to run a docker image created by our Dockerfile the entrypoint path to Main class is wrong. Should be org.fungover.thunder.Main

To Reproduce

Steps to reproduce the behavior:

1. docker build --tag thunder .
2. docker run thunder
3. Error: Could not find or load main class org.fungover.Main
   Caused by: java.lang.ClassNotFoundException: org.fungover.Main

Expected behavior

main method in class Main should be called.

Add a mechanism for authenticating and authorizing clients, including username/password authentication and access control based on client roles.

1. Username/Password Authentication:
   Implement a mechanism to require clients to provide a valid username and password during the connection handshake. This ensures that only authenticated clients can connect to the server. The credentials can be stored securely on the server side, and the client must provide the correct combination to establish a connection.

2. Access Control Based on Client Roles:
   Introduce the concept of roles for clients, allowing you to define specific permissions or access levels for different clients. For example, you might have clients with roles like "admin," "publisher," or "subscriber." Based on the role, you can enforce restrictions on actions such as publishing or subscribing to specific topics.

An easy way of distributing and running server applications is by using docker. To package our application as an image for docker we need a Dockerfile with layer commands.

We can build an image using the files in target folder that can be created by running mvn commands or we can make a multistage Dockerfile so the build process runs in a docker container. Using this technique we don't need java or maven installed on the computer running the build process.

I therefor suggests creating a multistage Dockerfile in root folder of this project.
When copying project files to the build image we can avoid copying files not needed for the build process. To specify which files we can create a .dockerignore file in the root folder. https://shisho.dev/blog/posts/how-to-use-dockerignore/

Add github action that runs compile and test when creating pull requests or pushing to main.

The project now has a Dockerfile that can produce a docker image for our project. When it's time to do a release we want to be able to create a version tag on main branch and then create a release on github.
The creation of this release should trigger an automatic build and publication of docker images for the following platforms and architectures: linux/amd64 and linux/arm64v8

The image should be uploaded both to github packages for this project and to dockerhub fungover/thunder

A working example from project haze can be found here: https://github.com/fungover/haze/blob/main/.github/workflows/release.yml
When using that file as a template we need to update destinations where to publish the images and for what platforms it should be built.
Action versions should also be updated to newest version. Make sure to check the documentation if some of the settings have changed to other names or default values that can be used.

Problem Description

Two other projects already have the MIT license. In light of this, wouldn't it be beneficial to also include a license for Thunder project

Problem Description

The Client uses a PUBLISH Packet to send an Application Message to the Server, for distribution to Clients with matching subscriptions.
The Server uses a PUBLISH Packet to send an Application Message to each Client which has a matching subscription.
https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718037

The receiver of a PUBLISH Packet MUST respond according to:
QoS 0 | None
QoS 1 | PUBACK Packet
QoS 2 | PUBREC Packet
https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718043

In this first implementation lets focus on handling QoS level 0 only so no response messages has to be sent.

Problem Description

At the moment our code in clientHandler.handleConnections() isn't using threads or async I/O and can therefore not handle multiple clients at the same time. Can be seen as some sort of a bug and will have impacts on future developement.

I propose moving the call to serverSocket.accept() into our start method in Server class and then call ClientHandler handleConnection with the retrieved Socket objekt in a new thread.

Since we are using Java21 i propose using virtual threads.
Code suggestions:

public void start() {
        try {
            System.out.println("Server started on port 1883");
            while (!serverSocket.isClosed()) {
                Socket connection = null;
                connection = serverSocket.accept();
                Thread.ofVirtual(() -> clientHandler.handleConnection(connection));
            }
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

Problem Description

The DISCONNECT Packet is the final Control Packet sent from the Client to the Server. It indicates that the Client is disconnecting cleanly.

Documentation

Problem Description

In a MQTT broker we need some way of handling topics that clients are registered to and to which topic some other clients wants to send messages. Topics can be an exact match like myhome/first_floor/kitchen/temperature but we can also use wildcards so myhome/first_floor/+/temperature is a valid topic name for subscribing to messages.
There is also some special topics starting with $ that are for internal use and sending out information about the broker.

We need some code that can see if two topics matches when using both exact matching and wildcard matching (+#). We could also have some methods that validate if a topic name is following best practices rules for naming and if it's a valid topic for subscription and/or publishing.

It’s important to note that wildcards can only be used for subscription and not for publishing messages.

We would like classes for handling topics, maybe a Topic datatype that can store information about the topic like it's name but also have methods for matching topics against each other using wildcards?

More reading about topics here.
And here

Describe the bug

GitHub doesn't currently support templating certain metadata for pull requests, such as labels, assignees, and title.
you can read more about it here or here .

Technically, there is a workaround, but it appears to be limited to managing labels and requires the use of Github Actions. If someone is interested in addressing this issue, it may be advisable to create a new one.

To Reproduce

Steps to reproduce the behavior:

1. Create a PR
2. You will see header of Metadata

Expected behavior

The Metadata section should not be displayed in the template as mentioned earlier, it is not currently supported.

name: Pull Request
about: Create a pull request to contribute to the project
title: "[PR] - Brief Description of Changes"
labels: ''
assignees: ''

Environment (please complete the following information):

• OS: [Windows]
• Java Version: [e.g. 21]
• Project Version [1.0.0]

Problem Description

A connected client can send a PINGREQ and get a PINGRESP back to make sure the connection is alive.

PINGREQ
PINGRESP

if (buffer[0] == (byte) 0xC0) {
                    System.out.println("Received MQTT PINGREQ message from client");
                    byte[] pingrespMessage = new byte[]{(byte) 0xD0, (byte) 0x00};
                    outputStream.write(pingrespMessage);
                    outputStream.flush();
                    System.out.println("Sent MQTT PINGRESP message to client");

Problem Description

The existing test cases in the ClientHandlerTest are currently ineffective and in need of refinement.
With the recent updates to the code in the ClientHandler, there is no working test coverage for ClientHandler. Therefore, it's imperative to enhance the test suite to ensure 80% coverage for the updated code

Problem Description

create a unit test for making sure that multiple clients can connect to the server simultaneously.

Describe the bug

When running version 0.1.0 and connecting a client successfully a java.net.SocketTimeoutException: Read timed out happens and prints a stacktrace. The application keeps running but no new connections can be made.

To Reproduce

Steps to reproduce the behavior:

1. Start the server as a docker container or in IntelliJ.
2. Connect a client and the server will accept the connection.
3. Wait for more than 30 sec.
4. Error message as a stacktrace will be printed in the console:
   java.net.SocketTimeoutException: Read timed out at java.base/sun.nio.ch.NioSocketImpl.timedRead(NioSocketImpl.java:278) at java.base/sun.nio.ch.NioSocketImpl.implRead(NioSocketImpl.java:304) at java.base/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:346) at java.base/sun.nio.ch.NioSocketImpl$1.read(NioSocketImpl.java:796) at java.base/java.net.Socket$SocketInputStream.read(Socket.java:1099) at java.base/java.io.InputStream.read(InputStream.java:220) at org.fungover.thunder.PackageReader.isCleanDisconnect(PackageReader.java:65) at org.fungover.thunder.ClientHandler.handleConnection(ClientHandler.java:28) at org.fungover.thunder.Server.lambda$start$0(Server.java:23) at java.base/java.lang.VirtualThread.run(VirtualThread.java:309)

Expected behavior

No socket timeout should happen and no stacktrace should be printed to stdout.
Only the first read for a MQTT_CONNECT message need a timeout. After that the timeout should be disabled using socket.setSoTimeout(0); since a connected client might not send anything for a long time.
After a client connects we want a timeout of around 30 seconds when waiting for Connect message and if no such message arrives the client should be disconnected and no stacktrace should be printed but a nicely formatted log message could be used.

Environment

• OS: docker / linux
• Java Version: 21
• Project Version 0.1.0

In reference to Issue #12, since there are websites that look at Githubs own Label 'good first issue' and lists these for other programmers to start with. We should add our own Label for good first issues that has a similar but different name, that we can use for marking issues that can be good to start with as a first task.