COLDBOX-1465 Resolve event-selected implicit views only from declared view folders

system/web/Renderer.cfc

@@ -61,6 +61,10 @@ component
 		// Per-name locate caches, gated by viewDiscoveryCaching
 		variables.locateViewCache   = {};
 		variables.locateLayoutCache = {};
+		// Normalized folders implicit views may resolve from, per configured folder path
+		variables.implicitViewRoots = {}
+		// Implicit view lookups, gated by viewDiscoveryCaching
+		variables.implicitViewCache = {}
 		super.init();
 		return this;
 	}
@@ -963,6 +967,67 @@ component
 		return locateView( arguments.view );
 	}
 
+	/**
+	 * Locate a view chosen from the request's event name (implicit views). Unlike locateView(), only views inside the
+	 * declared view folders are returned: the views convention folder, the external views location and, for a module
+	 * event, the module's views folder. Templates anywhere else under the application root are ignored.
+	 *
+	 * @view   The view to locate
+	 * @module The module of the event, if any
+	 *
+	 * @return The view path, or an empty string when the view doesn't exist in a declared view folder
+	 */
+	function locateImplicitView( required view, module = "" ){
+		// This runs on every render that uses an implicit view, so the result is remembered like locateView()'s.
+		// A module lookup depends on the module's current views folder, so that folder is part of the key:
+		// a module reloaded with another views folder gets a fresh lookup.
+		var cacheKey = "|" & arguments.view
+		if ( len( arguments.module ) ) {
+			var moduleConfig = variables.modulesConfig[ arguments.module ]
+			cacheKey         = arguments.module & "|" & moduleConfig.mapping & "/" & moduleConfig.conventions.viewsLocation & "|" & arguments.view
+		}
+		if ( variables.isDiscoveryCaching && variables.implicitViewCache.keyExists( cacheKey ) ) {
+			return variables.implicitViewCache[ cacheKey ]
+		}
+
+		var viewPath = findImplicitView( arguments.view, arguments.module )
+		if ( variables.isDiscoveryCaching ) {
+			variables.implicitViewCache[ cacheKey ] = viewPath
+		}
+		return viewPath
+	}
+
+	/**
+	 * Find a view chosen from the request's event name in the declared view folders, without the lookup cache
+	 *
+	 * @view   The view to locate
+	 * @module The module of the event, if any
+	 *
+	 * @return The view path, or an empty string when the view doesn't exist in a declared view folder
+	 */
+	private string function findImplicitView( required view, module = "" ){
+		// Remove a lowercase extension like locateView() does, which returns the view as is when no template exists
+		var extension = right( arguments.view, 4 )
+		if ( !compare( extension, ".cfm" ) || !compare( extension, ".bxm" ) ) {
+			arguments.view = left( arguments.view, len( arguments.view ) - 4 )
+		}
+		var viewPath = len( arguments.module ) ? locateModuleView( arguments.view, arguments.module ) : locateView(
+			arguments.view
+		)
+		if ( viewPath == arguments.view ) {
+			return ""
+		}
+
+		// Compare case-sensitively: on a case-sensitive file system, folders that differ only by case are different
+		var templatePath = normalizeFilePath( cachedExpandPath( viewPath ) )
+		for ( var viewRoot in getImplicitViewRoots( arguments.module ) ) {
+			if ( !compare( left( templatePath, len( viewRoot ) ), viewRoot ) ) {
+				return viewPath
+			}
+		}
+		return ""
+	}
+
 	/**
 	 * Discover view+helper path locations.  The returned helper and view locations will have the appropriate extension.
 	 *
@@ -1063,15 +1128,74 @@ component
 		var cEvent = reReplaceNoCase( event.getCurrentEvent(), "^([^:.]*):", "" );
 
 		// Implicit views
-		if ( variables.controller.getSetting( name = "caseSensitiveImplicitViews", defaultValue = true ) ) {
-			event.setView( replace( cEvent, ".", "/", "all" ) );
-		} else {
-			event.setView( lCase( replace( cEvent, ".", "/", "all" ) ) );
+		var implicitView = replace( cEvent, ".", "/", "all" )
+		if ( !variables.controller.getSetting( name = "caseSensitiveImplicitViews", defaultValue = true ) ) {
+			implicitView = lCase( implicitView )
 		}
 
+		// The view must exist in a declared view folder, never elsewhere under the app root
+		if ( !len( locateImplicitView( implicitView, event.getCurrentModule() ) ) ) {
+			throw(
+				message = "The implicit view [#implicitView#] for the event [#event.getCurrentEvent()#] was not found in the views folders.",
+				detail  = "Implicit views resolve only from the views folder, the external views location or a module's views folder. Use event.setView() to render a template from another location.",
+				type    = "Renderer.ImplicitViewNotFoundException"
+			)
+		}
+
+		event.setView( implicitView )
+
 		return this;
 	}
 
+	/**
+	 * The folders implicit views may resolve from, as normalized absolute paths ending in /
+	 *
+	 * @module The module of the event, if any
+	 */
+	private array function getImplicitViewRoots( module = "" ){
+		var roots = [ "/#variables.appMapping#/#variables.viewsConvention#" ]
+		if ( len( variables.viewsExternalLocation ) ) {
+			roots.append( variables.viewsExternalLocation )
+		}
+		// Read the module's folder on every lookup, a module reload can change it
+		if ( len( arguments.module ) ) {
+			roots.append(
+				"#variables.modulesConfig[ arguments.module ].mapping#/#variables.modulesConfig[ arguments.module ].conventions.viewsLocation#"
+			)
+		}
+
+		var viewRoots = []
+		for ( var root in roots ) {
+			if ( !variables.implicitViewRoots.keyExists( root ) ) {
+				variables.implicitViewRoots[ root ] = normalizeFilePath(
+					cachedExpandPath( reReplace( root, "//", "/", "all" ) )
+				) & "/"
+			}
+			viewRoots.append( variables.implicitViewRoots[ root ] )
+		}
+		return viewRoots
+	}
+
+	/**
+	 * Normalize an absolute file path: use / separators and remove . and .. segments, without resolving symlinks
+	 *
+	 * @path The absolute path
+	 */
+	private string function normalizeFilePath( required string path ){
+		var segments = []
+		for ( var segment in listToArray( replace( arguments.path, "\", "/", "all" ), "/" ) ) {
+			if ( segment == ".." ) {
+				if ( segments.len() ) {
+					segments.deleteAt( segments.len() )
+				}
+			} else if ( segment != "." ) {
+				segments.append( segment )
+			}
+		}
+		// Keep the leading slash of a *nix path, a Windows path starts with its drive
+		return ( left( arguments.path, 1 ) == "/" ? "/" : "" ) & segments.toList( "/" )
+	}
+
 	/**
 	 * Incorporate a rendering region into the arguments struct
 	 *

system/web/services/HandlerService.cfc

@@ -397,16 +397,16 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {
 		if ( find( ":", arguments.event ) ) {
 			// Validate that it is a valid module, else it is an invalid view.
 			if ( structKeyExists( variables.modules, targetModule ) ) {
-				targetView = renderer.locateModuleView( cEvent, targetModule );
+				targetView = renderer.locateImplicitView( cEvent, targetModule );
 			} else {
 				return false;
 			}
 		} else {
-			targetView = renderer.locateView( cEvent );
+			targetView = renderer.locateImplicitView( cEvent );
 		}
 
-		// locateView/locateModuleView return a path with .cfm/.bxm extension only when
-		// the file was verified to exist — no need for a redundant filesystem call here.
+		// locateImplicitView returns a path with .cfm/.bxm extension only when the file was verified
+		// to exist in a declared view folder — no need for a redundant filesystem call here.
 		if ( right( targetView, 4 ) == ".cfm" || right( targetView, 4 ) == ".bxm" ) {
 			arguments.ehBean.setViewDispatch( true );
 			return true;

test-harness/handlers/implicitViews.cfc

@@ -0,0 +1,19 @@
+/**
+ * Actions for the implicit view specs in tests/specs/integration/ImplicitViewsSpec.cfc
+ */
+component {
+
+	// Sets no view: views/implicitViews/withView.cfm renders implicitly
+	function withView( event, rc, prc ){
+	}
+
+	// Sets no view: the only matching template is implicitViews/noView.cfm in the app root
+	function noView( event, rc, prc ){
+	}
+
+	// Selects the app root template explicitly, which is still allowed
+	function explicitRoot( event, rc, prc ){
+		event.setView( "implicitViews/noView" )
+	}
+
+}

test-harness/implicitViews/noView.cfm

@@ -0,0 +1,2 @@
+<!--- App root template for implicitViews.noView: only rendered when a handler selects it explicitly. See ImplicitViewsSpec --->
+ROOT-ONLY-NO-VIEW

test-harness/main/rootonlyaction.cfm

@@ -0,0 +1,2 @@
+<!--- App root template for main.rootonlyaction (no such action): never an implicit view. See ImplicitViewsSpec --->
+ROOT-ONLY-ACTION

test-harness/rootonlyview.cfm

@@ -0,0 +1,2 @@
+<!--- App root template, outside the views folders: never an implicit view. See ImplicitViewsSpec --->
+ROOT-ONLY-VIEW

test-harness/views/implicitViews/withView.cfm

@@ -0,0 +1 @@
+IMPLICIT-WITH-VIEW-OK

tests/specs/integration/EventCachingSpec.cfc

@@ -576,7 +576,8 @@
 				} );
 
 				it( "keeps the serve-side and store-side keys in sync even with handlerCaching off", function(){
-					var handlerService = controller.getHandlerService()
+					var handlerService  = controller.getHandlerService()
+					var originalCaching = handlerService.getHandlerCaching()
 					handlerService.setHandlerCaching( false )
 
 					try {
@@ -593,7 +594,9 @@
 						expect( storeKey ).toInclude( "delta-present" )
 						expect( serveKey ).toBe( storeKey )
 					} finally {
-						handlerService.setHandlerCaching( true )
+						// Put back the setting the spec found. The test app runs with handler caching off, so
+						// turning it on here would leave every later spec with cached handler lookups.
+						handlerService.setHandlerCaching( originalCaching )
 					}
 				} );
 

tests/specs/integration/ImplicitViewsSpec.cfc

@@ -0,0 +1,86 @@
+/**
+ * Implicit views resolve only from the declared view folders: the views convention, the external views location
+ * and module views. Templates elsewhere under the app root are never chosen from the event name.
+ * Fixtures: test-harness/handlers/implicitViews.cfc, test-harness/rootonlyview.cfm,
+ * test-harness/main/rootonlyaction.cfm, test-harness/implicitViews/noView.cfm, test-harness/views/implicitViews/
+ * The root-only fixtures are lowercase because view dispatch lowercases the event, so on a case-sensitive file
+ * system the old lookup finds them too.
+ */
+component extends="tests.resources.BaseIntegrationTest" {
+
+	function run(){
+		describe( "Implicit views", function(){
+			beforeEach( function( currentSpec ){
+				setup()
+				// The harness leaves implicit views off. The renderer reads the setting, the handler service caches it.
+				variables.handlerService  = getController().getHandlerService()
+				variables.originalSetting = getController().getSetting( "ImplicitViews" )
+				getController().setSetting( "ImplicitViews", true )
+				prepareMock( variables.handlerService ).$property( "implicitViews", "variables", true )
+				// Don't cache handler lookups made with implicit views on: a later spec with them off would get them
+				variables.originalHandlerCaching = variables.handlerService.getHandlerCaching()
+				variables.handlerService.setHandlerCaching( false )
+			} )
+
+			afterEach( function( currentSpec ){
+				getController().setSetting( "ImplicitViews", variables.originalSetting )
+				variables.handlerService.$property(
+					"implicitViews",
+					"variables",
+					variables.originalSetting
+				)
+				variables.handlerService.setHandlerCaching( variables.originalHandlerCaching )
+			} )
+
+			describe( "dispatch from a declared view folder", function(){
+				it( "renders a view in the views folder for a missing handler", function(){
+					var e = execute( event = "simpleview", renderResults = true )
+					expect( e.getRenderedContent() ).toInclude( "I am a simple view" )
+				} )
+
+				it( "renders a view in the external views location for a missing handler", function(){
+					var e = execute( event = "externalview", renderResults = true )
+					expect( e.getRenderedContent() ).toInclude( "I am an external view" )
+				} )
+
+				it( "renders a module view for a missing module handler", function(){
+					var e = execute( event = "moduleLookup:module-level", renderResults = true )
+					expect( e.getRenderedContent() ).toInclude( "This is a module level view" )
+				} )
+
+				it( "renders the views folder view of an action that sets no view", function(){
+					var e = execute( event = "implicitViews.withView", renderResults = true )
+					expect( e.getRenderedContent() ).toInclude( "IMPLICIT-WITH-VIEW-OK" )
+				} )
+			} )
+
+			describe( "templates outside the declared view folders", function(){
+				it( "are not dispatched for a missing handler", function(){
+					var e = execute( event = "rootonlyview", renderResults = true )
+					expect( e.getPrivateValue( "invalidevent", "" ) ).toBe( "rootonlyview" )
+					expect( e.getStatusCode() ).toBe( 404 )
+					expect( e.getRenderedContent() ).notToInclude( "ROOT-ONLY-VIEW" )
+				} )
+
+				it( "are not dispatched for a missing action", function(){
+					var e = execute( event = "main.rootonlyaction", renderResults = true )
+					expect( e.getPrivateValue( "invalidevent", "" ) ).toBe( "main.rootonlyaction" )
+					expect( e.getStatusCode() ).toBe( 404 )
+					expect( e.getRenderedContent() ).notToInclude( "ROOT-ONLY-ACTION" )
+				} )
+
+				it( "are not rendered for an action that sets no view", function(){
+					expect( function(){
+						execute( event = "implicitViews.noView", renderResults = true )
+					} ).toThrow( type = "Renderer.ImplicitViewNotFoundException" )
+				} )
+
+				it( "still render when handler code selects them explicitly", function(){
+					var e = execute( event = "implicitViews.explicitRoot", renderResults = true )
+					expect( e.getRenderedContent() ).toInclude( "ROOT-ONLY-NO-VIEW" )
+				} )
+			} )
+		} )
+	}
+
+}

