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

Added a default value for ApplicationSessionCookieConfig#name #122

Closed
wants to merge 1 commit into from

Conversation

bitstorm
Copy link

@bitstorm bitstorm commented Sep 16, 2018

By spec. javax.servlet.SessionCookieConfig#getName should return 'JSESSIONID' as default value (see https://docs.oracle.com/javaee/7/api/javax/servlet/SessionCookieConfig.html).
However, ApplicationSessionCookieConfig has no default value for this field and returns 'null' if not set.
This PR set 'JSESSIONID' as default value.

@kkolinko
Copy link

The javadoc that you linked
https://docs.oracle.com/javaee/7/api/javax/servlet/SessionCookieConfig.html
says "Returns: [...] or null if setName(java.lang.String) was never called"

Thus it documents null as the default value.

BTW, the cookie name can be overwritten by sessionCookieName attribute on Context,
http://tomcat.apache.org/tomcat-9.0-doc/config/context.html#Common_Attributes

@markt-asf
Copy link
Contributor

Closing the PR as the requested change is not correct.

@markt-asf markt-asf closed this Sep 17, 2018
@bitstorm
Copy link
Author

@kkolinko

Thank you for the clarification!

@martin-g
Copy link
Member

The new question is "Should Tomcat call #setName("JSESSIONID") during start of an application (just after creation of SessionCookieConfig) ?" With the current behavior every application/framework should make checks for null and assume JSESSIONID.

@markt-asf
Copy link
Contributor

There is nothing in the spec that says Tomcat is required to do that.

@martin-g
Copy link
Member

There is also nothing that says not to do it. The question is which behavior is better.
IMO returning the actual value that is in use is much better than null.

@solomax
Copy link

solomax commented Sep 17, 2018

+1 no-one like NPE :)

@bitstorm
Copy link
Author

bitstorm commented Sep 17, 2018

To me specs are a little contradictory about this. They state that the default value is JSESSIONID but at the same time they allow to return a null if a custom value is not set for this parameter. Since ApplicationSessionCookieConfig already exposes a default value for max ege it would be nice to do the same for session cookie name and don't leave the responsibility for handling this situation to application code.

@martin-g
Copy link
Member

s/edge/age/

@solomax
Copy link

solomax commented Sep 19, 2018

@markt-asf ping :)

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