Vojtech Szocs has posted comments on this change. Change subject: webadmin: Small cleanup in RestApiSessionManager ......................................................................
Patch Set 1: (3 comments) Addressed Alon's comments, please review. http://gerrit.ovirt.org/#/c/35708/1/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java: Line 139 Line 140 Line 141 Line 142 Line 143 > from what I see we get the session id twice in this sequence, pass it to fu Sorry, I don't understand your question. getSessionId() just returns stored REST session ID value on client: String getSessionId() { return restApiSessionId; } getSessionId() does not trigger creation of new REST session. (In Java world, getSomething is usually just a getter to retrieve the local state.) Line 68: private static final Logger logger = Logger.getLogger(RestApiSessionManager.class.getName()); Line 69: Line 70: private static final String PREFER_HEADER = "Prefer"; //$NON-NLS-1$ Line 71: private static final String SESSION_ID_HEADER = "JSESSIONID"; //$NON-NLS-1$ Line 72: private static final String CSRF_HEADER = SESSION_ID_HEADER; > I think it is confusing to refer the same header with two names. Yes, it's confusing. There are two different logical HTTP headers: * request header containing CSRF token for REST API, named "JSESSIONID" * response header containing session ID value (same value as cookie), named "JSESSIONID" I made this change per Alex's request to distinguish between these two logical headers. So do you prefer to have following instead? private static final String SESSION_ID_HEADER = "JSESSIONID"; private static final String CSRF_HEADER = "JSESSIONID"; Note: the header for REST API CSRF token should be renamed to something more sensible, I'll raise email on devel list about it. (For now, it's named "JSESSIONID" and we need to deal with it.) Line 73: private static final String ENGINE_AUTH_TOKEN_HEADER = "OVIRT-INTERNAL-ENGINE-AUTH-TOKEN"; //$NON-NLS-1$ Line 74: Line 75: private static final String SESSION_ID_KEY = "RestApiSessionId"; //$NON-NLS-1$ Line 76: private static final String DEFAULT_SESSION_TIMEOUT = "30"; //$NON-NLS-1$ Line 126: RequestBuilder builder = createRequest(); Line 127: Line 128: // Enforce expiry of existing session when acquiring new session Line 129: String preferValue = builder.getHeader(PREFER_HEADER); Line 130: builder.setHeader(PREFER_HEADER, preferValue + ", new-auth"); //$NON-NLS-1$ > usually getHeader does not work properly in http frameworks, I was quite su RequestBuilder is GWT API for making async HTTP requests (JavaScript XMLHttpRequest object). RequestBuilder contains simple "Map<String, String> headers" that will be passed to native XMLHttpRequest object. Line 131: Line 132: // Map this (physical) REST API session to current user's (logical) Engine session Line 133: builder.setHeader(ENGINE_AUTH_TOKEN_HEADER, engineAuthToken); Line 134: -- To view, visit http://gerrit.ovirt.org/35708 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I98ad53785726c4ed1a8b1eaaf4fd473052496c3e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