tests/specs/web/RendererTest.cfc

@@ -211,6 +211,52 @@ component extends="tests.resources.BaseIntegrationTest" {
 				} );
 			} );
 
+			describe( "Implicit view lookup", () => {
+				it( "finds a view in the views folder", () => {
+					expect( renderer.locateImplicitView( "simpleview" ) ).toInclude( "simpleview.cfm" )
+				} )
+
+				it( "finds a view in the external views location", () => {
+					expect( renderer.locateImplicitView( "externalview" ) ).toInclude( "testViews/externalview.cfm" )
+				} )
+
+				it( "finds a module view", () => {
+					expect( renderer.locateImplicitView( "module-level", "moduleLookup" ) ).toInclude(
+						"moduleLookup/views/module-level.cfm"
+					)
+				} )
+
+				it( "ignores an app root template that locateView() finds", () => {
+					expect( renderer.locateView( "rootonlyview" ) ).toInclude( "rootonlyview.cfm" )
+					expect( renderer.locateImplicitView( "rootonlyview" ) ).toBe( "" )
+				} )
+
+				it( "returns an empty string for a missing view", () => {
+					expect( renderer.locateImplicitView( "no/such/view" ) ).toBe( "" )
+				} )
+
+				it( "returns an empty string for a missing view with an extension", () => {
+					expect( renderer.locateImplicitView( "/cbTestHarness/views/missing.CFM" ) ).toBe( "" )
+				} )
+
+				it( "uses a module's current views folder", () => {
+					var conventions   = renderer.$getProperty( "modulesConfig", "variables" ).moduleLookup.conventions
+					var viewsLocation = conventions.viewsLocation
+					expect( renderer.locateImplicitView( "module-level", "moduleLookup" ) ).toInclude(
+						"moduleLookup/views/module-level.cfm"
+					)
+					try {
+						// As after a module reload with another views folder
+						conventions.viewsLocation = "layouts"
+						expect( renderer.locateImplicitView( "module-level", "moduleLookup" ) ).toInclude(
+							"moduleLookup/layouts/module-level.cfm"
+						)
+					} finally {
+						conventions.viewsLocation = viewsLocation
+					}
+				} )
+			} )
+
 			feature( "ColdBox can render view collections", () => {
 				beforeEach( ( currentSpec ) => {
 					aUsers = [