Skip to content

Add JavaDoc for IpFilter #136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
HerrKanin wants to merge 1 commit into from
Closed

Add JavaDoc for IpFilter #136

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions src/main/java/org/juv25d/filter/IpFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,23 @@
import java.nio.charset.StandardCharsets;
import java.util.HashSet;
import java.util.Set;

/**
* Global IP-based request filter.
*
* <p>The filter runs early in the request pipeline and determines whether a request
* should be allowed to continue based on the client's IP address.</p>
*
* <p>Decision rules:</p>
* <ul>
* <li>If an IP is present in the whitelist, it is allowed.</li>
* <li>If an IP is present in the blacklist, it is denied.</li>
* <li>If an IP is present in both lists, {@code allowByDefault} is used.</li>
* <li>If an IP is present in neither list, {@code allowByDefault} is used.</li>
* </ul>
Comment on lines +19 to +25

@coderabbitai coderabbitai Bot Feb 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

JavaDoc rule ordering does not match actual precedence in isAllowed.

The list implies whitelist unconditionally takes priority (rule 1), but isAllowed checks "in both" before the pure-whitelist check (line 87 vs. line 89). A developer reading only the Javadoc would expect that a whitelisted IP is always allowed, which is incorrect when the IP also appears in the blacklist. The "both lists" case should be listed first to reflect actual code precedence.

📝 Proposed fix 
  * <p>Decision rules:</p>
  * <ul>
- *     <li>If an IP is present in the whitelist, it is allowed.</li>
- *     <li>If an IP is present in the blacklist, it is denied.</li>
  *     <li>If an IP is present in both lists, {`@code` allowByDefault} is used.</li>
+ *     <li>If an IP is present in the whitelist only, it is allowed.</li>
+ *     <li>If an IP is present in the blacklist only, it is denied.</li>
  *     <li>If an IP is present in neither list, {`@code` allowByDefault} is used.</li>
  * </ul>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* <p>Decision rules:</p>
* <ul>
* <li>If an IP is present in the whitelist, it is allowed.</li>
* <li>If an IP is present in the blacklist, it is denied.</li>
* <li>If an IP is present in both lists, {@code allowByDefault} is used.</li>
* <li>If an IP is present in neither list, {@code allowByDefault} is used.</li>
* </ul>
* <p>Decision rules:</p>
* <ul>
* <li>If an IP is present in both lists, {`@code` allowByDefault} is used.</li>
* <li>If an IP is present in the whitelist only, it is allowed.</li>
* <li>If an IP is present in the blacklist only, it is denied.</li>
* <li>If an IP is present in neither list, {`@code` allowByDefault} is used.</li>
* </ul>
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/filter/IpFilter.java` around lines 19 - 25, Update
the JavaDoc in class IpFilter to reflect actual precedence used in the isAllowed
method: list the "present in both lists" rule before the individual
whitelist/blacklist rules so readers see that the both-case is evaluated first
and falls back to allowByDefault; reference the isAllowed method and the
allowByDefault flag when rewording the bullets to ensure the Javadoc matches
code behavior.

*
* <p>The default constructor loads configuration from {@link IpFilterConfig}.</p>
*/
@Global(order = 2)
public class IpFilter implements Filter {

Expand All @@ -17,6 +34,14 @@ public class IpFilter implements Filter {

private final boolean allowByDefault;

/**
* Creates an {@code IpFilter} with explicit configuration.
*
* @param whitelist IP addresses that should be allowed (may be {@code null})
* @param blacklist IP addresses that should be denied (may be {@code null})
* @param allowByDefault fallback decision when an IP is not listed, or listed in both sets
*/

public IpFilter(Set<String> whitelist, Set<String> blacklist, boolean allowByDefault) {
Comment on lines +37 to 45

@coderabbitai coderabbitai Bot Feb 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Blank lines between Javadoc blocks and their declarations detach the docs from generated output.

Lines 44, 58, and 83 each introduce a blank line between the closing */ and the corresponding constructor/method declaration. The standard javadoc tool requires the comment to immediately precede the element it documents; a blank line causes the association to be silently dropped, so the generated HTML will show these members as undocumented.

📝 Proposed fix (shown for parameterized constructor; apply the same to the no-arg constructor and `isAllowed`) 
     * `@param` allowByDefault fallback decision when an IP is not listed, or listed in both sets
     */
-
     public IpFilter(Set<String> whitelist, Set<String> blacklist, boolean allowByDefault) {

Also applies to: 55-59, 77-84

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/filter/IpFilter.java` around lines 37 - 45, The
Javadoc comments for the IpFilter constructors and isAllowed method are detached
by stray blank lines; remove the empty line immediately following each closing
*/ so the Javadoc block directly precedes the corresponding declaration (apply
to the parameterized constructor IpFilter(Set<String> whitelist, Set<String>
blacklist, boolean allowByDefault), the no-arg constructor IpFilter(), and the
isAllowed(...) method) to ensure javadoc associates the comments with those
members.

if (whitelist != null) {
this.whitelist.addAll(whitelist);
Expand All @@ -27,6 +52,10 @@ public IpFilter(Set<String> whitelist, Set<String> blacklist, boolean allowByDe
this.allowByDefault = allowByDefault;
}

/**
* Creates an {@code IpFilter} using configuration loaded from {@link IpFilterConfig}
*/

public IpFilter() {
IpFilterConfig config = new IpFilterConfig();
this.whitelist.addAll(config.whitelist());
Expand All @@ -45,8 +74,16 @@ public void doFilter(HttpRequest req, HttpResponse res, FilterChain chain) throw
}
}

/**
* Evaluates whether the given IP address should be allowed.
*
* @param ip client IP address
* @return {@code true} if the request is allowed, {@code false} otherwise
*/

public boolean isAllowed(String ip) {

// If an IP exists in both lists, fall back to allowByDefault
if (whitelist.contains(ip) && blacklist.contains(ip)) return allowByDefault;

if (whitelist.contains(ip)) return true;
Expand Down