Skip to content

Conversation

Copy link

Copilot AI commented Jul 20, 2025

Summary

This PR adds comprehensive test coverage for the DNS cache functionality by creating a new dedicated test file src/gwproxy/tests/dns_cache.c. The tests follow existing patterns in the codebase and provide thorough validation of all DNS cache operations.

Changes Made

New Test File: src/gwproxy/tests/dns_cache.c

  • 11 comprehensive test functions covering all DNS cache functionality:
    • test_dns_cache_init_free() - Basic initialization and cleanup
    • test_dns_cache_basic_insert_lookup() - Insert and lookup operations
    • test_dns_cache_ipv6_support() - IPv6 address handling
    • test_dns_cache_mixed_ipv4_ipv6() - Mixed IPv4/IPv6 entries
    • test_dns_cache_entry_replacement() - Entry replacement behavior
    • test_dns_cache_expiration() - Cache expiration handling
    • test_dns_cache_housekeeping() - Cleanup of expired entries
    • test_dns_cache_hash_collisions() - Hash collision handling
    • test_dns_cache_reference_counting() - Reference counting validation
    • test_dns_cache_invalid_inputs() - Error handling and edge cases
    • test_dns_cache_large_dataset() - Performance with large datasets

Updated Build System

  • Modified Makefile to include the new DNS cache test:
    • Added LIBGWDNS_CACHE_TEST_TARGET and related variables
    • Added build rule for dns_cache.t test executable
    • Updated main test target to include DNS cache tests

Test Coverage

The tests validate all public APIs from gwproxy/dns_cache.h:

  • gwp_dns_cache_init() and gwp_dns_cache_free()
  • gwp_dns_cache_insert() and gwp_dns_cache_getent()
  • gwp_dns_cache_putent() for reference counting
  • gwp_dns_cache_housekeep() for cleanup
  • Helper functions gwp_dns_cache_entget_i4() and gwp_dns_cache_entget_i6()

Usage

# Build and run DNS cache tests specifically
make src/gwproxy/tests/dns_cache.t
LD_LIBRARY_PATH=$(pwd) ./src/gwproxy/tests/dns_cache.t

# Or run as part of the full test suite
make test  # (when other tests are fixed)

Test Results

All 11 DNS cache tests pass successfully:

Running DNS cache tests...
test_dns_cache_init_free: passed
test_dns_cache_basic_insert_lookup: passed
test_dns_cache_ipv6_support: passed
test_dns_cache_mixed_ipv4_ipv6: passed
test_dns_cache_entry_replacement: passed
test_dns_cache_expiration: passed
test_dns_cache_housekeeping: passed
test_dns_cache_hash_collisions: passed
test_dns_cache_reference_counting: passed
test_dns_cache_invalid_inputs: passed
test_dns_cache_large_dataset: passed
All DNS cache tests passed!

Code Quality

  • Follows existing code patterns and style
  • Includes proper GPL-2.0 license header
  • Uses assert-based testing like other test files
  • Handles both positive and negative test cases
  • Tests edge cases and error conditions
  • No compiler warnings

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

ammarfaizi2 and others added 10 commits July 17, 2025 19:21
* dns:
  gwproxy: Swap out the old DNS setup for the new library version

Link: #3
Signed-off-by: Ammar Faizi <[email protected]>
If there are 1 entries and 5 workers, and nr_sleeping is 5. It will
always batch because nr_entries + 16 is always greater than 5. That's
just wrong.

