Skip to content

feat: support H2C and QUICv2 sniffing - #3036

Merged
wwqgtxx merged 5 commits into
MetaCubeX:Alphafrom
MakostaDev:refactor-sniffer
Jul 28, 2026
Merged

feat: support H2C and QUICv2 sniffing#3036
wwqgtxx merged 5 commits into
MetaCubeX:Alphafrom
MakostaDev:refactor-sniffer

Conversation

@MakostaDev

@MakostaDev MakostaDev commented Jul 26, 2026

Copy link
Copy Markdown

When I was looking into the mihomo code, I discovered that the sniffer doesn't support HTTP/2 or QUICv2, so I decided to submit this PR. Since the existing HTTP sniffer didn't have a clean structure to extend, I rewrote it from scratch.

Changes:

  1. Added HTTP/2 and wrote the HTTP/1.x sniffer from scratch.
  2. Added QUICv2 (draft-29, v1 and v2 share the same structure but differ in content).
  3. In the QUIC sniffer, labels are now expanded only once, instead of being expanded on every incoming packet.
  4. I used constant names and comments to explain what each part is responsible for, so that anyone else can easily read the code.
  5. Added tests to verify HTTP/2 and QUICv2 sniffing.

@wwqgtxx

wwqgtxx commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Thank you for your contribution; however, there are some implementation issues:

Regarding quicPacketSender: it already possesses an RWMutex, so why add multiple atomic.Pointer components? Do not assume that passing test -race implies the absence of thread-safety issues simply because atomic operations are used; safety across multiple variables still requires locking mechanisms.

As for the HTTP component, why use an unsafe function like utils.ImmutableBytesFromString? Such functions should be reserved for performance-critical areas rather than being misused here. Furthermore, a comparison between the old and new code reveals that the new implementation rejects lowercase HTTP methods; regardless of RFC specifications, there is no need to remove support for this. The handling of the protocol is also peculiar: a new value is introduced, yet in practice, all unknown values ​​are effectively treated as HTTP.

@wwqgtxx

wwqgtxx commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

I still fail to see the purpose of closeOnce in the QUIC implementation, or the significance of the changes made to (http *HTTPSniffer) Protocol() string.

@wwqgtxx wwqgtxx changed the title feat(sniffer): add HTTP/2 and QUICv2 sniffing feat: support H2C and QUICv2 sniffing Jul 27, 2026
@wwqgtxx

