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

[ELY-654] Ensure that negative values are converted to zero when setting sessionCacheSize and sessionTimeout in SSLSessionContext. #505

Merged
merged 1 commit into from Oct 11, 2016

Conversation

ivassile
Copy link
Contributor

@ivassile ivassile commented Oct 7, 2016

sessionCacheSize and sessionTimeout in SSLSessionContext.
@wildfly-ci
Copy link

Can one of the admins verify this patch?

@darranl
Copy link
Contributor

darranl commented Oct 10, 2016

This is Ok to test

@darranl darranl added the +1 DAL label Oct 10, 2016
@mchoma
Copy link
Contributor

mchoma commented Oct 10, 2016

Personally, I would prefer not allow user to configure negative values, rather than automagically convert to 0. Also because 0 means there is no limit. User can input negative value by mistake and EAP will accept it with least restrictive semantic.

Copy link
Contributor

@pskopek pskopek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is OK to have this check, but subsystem should not allow negative values anyway.

@pskopek pskopek added the +1 PS label Oct 11, 2016
@pskopek pskopek merged commit 6288c67 into wildfly-security:master Oct 11, 2016
@mchoma
Copy link
Contributor

mchoma commented Oct 12, 2016

Still, from my point of view this change is not right. I think proper solution would be to throw exception once elytron get negative value. It can be achieved by explicit elytron check or by implicit java check , as java should throw IllegalArgumentException in that case [1].

Imagine user of elytron library screw something up (just misunderstoonding of option, or arithmetic overflow of automatic script). In that case, insted of noticing user that something is wrong, Elytron library will accept negative value as least restrictive.

If we agree subsystem should have negative value check, then in my opinion same check should be in Elytron library.

[1]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants