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

Exception Handler: do not try to create sessions #11737

Closed
NicolaIsotta opened this issue Apr 9, 2024 · 4 comments · Fixed by #11746
Closed

Exception Handler: do not try to create sessions #11737

NicolaIsotta opened this issue Apr 9, 2024 · 4 comments · Fixed by #11746
Assignees
Labels
enhancement Additional functionality to current component
Milestone

Comments

@NicolaIsotta
Copy link
Contributor

As discussed on discord (https://discord.com/channels/787967399105134612/787967662293909524/1225465731215392838)

Currently PrimeExceptionHandler and PrimeExceptionHandlerELResolver use ExternalContext#getSessionMap to retrieve the session map. This triggers session creation if it doesn't exist, and it's bad for two reasons:

  • if the response is already commited, trying to create a session causes an exception
  • if the user want stateless mode (f:view transient="true")

Those classes should check session existence before using the session map.
Some alternatives I thought about:

  • using ClientWindow as suggested by @tandraschko -> still needs session if i'm not wrong
  • using Flash -> will fail if the response is already commited
  • sending exception info as encoded url params -> is it safe?
  • saving exception infos in the application map using a random id and send the id as a url param -> how and when the map will be cleaned?

Any thoughts?

@melloware melloware added the discussion Item needs to be discussed by core devs label Apr 9, 2024
@fcorneli
Copy link
Contributor

fcorneli commented Apr 9, 2024

A possible implementation could use "crypto tokens".
Create an in-memory AES256 global key when PF starts.
Use this to AES256-GCM protect your URL params or cookie value or whatever you think that needs to be secured.
Advantage here is that you don't have to worry about your security token "lifecycle", i.e., no server-side map risks of being saturated.

@tandraschko
Copy link
Member

tandraschko commented Apr 9, 2024

  1. we need this, Independent of your exception. Session doesnt work well with multiple tabs.
  2. doesnt fix it
  3. stacktrace in url param? -1
  4. possible, i wont invest any time here. A user doesnt need exception to be displayed anyway, its just a nice gimmick. Generic error page should be enough - at least in production.

@NicolaIsotta
Copy link
Contributor Author

I'll give it a try with null check + client window

(another idea: we could skip creating and saving ExceptionInfo if PrimeExceptionHandlerELResolver isn't enabled, but I'm not sure if there's a way to check the available el resolvers)

@tandraschko
Copy link
Member

+1

even if the user added the ELResolver, we dont know if he really uses the EL

i would just do this for now:

if (clientWindow != null) { put }
else if (session != null) { put }
else { do nothing }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Additional functionality to current component
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants