Skip to content

Nested calls run before the parse-error guard, so a bad path value reaches the receiver as a zero value #154

Description

@crhntr

muxt generate emits a nested call before the guard that checks whether argument parsing failed, so the receiver method runs with a zero value when a path parameter does not parse.

For {{define "GET /article/{id} Show(ctx, Load(ctx, id))"}} and a receiver with Load(ctx context.Context, id int), the generated handler is:

idParsed, err := strconv.Atoi(request.PathValue("id"))
if err != nil {
	td.errList = append(td.errList, err)
	td.errStatusCode = http.StatusBadRequest
}
idPathParam := idParsed
result0, err := receiver.Load(ctx, idPathParam)   // runs with 0
if err != nil {
	td.errList = append(td.errList, err)
	td.errStatusCode = http.StatusInternalServerError
}
...
if len(td.errList) == 0 {                          // guards only the outer call
	td.result = receiver.Show(ctx, result0)
	td.okay = true
}

GET /article/not-an-int therefore calls Load(ctx, 0). Two consequences:

  • Application code runs on malformed input with a zero value, which may mean something (a sentinel row, an unscoped query, a write), and costs a backend round trip that should have been refused at parse time.
  • When the nested method returns an error for that zero value, the 400 becomes a 500 and the application's error text is appended to the body: status=500 body="ERR:strconv.Atoi: parsing \"not-an-int\": invalid syntax\nno such article".

The same method called at the top level (GET /article/{id} Load(ctx, id)) is generated inside if len(td.errList) == 0 and is never invoked. Only nested calls are emitted unguarded.

internal/generate/routes.go appends nestedCall.DefineStmts(), and DefineStmts forces :=, which is what keeps the call from being wrapped. receiverMethodCall already separates VarDecl, Assign and Check, so a fix can hoist var result0 T, then wrap the assignment and its error check in if len(td.errList) == 0 { ... }, as the top-level call already is. It needs to apply at every nesting level, and the generate snapshots will need rewriting with -update.

Reproduced with binaries built from f7e9368 (main) and from the tip of the PR stack (#146#153): the generated files are byte-identical apart from the version string, so this is long-standing behavior, not something the stack changed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions