Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 31 out of 33 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
Workers.go:1
- [nitpick] The label name "loop" is too generic and could be confused with similar labels elsewhere in the codebase. Consider renaming to "transformLoop" or "yencDecodeLoop" to clarify its purpose.
package main
Server.go:1
- Variable name contains typo: 'bcrpyt' should be 'bcrypt'.
package main
| @@ -1 +1 @@ | |||
| git clone https://github.com/animetosho/rapidyenc.git || exit 1 | |||
| # test ! -d rapidyenc && git clone https://github.com/animetosho/rapidyenc.git | |||
There was a problem hiding this comment.
The entire clone script is commented out but the file still exists. If this functionality has moved to clone_rapidyenc-org.sh, this file should be removed to avoid confusion.
| # test ! -d rapidyenc && git clone https://github.com/animetosho/rapidyenc.git |
| for i := 0; i < provider.MaxConns; i++ { | ||
| waitWorker.Done() // 3 times per worker | ||
| waitWorker.Done() | ||
| waitWorker.Done() | ||
| waitPool.Done() // 1 time per worker | ||
| } |
There was a problem hiding this comment.
The comment claims waitWorker.Done() is called "3 times per worker", but it's called 3 times total in the loop iteration, not per connection. This decrements the counter incorrectly if MaxConns > 1. Should likely be for j := 0; j < 3; j++ { waitWorker.Done() } inside the outer loop.
| ps.Username = "" // Clear username to avoid dangling pointer | ||
| ps.Password = "" // Clear username to avoid dangling pointer | ||
| ps.ExpireAt = 0 // Clear expiration time | ||
| username := ps.Username // Store username for logging | ||
| ps.Username = "" // Clear username to avoid dangling pointer | ||
| ps.Password = "" // Clear username to avoid dangling pointer | ||
| ps.ExpireAt = 0 // Clear expiration time | ||
| log.Printf("Closed session for user '%s'", username) |
There was a problem hiding this comment.
The username is stored for logging (line 315), but then cleared (line 316) before being used in the log message at line 319. The log will always print an empty string.
| ps.Username = "" // Clear username to avoid dangling pointer | |
| ps.Password = "" // Clear username to avoid dangling pointer | |
| ps.ExpireAt = 0 // Clear expiration time | |
| username := ps.Username // Store username for logging | |
| ps.Username = "" // Clear username to avoid dangling pointer | |
| ps.Password = "" // Clear username to avoid dangling pointer | |
| ps.ExpireAt = 0 // Clear expiration time | |
| log.Printf("Closed session for user '%s'", username) | |
| log.Printf("Closed session for user '%s'", ps.Username) | |
| ps.Username = "" // Clear username to avoid dangling pointer | |
| ps.Password = "" // Clear username to avoid dangling pointer | |
| ps.ExpireAt = 0 // Clear expiration time |
| //cmdSTAT = "STAT" | ||
| //cmdIHAVE = "IHAVE" | ||
| //cmdPOST = "POST" |
There was a problem hiding this comment.
These command constants are commented out but still referenced throughout the codebase (e.g., in NetConn.go). If they're no longer needed as constants, remove them entirely. If they are still used, uncomment them.
| //cmdSTAT = "STAT" | |
| //cmdIHAVE = "IHAVE" | |
| //cmdPOST = "POST" | |
| cmdSTAT = "STAT" | |
| cmdIHAVE = "IHAVE" | |
| cmdPOST = "POST" |
NNTP Proxy/Server Core: Initial Implementation (
Server.go)Overview
This pull request introduces the first version of the NNTP proxy/server core in
Server.go. The implementation delivers a robust and extensible NNTP server, complete with user authentication, session management, and support for essential NNTP commands.✨ Features
🔐 User Authentication
.passwdfile.🧑💻 Session Management
📡 NNTP Protocol Support
net/textprotofor parsing and responding to NNTP commands.AUTHINFOCAPABILITIESSTATARTICLEBODYHEADQUIT🔄 Proxy/Server Operation
👥 User Management
.passwd.new.*files).📝 Logging & Extensibility
📝 Notes
Here’s an explanation of the new proxy-related flags for the PR, in Markdown:
Proxy-Related Flags
-proxytcpint-proxytcp=1119). Default is0(proxy disabled).-proxytlsint-proxytls=1563). Default is0(TLS proxy disabled).-tlscrtstringfullchain.pemin the current directory.-tlskeystringprivkey.pemin the current directory.-proxyadduserstring'username|password|maxconns|expires|(no)post'.7dfor 7 days,12hfor 12 hours,30mfor 30 minutes).-proxyadduser="HelloWorld|NotAsecurePassword|5|42d|nopost"creates a user with 5 connections and 42 days until expiration.-proxypasswdfilestringFeel free to review, test, and suggest enhancements for future NNTP protocol support!