Enhance CyberCafe infrastructure and testing#43
Conversation
…s commands for correct syntax
Daniel-Lopez246
merged commit 9ad59ad
into
Feature/Utility-Library-(ASU-CIDSE-2025-Fall)
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes a variable reference in a database query, updates iptables destination formatting for consistency, and introduces eval for command execution in the setup script. Feedback highlights a redundant database query that could be removed for efficiency and suggests simplifying run_cmd calls by removing redundant error handling and redirections already managed by the function.
| #Necessary data from internet_session row | ||
| { | ||
| RESPONSE=$(sqlite3 "${DATABASE_PATH}" "SELECT * FROM internet_sessions WHERE table_index=${I}") > /dev/null 2>> error.log | ||
| RESPONSE=$(sqlite3 "${DATABASE_PATH}" "SELECT * FROM internet_sessions WHERE table_index=${1}") > /dev/null 2>> error.log |
There was a problem hiding this comment.
While this change correctly fixes the variable from ${I} to ${1}, this database query is redundant. The RESPONSE variable is already populated with the same data from the query on line 117. This second query can be removed to improve efficiency.
|
|
||
| # also attempt to delete the specific rules added by John's setup (tcp redirect & FORWARD DROP) if present | ||
| run_cmd "iptables -t nat -D PREROUTING -p tcp -i '${HS_INTERFACE}' -j DNAT --to-destination '${LOCAL_IP}':80 > /dev/null 2>> error.log || true" | ||
| run_cmd "iptables -t nat -D PREROUTING -p tcp -i '${HS_INTERFACE}' -j DNAT --to-destination '${LOCAL_IP}:80' > /dev/null 2>> error.log || true" |
There was a problem hiding this comment.
The run_cmd function already handles redirecting stderr to error.log and suppressing command failures with || true. Including > /dev/null 2>> error.log || true inside the string passed to run_cmd is redundant and makes the command harder to read. The call should be simplified to only pass the core command.
| run_cmd "iptables -t nat -D PREROUTING -p tcp -i '${HS_INTERFACE}' -j DNAT --to-destination '${LOCAL_IP}:80' > /dev/null 2>> error.log || true" | |
| run_cmd "iptables -t nat -D PREROUTING -p tcp -i '${HS_INTERFACE}' -j DNAT --to-destination '${LOCAL_IP}:80' > /dev/null" |
1 participant
Integrate John's prototype work, improve session management, and enhance logging and error handling. Implement automated tests for critical functions, ensuring better reliability and maintainability of the CyberCafe system.