Do the samething for helpers.js

[HackingRepo]

PR Summary by Qodo

Improve DNS record resolution with parallel queries and lookup fallback
🐞 Bug fix ✨ Enhancement 🕐 10-20 Minutes

Walkthroughs

Description

• Resolve A/AAAA/CNAME records in parallel to reduce DNS latency.
• Fall back to dns.lookup(all:true) when resolve4/resolve6 return no IPs.
• Preserve CNAME resolution while improving robustness for edge-case hostnames.

Diagram

graph TD
  Caller["Caller"] --> R["resolve_all_records(host)"] --> P["Promise.all: resolve4/6/CNAME"] --> D{"A & AAAA empty?"}
  D -- "No" --> Ret["Return {A, AAAA, CNAME}"]
  D -- "Yes" --> L["dns.lookup(all:true)"] --> M["Append IPs by family"] --> Ret

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use dns.resolveAny() to fetch all record types

• ➕ Single API call can return multiple record types (including A/AAAA/CNAME)
• ➕ Potentially simpler control flow
• ➖ Inconsistent support/behavior across environments and record types
• ➖ Requires extra parsing and type-guarding of heterogeneous results

2. Use dns.lookup(all:true) as the primary mechanism

• ➕ Matches OS resolver behavior (can work when authoritative DNS queries fail)
• ➕ Often sufficient if only IP addresses are needed
• ➖ May not provide CNAME chain information reliably (still need resolveCname)
• ➖ Can differ from direct DNS resolution semantics (search domains, etc.)

Recommendation: Keep the PR’s approach: parallel resolve4/resolve6/resolveCname for direct DNS semantics and add a targeted dns.lookup fallback only when A/AAAA are empty. This balances performance, preserves CNAME behavior, and improves robustness for environments where resolve4/resolve6 can return empty despite the host being reachable.

File Changes

Bug fix (1)

helpers.js Parallelize DNS resolution and add lookup fallback for missing A/AAAA +18/-8

Parallelize DNS resolution and add lookup fallback for missing A/AAAA

• Reworks resolve_all_records() to resolve A/AAAA/CNAME concurrently via Promise.all. If both A and AAAA are empty, it attempts dns.lookup({ all: true }) and populates A/AAAA based on address family before returning the aggregated result.

dist/helpers.js

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0)

1. Lookup errors ignored 🐞 Bug ⛨ Security

Description

resolve_all_records() swallows dns.promises.lookup() failures with an empty catch, so when
resolve4/resolve6 return no A/AAAA and lookup fails, callers receive empty IP lists
indistinguishable from “no records”. is_url_safe() ultimately treats this as safe because
is_hostname_resolve_to_internal_ip() returns false on empty results and is_parsed_url_safe() only
blocks on true.

Code

dist/helpers.js[R260-271]

+    if (A.length === 0 && AAAA.length === 0) {
+        try {
+            const lookups = await dns_1.promises.lookup(host, { all: true });
+            for (const entry of lookups) {
+                if (entry.family === 4)
+                    A.push(entry.address);
+                if (entry.family === 6)
+                    AAAA.push(entry.address);
+            }
+        }
+        catch { }
+    }

Evidence

The diff introduces an empty catch around dns.lookup() in resolve_all_records. In the same module,
is_hostname_resolve_to_internal_ip returns false when resolution yields no IPs, and
is_parsed_url_safe only blocks when is_hostname_resolve_to_internal_ip returns true—so lookup
failures can result in a URL being considered safe.

dist/helpers.js[253-273]
dist/helpers.js[289-343]
dist/helpers.js[396-406]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`resolve_all_records()` added a `dns.promises.lookup(host, { all: true })` fallback but wraps it in an empty `catch {}`. If `resolve4/resolve6` yield no A/AAAA and `lookup()` fails (transient DNS/OS resolver issue), the function returns empty arrays and `is_hostname_resolve_to_internal_ip()` returns `false`, which allows `is_parsed_url_safe()` / `is_url_safe()` to treat the hostname as non-internal.

### Issue Context
This library is an SSRF defense utility. When DNS resolution is failing, returning “not internal” can become a fail-open behavior.

### Fix Focus Areas
- dist/helpers.js[260-271]

### Suggested fix
1) Replace `catch { }` with `catch (e) { /* propagate or mark error */ }`.
2) Prefer fail-closed behavior: either rethrow the lookup error and have `is_hostname_resolve_to_internal_ip()` treat resolution errors as internal/unsafe, or return a flag (e.g., `{ A, AAAA, CNAME, resolutionError: true }`) that causes `is_hostname_resolve_to_internal_ip()` to return `true` (block) when all resolution strategies fail due to errors.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Higher DNS concurrency burst 🐞 Bug ➹ Performance

Description

resolve_all_records() now runs resolve4/resolve6/resolveCname concurrently via Promise.all,
increasing in-flight DNS requests per call from 1 to 3. is_hostname_resolve_to_internal_ip() calls
resolve_all_records multiple times per check (retries + extra resolutions), so this change can
increase resolver load and timeout likelihood under high traffic.

Code

dist/helpers.js[R255-259]

+    const [A, AAAA, CNAME] = await Promise.all([
+        dns_1.promises.resolve4(host).catch(() => []),
+        dns_1.promises.resolve6(host).catch(() => []),
+        dns_1.promises.resolveCname(host).catch(() => [])
+    ]);

Evidence

The diff shows resolve4/resolve6/resolveCname moved to Promise.all (parallel). The same file shows
is_hostname_resolve_to_internal_ip repeatedly calling resolve_all_records via resolveWithDelay and
additional CNAME checks, multiplying the per-check DNS load and making the new parallelism more
impactful.

dist/helpers.js[254-272]
dist/helpers.js[289-335]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`resolve_all_records()` was changed from sequential DNS calls to `Promise.all([...])`. While the total number of queries is similar, this increases burst concurrency (3 parallel DNS queries per call). Since `is_hostname_resolve_to_internal_ip()` invokes `resolve_all_records()` repeatedly, this can amplify resolver pressure and cause more timeouts under load.

### Issue Context
This code may run on request paths (URL safety checks). Increased DNS burstiness can become a reliability/perf bottleneck at scale.

### Fix Focus Areas
- dist/helpers.js[255-259]
- dist/helpers.js[298-335]

### Suggested fix
Either:
- Revert to sequential awaits inside `resolve_all_records()` (serialize resolve4/resolve6/resolveCname), or
- Introduce a small concurrency limiter/shared queue for DNS resolutions (process-wide), so parallelism is bounded even when many requests call these utilities concurrently.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}