wwqgtxx commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Issues requiring fixes:

  1. Writing to http.version causes a data race (newly introduced; needs fixing).

    func (http *HTTPSniffer) SniffData(b []byte) (string, error) {
      if !bytes.HasPrefix(b, h2ClientPreface) {
        http.version = HTTP1   // ← Writing to a shared instance

    The sniffer instance is created once during configuration and stored in the dispatcher.sniffers map (dispatcher.go:285). Subsequently, each connection calls s.SniffData(bytes) within its own goroutine (dispatcher.go:212). Consequently, multiple goroutines concurrently write to the same field, triggering a -race warning.

    The original code avoided this issue by writing the version to a discarded temporary struct (_ = &HTTPSniffer{version: HTTP1}, which was essentially dead code). Furthermore, the version field is actually only read within Protocol() (returning strings like "http" or "h2c"). The PR effectively turned it into "global state shared across connections"—even ignoring the race condition, the semantics are incorrect: if Connection A is h2c, Connection B might retrieve h2c when calling Protocol().

    Recommendation: SniffData should not modify the receiver's state. To distinguish versions, keep Protocol() static or return the version as a return value or part of the log output.

  2. H2 detection relies on the preface and the HEADERS frame being in the same buffer, without requesting additional data.
    sniffHTTP2 requires bytes.HasPrefix(b, h2ClientPreface) and for the HEADERS frame to have fully arrived. However, the dispatcher only calls Peek(conn.Buffered())—retrieving only the currently buffered bytes—without guaranteeing the inclusion of the complete preface (24 bytes) plus SETTINGS and HEADERS frames.

    The framework provides a mechanism for this: the TLS sniffer returns &errNeedAtLeastData{length: N}, and upon receiving it, the dispatcher calls Peek(e.length) to wait for more data (dispatcher.go:213-219). This PR fails to utilize this mechanism entirely (errNeedAtLeastData appears zero times in the patch), opting instead to return ErrNoClue in all cases. Consequence: H2C fails silently during data fragmentation, and the failure counts towards the skipList (after 5 failures, the target is no longer sniffed). The same applies to H1—if bytes.Cut(b, "\r\n") fails to find the delimiter, it immediately returns ErrNoClue instead of waiting for more data.

    This is a functional correctness issue, not a theoretical one: typical H2C client behavior involves sending the preface and the first HEADERS frame separately.

  3. parseHost rejects IPv6 too broadly, and the port stripping logic is buggy:

    if h[0] == '[' { return "", errHostIsIP } // Checks only the first character
    if i := bytes.LastIndexByte(h, ':'); i >= 0 { h = h[:i] } // Unconditionally strips content after the last colon

    For [::1]:443, the first character is [ → correctly rejected. However, for a raw IPv6 address (e.g., ::1—non-compliant in H2 :authority headers but possible in practice), the first character isn't [, so it proceeds to port stripping: LastIndexByte(':') truncates ::1 to ::netip.ParseAddr("::") succeeds → returns errHostIsIP. The result happens to be correct, but the logic path is flawed; a case like ::1:2 might slip through.
    The original implementation used net.SplitHostPort, which is more robust. Switching to manual byte manipulation saved allocations but introduced correctness risks.
    Recommendation: Continue using net.SplitHostPort to handle cases with ports.

  4. parseHeaderHostH1 calls bytes.ToLower on the entire buffer:

    _, b, found := bytes.Cut(bytes.ToLower(b), []byte("\r\nhost:"))

    ToLower allocates memory and copies the entire buffer. The buffer contains the full output of conn.Buffered(), which could include a POST body of several kilobytes. The original implementation only applied ToLower to the header key (after splitting line-by-line), resulting in much smaller allocations. The sniffer runs whenever a connection is established, making this a hot path.

    Recommendation: Use a case-insensitive search (or split by \r\n first and lowercase only the key) to avoid a full buffer copy. Side note: This also forces the returned host string to lowercase. Since the original implementation also used lowercase, the behavior remains consistent—so it’s not a regression. It is worth noting, however, that the domain portion of the Host header is theoretically case-insensitive, so this shouldn't cause issues.

  5. QUIC: expandLabels uses sync.Once for caching, but the labels depend on destConnID.

    func (q *quicPacketSender) expandLabels(destConnID []byte, s *quicStructure) quicLabels {
      q.labelsOnce.Do(func() { ... derive using destConnID ... })
      return q.labels
    }

    The PR description frames this as an optimization ("labels are now expanded only once"). For multiple Initial packets on the same QUIC connection, the DCID remains the same; thus, caching is correct, and the optimization is valid and valuable (saving four HKDF operations per packet).

    However, there is a caveat: if the client changes the DCID within the same packetSender lifecycle (e.g., retransmitting an Initial packet with a new DCID after receiving a Retry), the cached labels will mismatch, causing decryption to fail. In this scenario, sniffing fails (gracefully degrading to "no result") rather than crashing—which is acceptable, though it might be worth confirming if the author cares about the Retry scenario.

    One thing done right: The PR also changes iv to bytes.Clone(labels.iv) before performing the XOR operation. This is essential because the labels are now cached and reused; an in-place XOR would corrupt subsequent packets. The author's awareness of this coupling is a plus.

  6. QUIC: The return statement was removed when err != nil in Send.

    err := q.readQUICData(current.Data())
    if err != nil {
      q.close()
      // There used to be a return here
    }

    Since there are no subsequent statements, the behavior remains equivalent. However, this obscures the intent to "stop processing on error," and adding code to the end of the function later could introduce bugs. It is a matter of coding style; retaining the return is recommended.

@MakostaDev

MakostaDev commented Jul 27, 2026

Copy link
Copy Markdown
Author

Thanks for the review!

While working on this, I noticed a few things that could be cleaned up - they're unrelated to the H2C/QUICv2 sniffing changes, but I figured I'd raise them since I'm already touching this code:

  1. Removing redundant Protocol() and SupportNetwork() overrides.
    BaseSniffer already implements SupportNetwork() correctly by returning bs.supportNetworkType, so the overrides in TLSSniffer, HTTPSniffer and QUICSniffer are dead code. Similarly, Protocol() could be moved to BaseSniffer by storing the protocol type there:
func (bs *BaseSniffer) Protocol() string {  
    return strings.ToLower(bs.protocol.String())  
}

sniffer.Type.String() already returns "TLS" / "HTTP" / "QUIC", so this would just require passing the type into NewBaseSniffer and removing the per-sniffer overrides.

  1. Change default port ranges
    The current defaults are 443 for TLS/QUIC and 80 for HTTP. The docs example includes 8080-8880 for HTTP and 8443 for TLS/QUIC. It might be worth aligning the defaults with the docs.

What do you think?

@wwqgtxx

wwqgtxx commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

This PR has already accomplished what it was supposed to; the remaining refactoring is not necessary.

@wwqgtxx
wwqgtxx merged commit e889b68 into MetaCubeX:Alpha Jul 28, 2026
@MakostaDev
MakostaDev deleted the refactor-sniffer branch July 28, 2026 06:24
ruattd pushed a commit to ruattd/swihomo-core that referenced this pull request Jul 28, 2026
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