Fixes: 317dc79 ("dns: Handle DNS query in batch via `getaddrinfo_a()`")
Signed-off-by: Ammar Faizi <[email protected]>
…n_handle_data()`

The `gwp_socks5_conn_handle_data()` does not properly handle socks5
data if the CONNECT command comes together with the proxied data.

Fixes: 97a3bc1 ("socks5: Initial socks5 modularization")
Signed-off-by: Ammar Faizi <[email protected]>
`gwp_socks5_conn_handle_data()` may return `-EAGAIN` and have `in_len`
or `out_len` greater than zero. Make sure to account those values.

Fixes: 3fad620 ("gwproxy: socks5: Use the socks5 library")
Signed-off-by: Ammar Faizi <[email protected]>
The call to `handle_socks5_data()` must only be done when the
connection is in `CONN_STATE_SOCKS5_DATA` state.

Fixes: 3fad620 ("gwproxy: socks5: Use the socks5 library")
Signed-off-by: Ammar Faizi <[email protected]>
After stress-testing the SOCKS5 proxy server, I found various memory
corruption bugs. Here is the stress-test program:

  https://gist.github.com/ammarfaizi2/53324bb7dd8ca7cb73a92b80b62f8f3b

Only the first patch is not related to memory corruption bugs.

* bug-fix:
  gwproxy: Fix an invalid `handle_socks5_data()` call
  gwproxy: Fix missing buf advance and `target.len` increment
  gwproxy/socks5: Handle `GWP_SOCKS5_ST_CMD_CONNECT` in `gwp_socks5_conn_handle_data()`
  gwproxy/dns: Fix the wrong batch condition decision

Link: #4
Signed-off-by: Ammar Faizi <[email protected]>
For future reusability.

Signed-off-by: Ammar Faizi <[email protected]>
* extract-syscall:
  gwproxy: Split syscall code into a separate header file

Signed-off-by: Ammar Faizi <[email protected]>
The old DNS cache setup in dns.c was kinda clunky because it needed to
slog through everything in O(n) time. This patch series swaps it out
for something way better and tidier. Create a new C file, dns_cache.c,
just for the DNS cache stuff to keep things nice and separate.

Plus, the storage is now smarter about memory, using this compact
structure:
```
struct gwp_dns_cache_entry {
	/**
	 * Super compact domain name and IP address list.
	 *
	 * - name_len: length of the domain name (including null terminator).
	 * - nr_i4: number of IPv4 addresses.
	 * - nr_i6: number of IPv6 addresses.
	 * - block: flexible array member that contains:
	 *     - The first name_len octets are the domain name.
	 *     - The next nr_i4 * 4 octets are IPv4 addresses.
	 *     - The next nr_i6 * 16 octets are IPv6 addresses.
	 */
	uint8_t		name_len;
	uint8_t		nr_i4;
	uint8_t		nr_i6;
	uint8_t		block[];
};
```

Also, the lookup now uses a hashmap, so it's super fast, O(1) on average.

* dns-cache:
  gwproxy/dns: Replace the old DNS cache system with a new one.
  gwproxy/dns_cache: Introduce entry block getter functions
  gwproxy/dns_cache: Don't return expired cache entry
  gwproxy/dns_cache: Add a forward declaration of 'struct addrinfo'
  gwproxy/dns_cache: Add `nr_entries` for accounting purpose
  gwproxy/dns_cache: Initial DNS cache rework

Link: #5
Signed-off-by: Ammar Faizi <[email protected]>
Copilot AI changed the title [WIP] Add tests for the DNS cache feature (create a new file dns_cache.c). Follow other test patterns. Add comprehensive DNS cache tests in new dns_cache.c file Jul 20, 2025
Copilot AI requested a review from ammarfaizi2 July 20, 2025 15:24
Copilot finished work on behalf of ammarfaizi2 July 20, 2025 15:24

printf("All DNS cache tests passed!\n");
return 0;
} No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

End with a blank line.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in commit a84e16a - added trailing newline to the end of the file.

Copilot AI requested a review from ammarfaizi2 July 20, 2025 15:34
Copilot finished work on behalf of ammarfaizi2 July 20, 2025 15:34
assert(ai != NULL);

/* Insert entry */
time_t expire_time = time(NULL) + 300; /* 5 minutes from now */
Copy link
Contributor

Choose a reason for hiding this comment

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

Put all variable declarations at the beginning of the scope (so move that time_t var to the top). Apply that to other places, too.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in commit 840f5cd - moved all variable declarations to the beginning of their respective function scopes across all test functions.

Comment on lines 555 to 565
test_dns_cache_init_free();
test_dns_cache_basic_insert_lookup();
test_dns_cache_ipv6_support();
test_dns_cache_mixed_ipv4_ipv6();
test_dns_cache_entry_replacement();
test_dns_cache_expiration();
test_dns_cache_housekeeping();
test_dns_cache_hash_collisions();
test_dns_cache_reference_counting();
test_dns_cache_invalid_inputs();
test_dns_cache_large_dataset();
Copy link
Contributor

Choose a reason for hiding this comment

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

Test these 5000 times.

Copy link
Author

Choose a reason for hiding this comment

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

Modified the main function to run all DNS cache tests 5000 times in a loop with progress indication every 500 iterations. Commit 8f3c942

Copilot AI requested a review from ammarfaizi2 July 20, 2025 15:45
Copilot finished work on behalf of ammarfaizi2 July 20, 2025 15:45
Copilot finished work on behalf of ammarfaizi2 July 20, 2025 15:47
@ammarfaizi2 ammarfaizi2 changed the base branch from master to dns-cache July 20, 2025 16:53
Copy link
Contributor

@ammarfaizi2 ammarfaizi2 left a comment

Choose a reason for hiding this comment

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

Rebase on top of the dns-cache branch.

Copy link
Author

Copilot AI commented Jul 20, 2025

Rebase on top of the dns-cache branch.

Rebased on top of the dns-cache branch (commit 82c86a7) as requested. All DNS cache tests continue to pass successfully across 5000 iterations. Commit 219e83f

Copilot AI requested a review from ammarfaizi2 July 20, 2025 16:58
Copilot finished work on behalf of ammarfaizi2 July 20, 2025 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants