Consolidation and Enhancement of BSD Socket Layer
Hello Botan Development Team,
In reference to Issue #2216, I have undertaken the consolidation and enhancement of the BSD socket layer. This endeavor involved refactoring redundant code within socket.cpp, socket_udp.cpp, and cli/socket_utils.h to foster a more streamlined and maintainable architecture.
Post-refactoring, the codebase compiles successfully, and the botan-test suite executes without errors. However, I am encountering challenges in testing the tls-server and tls-client applications. I would greatly appreciate guidance and assistance in effectively conducting these tests.
As a newcomer to the Botan project, I am impressed by its adherence to high C++ standards, utilization of CMake, and comprehensive documentation via Doxygen. I am eager to contribute to both my personal development and the project's advancement.
I recognize that further simplification could be applied to the files in question. Nonetheless, I am aware that extensive modifications can complicate analysis and review processes. Following this PR, I am open to implementing additional refinements based on your guidance.
I acknowledge that there may be errors in my contributions and am open to making necessary changes. I welcome any feedback and am willing to collaborate to ensure the highest quality of code.
Note: I checked the necessary files with clang-format to ensure that they comply with the file standards. However, despite updating to the latest version, I still received warnings on Debian 12 about "RequiresClausePositin and IndentRequiresClause", so I removed them.
Thank you for considering my contribution.
Sincerely
Hi Kağan! Thanks a lot for looking into this. I didn't look into your code so far, just a quick reply to:
I am encountering challenges in testing the tls-server and tls-client applications. I would greatly appreciate guidance and assistance in effectively conducting these tests.
You can use those CLI-tools to quickly set up and test a TLS client and/or server. It makes sense to test those extensively, as they make use of the socket abstraction API, as you know.
The easiest way to test them is via the CLI-tests. For that, just build the library as usual and enable the CLI. Eg. like that:
./configure.py --without-documentation --cc=clang --compiler-cache=ccache --build-targets=static,cli --build-tool=ninja
ninja cli
The CLI-tests are implemented as a bunch of python functions in src/scripts/test_cli.py. To run the TLS-client/server specific one just go for:
python3 src/scripts/test_cli.py ./botan cli_tls_socket_tests
This runs several tests with a Botan tls_client connecting to a Botan tls_server with different protocol configurations. This should be enough to effectively test those CLI tools.
Thanks, @reneme. Your guidance was extremely helpful.
I reviewed and updated my ninja and ccache tools:
sudo apt-get upgrade ninja-build
sudo apt-get upgrade ccache
Then, after compiling the project, I ran the command you shared:
python3 src/scripts/test_cli.py ./botan cli_tls_socket_tests
The result seems to be successful:
INFO: Ran cli_tls_socket_tests in 0.91 sec 5 tests were run in 0.90 seconds with 0 failures
I would like to state that I am open to doing additional testing and improvements. After reviewing the code content, we can refactor the code for readability if necessary. I am open to additional feedback.
Thanks again for your time and support.
Note: I also want to add. There are still repetitive structures in these sections. But I know that major edits will not be accepted. If this PR goes well and I embrace the scope a little more, we can make another simplification with your guidance.
I have found the time to look at this problem again. I see that 19 out of 38 tests failed, but the Nightly build completed successfully. I will look for errors and try to fix them. Then if you specify any changes you want to make to the parts we reviewed, I will try to make them.
First I look this -> InternalError: No header guard start found in ./src/lib/utils/socket/socket_platform.h
coverage: 90.648% (-0.004%) from 90.652% when pulling 1b04770609a873f39a26dc2cadc8db8da1642dad on KaganCanSit:feature into a2a0b43c3811f9f2a44d271fe590040c72119703 on randombit:master.
Hello again @reneme,
I noticed my mistake at a few points and corrected it. Right now, all the compilations and analyses like valgrind seem to be successful. It seems that the branch can move from draft to review. As I said in my first comment, I think we can manage the process more cleanly and with fewer lines here. However, I am undecided because I am not familiar with the project.
If you want to make edits and improvements, I can do my best and spend the rest of my free time. I am open to learning.
Thanks for sharing your knowledge to me, I am just a comment away. Have a nice day.
Thank you for sharing your knowledge with me and guiding me. I have gained new knowledge. @randombit can be merged after checking. I am just a message away if new edits are needed.
Good day and see you for new merge requests.