e2b-dev / e2b-dev/runtime

nfsproxy: middleware chain hides CachingHandler and disables directory verifier caching

Open
#3,552 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
1.6k
Forks
438
PR merge metrics
No merged PRs in 30d

Description

Symptom

The NFS proxy's directory verifier cache is silently bypassed.

Paginated READDIR and READDIRPLUS requests cannot reuse the directory listing associated with the cookie verifier returned by the first page. Each subsequent request reads and sorts the directory again.

If the directory changes between pages, the newly calculated verifier differs from the verifier supplied by the client, causing go-nfs to return NFS3ERR_BAD_COOKIE:

if obj.Cookie > 0 && obj.CookieVerif > 0 && verifier != obj.CookieVerif {
    return &NFSStatusError{NFSStatusBadCookie, nil}
}

Even when the directory does not change, the repeated ReadDir calls add unnecessary filesystem work for large paginated listings.

Root cause

The proxy creates a helpers.CachingHandler before applying its middleware:

handler = helpers.NewCachingHandler(handler, cacheLimit)

if config.Tracing {
    handler = tracing.WrapWithTracing(handler, config)
}

if config.Metrics {
    handler = metrics.WrapWithMetrics(handler, config)
}

if config.Logging {
    handler = logged.WrapWithLogging(ctx, handler, config)
}

handler = recovery.WrapWithRecovery(ctx, handler)

go-nfs checks the outermost handler for the optional nfs.CachingHandler interface:

if vh, ok := userHandle.(CachingHandler); verifier != 0 && ok {
    entries := vh.DataForVerifier(path, verifier)
    if entries != nil {
        return entries, verifier, nil
    }
}

The tracing, metrics, logging, and recovery handlers only implement nfs.Handler. They do not expose or forward:

VerifierFor(path string, contents []fs.FileInfo) uint64
DataForVerifier(path string, verifier uint64) []fs.FileInfo

Because recovery middleware is always installed, the outermost handler never satisfies nfs.CachingHandler, regardless of whether tracing, metrics, or logging are enabled.

As a result, go-nfs never reaches the verifier cache maintained by helpers.CachingHandler:

func (c *CachingHandler) VerifierFor(path string, contents []fs.FileInfo) uint64 {
    id := hashPathAndContents(path, contents)
    c.activeVerifiers.Add(id, verifier{path, contents})
    return id
}

func (c *CachingHandler) DataForVerifier(path string, id uint64) []fs.FileInfo {
    if cache, ok := c.activeVerifiers.Get(id); ok {
        return cache.contents
    }

    return nil
}

Why it matters

NFS directory cookies are indexes into the sorted directory listing:

cookie := uint64(i + 2)

For pagination to remain consistent, subsequent requests should use the same listing that produced the original cookie verifier.

Without verifier caching:

  • Every page performs another ReadDir and sort.
  • Insertions or removals can shift cookie indexes between pages.
  • Directory changes can cause NFS3ERR_BAD_COOKIE.
  • Large directory listings perform unnecessary repeated filesystem operations.

Proposed fix

Preserve nfs.CachingHandler through the complete middleware chain:

  • Implement VerifierFor and DataForVerifier on the tracing handler.
  • Implement VerifierFor and DataForVerifier on the metrics handler.
  • Implement VerifierFor and DataForVerifier on the logging handler.
  • Implement VerifierFor and DataForVerifier on the recovery handler.
  • Forward both calls to the wrapped handler when it implements nfs.CachingHandler.
  • Add compile-time assertions that each middleware implements nfs.CachingHandler.
  • Preserve panic recovery around verifier operations in the recovery handler.
  • Add a test covering the complete middleware chain and verifying that arguments and return values reach the underlying caching handler.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the tracing, metrics, logging, and recovery middleware implementations and helpers.CachingHandler. Trace how the middleware chain is assembled, then inspect the existing middleware tests before adding coverage for the complete chain. Done means verifier calls and their arguments and return values reach the underlying caching handler, with recovery behavior preserved.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
64/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.