Page not found errors for certain types
Description
Environment
Activity
Justin Carter November 23, 2018 at 5:58 AM
Thanks so much for the detailed bug report Sean
> Create duplicates of the structs in webtopTreeChildRows and modify those instead of the referenced struct
When webtopTreeChildRows calls the oTree.getLeaves(), it in turn calls fapi.getContentObject(), which in Core's default object broker would be a struct in memory. It seems that if the object is not in object broker and is loaded for the very first time via the code in webtopTreeChildRows then it will put the struct into object broker and then return the same struct back to the calling code by reference – where it is then modified, which affects the struct in object broker.
In this case I think the fix is to duplicate() the struct before putting it into the object broker cache (in cacheAdd()), which means the object broker cache will definitely contain an unmodified struct, and the object returned to the calling code can be modified if need be and won't affect future retrievals from the cache.
This seems like it would have affected any object that was loaded for the first time and then modified within the same request, since the object reference would have been shared from object broker right back through to where the object was used.
> Or instead of setting status to an empty string, set it to approved. If it doesn't extend versions then it should be considered approved anyway
If the original object is no longer tainted when the status field is set in webtopTreeChildRows then future reads from object broker won't cause any issues, so we could probably leave this logic as is.
> Modify navajo/display.cfm to be aware that status might be an empty string and if it is, assume it is approved
I've searched core and there doesn't seem to be any instances of status being set to an empty string so for now I would probably leave this logic as is too.
Got a weird one here.
We've been getting some periodic errors with pages on a new site we're building. We have a content type set to bUseInTree = true so it can be attached to a nav node.
Sometimes when viewing this page it will return a 404 error as if the page were in draft (it works fine when logged in). This content type does NOT extend versions.
I dug into core and found that in navajo/display.cfm around line 146 of the current p720 branch it checks for a valid status. The status key of the record in question (which should not even exist) was set to an empty string.
Digging further I found that in webtopTreeChildRows.cfm (a webskin of dmNavigation) I see there is code that sets status to an empty string if it is not already set (as well as sets bHasVersion, versionid keys). This is around line 283.
The problem is that in CF structs are passed by reference, so if this is the first time that getData has been called for a record then those keys in the struct actually added not only to the struct being used for that webskin but also the copy that is in the session's tempObjectStore and apparently gets as far as the object in the objectBroker. So all subsequent calls to getData that respect the cache get a copy of the object with a status key set to an empty string.
You can rereate this by restarting ColdFusion, the logging into the webtop, go to the site tab so that you can view the tree. If you have a content type in the tree that does not extend versions, then you will see a 404 error if you go to view that page in an other browser (not logged into FarCry). So if a call to webtopTreeChildRows is the FIRST time the record is called via getData then those keys end up in the cache.
Obviously this is a big issue if you use content types that don't extend versions in the tree.
Ways to fix this:
Create duplicates of the structs in webtopTreeChildRows and modify those instead of the referenced struct.
Or instead of setting status to an empty string, set it to approved. If it doesn't extend versions then it should be considered approved anyway. (This is the option I'd suggest)
Modify navajo/display.cfm to be aware that status might be an empty string and if it is, assume it is approved. (Might be a good idea anyway just in case there are other places in the code that might incorrectly set status to an empty string)
If you'd rather I submit a pull request to fix it, let me know.