Skip to content

Empty Verb list error and missing docblocks #54

Description

@reubenromandy

Title

Unrestricted-verb routes (route(pattern, event) with no verbs) are documented with zero HTTP methods and no docblock metadata

Environment

  • cbswagger 3.1.3+14
  • ColdBox 8.1.0
  • Lucee 6.2.8.20

Summary

RoutesParser.cfc's addPathFromRouteConfig() silently produces an empty path object ({}, zero HTTP methods) for any route registered the plain way — route( "/auth/login", "auth.login" ) — with no explicit verb restriction. This is ColdBox's default/most common route style (an unrestricted route dispatches the same action regardless of HTTP method), so in a typical app the majority of routes are affected, not an edge case.

There are two independent bugs in the same function, both triggered by the same route shape (verbs = "", ColdBox's "any verb" convention).

Bug 1 — verbs="" gets converted into an empty action struct, which then documents 0 methods

// RoutesParser.cfc, ~line 277
if ( !isStruct( actions ) && structKeyExists( arguments.routeConfig, "verbs" ) ) {
    var targetAction = len( arguments.routeConfig.event ) ? listLast( arguments.routeConfig.event, "." ) : arguments.routeConfig.action;
    actions          = arguments.routeConfig.verbs
        .listToArray()
        .reduce( function( acc, verb ){
            acc[ verb ] = targetAction;
            return acc;
        }, {} );
}

For a route with verbs = "" (unrestricted — every bare route(pattern, event) registration has this), "".listToArray() is an empty array, so .reduce() returns the untouched initial value {} — an empty struct. Downstream, isStruct(actions) is now true (previously it was the plain event string), so the code takes the "verb-map" branch and loops for (var methodList in actions) zero times. Net effect: the route is documented with zero HTTP methods instead of falling through to the "unrestricted, document all default methods" branch.

Minimal fix: guard on len(arguments.routeConfig.verbs) too, so only a genuinely verb-restricted single-event route (verbs="GET", etc.) takes this conversion path:

if ( !isStruct( actions ) && structKeyExists( arguments.routeConfig, "verbs" ) && len( arguments.routeConfig.verbs ) ) {

Bug 2 — the "default methods" branch never looks at route.event, so docblock metadata never applies to this route style

// ~line 313, the else branch taken once bug 1 is fixed
} else {
    for ( var methodName in getOpenAPIUtil().defaultMethods() ) {
        path[ lCase( methodName ) ] = getOpenAPIUtil().newMethod();
        appendPathParams( pathKey = arguments.pathKey, method = path[ lCase( methodName ) ] );
        if ( len( actions ) && !isNull( arguments.handlerMetadata ) ) {
            appendFunctionInfo(
                methodName      = methodName,
                method          = path[ lCase( methodName ) ],
                functionName    = actions,
                handlerMetadata = arguments.handlerMetadata
            );
        }
    }
}

actions here is bound to route.action (the verb-map struct field), which is "" for an ordinary bare route(pattern, event) registration — the actual single action name lives on route.event (e.g. "auth.login") instead. So len(actions) is always 0 for this entire route style, and appendFunctionInfo() — the call that pulls a handler action's real docblock hint into the OpenAPI summary/description — never runs at all. This silently defeats cbswagger's own headline feature (auto-generating docs from existing handler comments) for every unrestricted-verb route.

Note getHandlerMetadata() (just above, in the same file) already derives the handler's invocation path from route.event the same way — the fix mirrors that:

} else {
    var singleActionName = len( actions ) ? actions : (
        structKeyExists( arguments.routeConfig, "event" ) && len( arguments.routeConfig.event ) ? listLast( arguments.routeConfig.event, "." ) : ""
    );
    for ( var methodName in getOpenAPIUtil().defaultMethods() ) {
        path[ lCase( methodName ) ] = getOpenAPIUtil().newMethod();
        appendPathParams( pathKey = arguments.pathKey, method = path[ lCase( methodName ) ] );
        if ( len( singleActionName ) && !isNull( arguments.handlerMetadata ) ) {
            appendFunctionInfo(
                methodName      = methodName,
                method          = path[ lCase( methodName ) ],
                functionName    = singleActionName,
                handlerMetadata = arguments.handlerMetadata
            );
        }
    }
}

Repro

// Router.cfc
route( "/hello", "greeter.sayHello" );
// handlers/Greeter.cfc
component {
    /**
     * A friendly hello.
     */
    function sayHello( event, rc, prc ){
        event.renderData( data = "hi" );
    }
}

Hit /cbswagger (with "routes": [""] or any prefix that matches /hello) — the resulting paths["/hello"] is {}. With both patches applied, it correctly comes back with all 6 default HTTP methods, and the sayHello() docblock's hint populates summary/description on each one.

Impact

Any ColdBox app that predominantly uses the plain route(pattern, event) shorthand (arguably the most common style in ColdBox apps) gets effectively no useful OpenAPI output from cbswagger out of the box — routes appear with no methods and no docs, and the only way to get anything is to rewrite every route to the .withHandler().toAction({verb:action}) verb-map style purely to work around this, which is an unrelated and fairly invasive change for existing apps.

Happy to open a PR with the two diffs above if that's preferred over a maintainer picking it up.

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