Skip to content

add custom dns settings#337

Merged
SuperCoolPencil merged 8 commits intomainfrom
fix-certain-downloads
Apr 9, 2026
Merged

add custom dns settings#337
SuperCoolPencil merged 8 commits intomainfrom
fix-certain-downloads

Conversation

@SuperCoolPencil
Copy link
Copy Markdown
Member

@SuperCoolPencil SuperCoolPencil commented Apr 9, 2026

  • feat: include request headers in download debug logs
  • refactor: enhance download interception with redirect tracking, cookie-based authentication, and improved filename resolution for Firefox
  • feat: add custom DNS server configuration support and utility for dialer resolution
  • feat: custom dns settings

Greptile Summary

This PR adds custom DNS server configuration (plumbed through the Go download engine via ConfigureDialer), enhances both browser extensions with redirect chain tracking and cookie-based authentication, and improves Firefox filename resolution via polling. The Go-side changes are clean: the dns.go utility correctly uses a separate inner dialer to avoid recursive resolution, and the probe client cache key was fixed to use quoted values to prevent separator collision.

Confidence Score: 5/5

Safe to merge; the single finding is a P2 edge case in the Firefox duplicate-download notification path that falls back gracefully.

All P0/P1 concerns from previous review rounds are addressed (recursive DNS fixed, cache key collision fixed). The only remaining finding is a P2: filename polling in Firefox's duplicate path fires after erase, but the fallback to URL-based extraction means functionality is not broken, just slightly degraded for an uncommon code path.

extension-firefox/background.js — resolveFilename ordering in duplicate handler

Vulnerabilities

  • Cookie forwarding logs only the boolean presence of cookies (!!cookieString), not the actual values — no accidental secret exposure in logs.
  • getCookiesAsHeader reads from the browser's own cookie store (scoped to the URL) and forwards to a locally-running Surge instance, which is the intended trust boundary.
  • No security concerns identified in the Go DNS utility or config plumbing.

Important Files Changed

Filename Overview
internal/utils/dns.go New DNS utility that configures a net.Dialer with a custom resolver; uses a clean inner dialer to avoid recursive resolution; correctly falls back to port 53 when missing.
internal/processing/probe.go Adds CustomDNS support to probe transport; cache key now uses fmt.Sprintf quoting to avoid separator collision; ConfigureDialer wired into newProbeTransport cleanly.
extension-firefox/background.js Adds redirect chain tracking, cookie-based auth, and Firefox-specific filename polling; minor issue: resolveFilename is called after download erase in the duplicate path, defeating the polling.
extension-chrome/background.js Adds redirect chain tracking and cookie-based auth for Chrome; Chrome uses onDeterminingFilename so filename is already resolved before cancel/erase — no ordering issue.
internal/config/settings.go Adds CustomDNS field to NetworkSettings and RuntimeConfig; DefaultSettings initialises ProxyURL and CustomDNS to empty strings; conversion propagated correctly.
internal/download/manager.go ProbeMirrorsWithProxy call updated to pass a RuntimeConfig struct carrying both ProxyURL and CustomDNS instead of a bare string.

Sequence Diagram

sequenceDiagram
    participant B as Browser (FF/Chrome)
    participant EXT as Extension BG Script
    participant RC as redirectChains Map
    participant CS as Cookie Store
    participant S as Surge Server
    participant DNS as Custom DNS

    B->>EXT: onBeforeRedirect(url → finalUrl)
    EXT->>RC: set(originUrl, {finalUrl, ts})

    B->>EXT: onCreated / onDeterminingFilename
    EXT->>RC: resolveDownloadUrl(originalUrl)
    RC-->>EXT: finalUrl
    EXT->>CS: getCookiesAsHeader(finalUrl)
    CS-->>EXT: cookieString
    EXT->>S: POST /download {url: finalUrl, headers: {Cookie: ...}}
    S->>DNS: resolve host via CustomDNS (if configured)
    DNS-->>S: IP
    S-->>EXT: {success: true}
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: extension-firefox/background.js
Line: 811-818

Comment:
**`resolveFilename` called after download erase**

The download is cancelled and erased (lines 811–813) before `resolveFilename` is invoked on line 818. The polling loop inside `resolveFilename` calls `browser.downloads.search({ id: downloadItem.id })`, which returns an empty result once the item is erased, so the polling always falls through to the URL-based last-resort extraction — potentially giving a less accurate display name in the duplicate notification. Move the `resolveFilename` call before the cancel/erase block:

```suggestion
    // Resolve filename before erasing so polling can still find the item
    const filename = await resolveFilename(downloadItem);

    // Cancel the browser download
    try {
      await browser.downloads.cancel(downloadItem.id);
      await browser.downloads.erase({ id: downloadItem.id });
    } catch (e) {
      console.log('[Surge] Error canceling duplicate:', e);
    }
    
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (2): Last reviewed commit: "Potential fix for pull request finding '..." | Re-trigger Greptile

SuperCoolPencil and others added 3 commits April 9, 2026 12:48
…tion or class'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
@SuperCoolPencil SuperCoolPencil merged commit 9442980 into main Apr 9, 2026
15 checks passed
@SuperCoolPencil SuperCoolPencil deleted the fix-certain-downloads branch April 9, 2026 07:28
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.

1 participant