Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fragment cache not cleared on forward error dispatch #189

Closed
OrangeDog opened this issue Jun 7, 2019 · 5 comments
Closed

Fragment cache not cleared on forward error dispatch #189

OrangeDog opened this issue Jun 7, 2019 · 5 comments
Assignees
Labels

Comments

@OrangeDog
Copy link

See thymeleaf/thymeleaf#750

That problem was solved by adding code to delete the LayoutDialect::FragmentCollection attribute before returning the error view.

@ultraq ultraq added the bug label Jun 8, 2019
@ultraq ultraq self-assigned this Jun 8, 2019
@ultraq
Copy link
Owner

ultraq commented Jun 8, 2019

Hmm, well that's no good. I'll try reproduce it but from your explanation on the other issue it looks like that fragment collection is surviving between the original exception and the rendering of the error page, causing the original exception to happen again. I might need to think of something as alternative to that fragment cache... 🤔

@ultraq
Copy link
Owner

ultraq commented Oct 28, 2019

Notes to future self:

Had a think about this and there aren't any hooks that I can think of for using to clear the fragment cache. Thymeleaf's context object, in a web environment, uses an HttpServletRequest under the hood to store all of the context variables (with a stack-like structure to act as scoping values to elements, pushing/popping them as they're encountered/closed), and that request is the same object when going from standard processing to error processing, hence the bug raised in the OP. I had wrongly assumed that the context was bound/scoped to a call of Thymeleaf's TemplateEngine.process(...) methods.

I tried to see if there were any hooks in Thymeleaf for being able to manage this myself. The template start/end looked promising, but a single TemplateEngine.process(...) call can generate several start/end template events as including fragments opens up new templates. Also, all the template info is discarded so I can't try hack something together using like the first encountered template and waiting until that is done: https://github.com/thymeleaf/thymeleaf/blob/3a8366126d51461e59bfdc5676fea334b1184a72/src/main/java/org/thymeleaf/engine/TemplateHandlerAdapterMarkupHandler.java#L92

So I'll need to think of something else. The first idea was to do away with the cache completely. This should work but I know some features like deeply-nested layout hierarchies currently rely on the cache as a mechanism for passing fragments across templates when there's no obvious link between them, eg: child template defines a "content" section and isn't used in the parent template but the "grandparent" template.

@OrangeDog
Copy link
Author

Does counting the template depth work? You don't need to remember what the first template was as long as there are the same number of start/end events.

@ultraq
Copy link
Owner

ultraq commented Oct 29, 2019

Definitely something to look into. Seems Thymeleaf even has a EngineContext.getTemplateStack method for that which could be useful.

@ultraq
Copy link
Owner

ultraq commented Nov 7, 2019

OK, I've got a fix for this which will be part of the upcoming 2.5.0 release, but for now is available in the current 2.5.0-SNAPSHOT version available on the Maven Central snapshots repository. (I managed to recreate the problem in a personal project, then applied the snapshot version and it went away.)

I've written some unit tests for always returning a fresh fragment collection/cache when it looks like we're processing a new layout (by checking the templateStack as mentioned above), but am contemplating if I need to create some kind of integration-style test since this bug was really only discovered by creating an error scenario in Spring.

@ultraq ultraq closed this as completed Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants