Skip to content
Permalink
Browse files
Fix session management in manual trigger page
In ManualTriggerAction, session is always created if nothing.
It might be issue if session is closed before build.

This patch creates session once then reuse it.

Fix for JENKINS-19013

Task-Url: https://issues.jenkins-ci.org/browse/JENKINS-19013
  • Loading branch information
rinrinne committed Sep 9, 2014
1 parent 39171d5 commit 02c4d4febc31582d2d39c5955d3ae4dd40d8c01b
@@ -37,13 +37,15 @@
import com.sonyericsson.hudson.plugins.gerrit.trigger.events.ManualPatchsetCreated;
import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.GerritTriggerParameters;
import com.sonyericsson.hudson.plugins.gerrit.trigger.utils.StringUtil;

import hudson.Extension;
import hudson.model.Hudson;
import hudson.model.ParameterValue;
import hudson.model.RootAction;
import hudson.security.Permission;
import net.sf.json.JSONArray;
import net.sf.json.JSONObject;

import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
@@ -56,6 +58,8 @@
import java.util.LinkedList;
import java.util.List;

import javax.servlet.http.HttpSession;

import static com.sonyericsson.hudson.plugins.gerrit.trigger.utils.StringUtil.getPluginImageUrl;

/**
@@ -283,7 +287,12 @@ public void doGerritSearch(@QueryParameter("queryString") final String queryStri
@QueryParameter("selectedServer") final String selectedServer, StaplerRequest request,
StaplerResponse response) throws IOException {

request.getSession(true).setAttribute("selectedServer", selectedServer);
HttpSession session = request.getSession();
// Create session if nothing.
if (session == null) {
session = request.getSession(true);
}
session.setAttribute("selectedServer", selectedServer);
if (!isServerEnabled(selectedServer)) {
response.sendRedirect2(".");
return;
@@ -293,20 +302,20 @@ public void doGerritSearch(@QueryParameter("queryString") final String queryStri

if (config != null) {
GerritQueryHandler handler = new GerritQueryHandler(config);
clearSessionData(request);
request.getSession(true).setAttribute("queryString", queryString);
clearSessionData(session);
session.setAttribute("queryString", queryString);

try {
List<JSONObject> json = handler.queryJava(queryString, true, true, false);
request.getSession(true).setAttribute(SESSION_RESULT, json);
session.setAttribute(SESSION_RESULT, json);
//TODO Implement some smart default selection.
//That can notice that a specific revision is searched or that there is only one result etc.
} catch (GerritQueryException gqe) {
logger.debug("Bad query. ", gqe);
request.getSession(true).setAttribute(SESSION_SEARCH_ERROR, gqe);
session.setAttribute(SESSION_SEARCH_ERROR, gqe);
} catch (Exception ex) {
logger.warn("Could not query Gerrit for [" + queryString + "]", ex);
request.getSession(true).setAttribute(SESSION_SEARCH_ERROR, ex);
session.setAttribute(SESSION_SEARCH_ERROR, ex);
}
response.sendRedirect2(".");
} else {
@@ -327,28 +336,37 @@ public void doGerritSearch(@QueryParameter("queryString") final String queryStri
public void doBuild(@QueryParameter("selectedIds") String selectedIds, StaplerRequest request,
StaplerResponse response) throws IOException {

String selectedServer = (String)request.getSession().getAttribute("selectedServer");
HttpSession session = request.getSession();
if (session == null) {
logger.debug("Session alreay closed.");
session = request.getSession(true);
session.setAttribute(SESSION_BUILD_ERROR, Messages.ErrorSessionAlreadyClosed());
response.sendRedirect2(".");
return;
}

String selectedServer = (String)session.getAttribute("selectedServer");
if (!isServerEnabled(selectedServer)) {
response.sendRedirect2(".");
return;
}
Hudson.getInstance().checkPermission(PluginImpl.MANUAL_TRIGGER);

request.getSession(true).removeAttribute(SESSION_BUILD_ERROR);
session.removeAttribute(SESSION_BUILD_ERROR);
String[] selectedRows = null;
if (selectedIds != null && selectedIds.length() > 0) {
selectedRows = selectedIds.split("\\[\\]");
}
if (selectedRows == null || selectedRows.length <= 0) {
logger.debug("No builds selected.");
request.getSession(true).setAttribute(SESSION_BUILD_ERROR, Messages.ErrorSelectSomethingToBuild());
session.setAttribute(SESSION_BUILD_ERROR, Messages.ErrorSelectSomethingToBuild());
response.sendRedirect2(".");
} else {
logger.debug("Something to build.");
List<JSONObject> result = (List<JSONObject>)request.getSession(true).getAttribute(SESSION_RESULT);
List<JSONObject> result = (List<JSONObject>)session.getAttribute(SESSION_RESULT);
TriggerMonitor monitor = new TriggerMonitor();
logger.trace("Putting monitor into session.");
request.getSession(true).setAttribute(SESSION_TRIGGER_MONITOR, monitor);
session.setAttribute(SESSION_TRIGGER_MONITOR, monitor);
logger.trace("Calling to index the search result.");
HashMap<String, JSONObject> indexed = indexResult(result);
logger.debug("Creating and triggering events.");
@@ -372,13 +390,13 @@ public void doBuild(@QueryParameter("selectedIds") String selectedIds, StaplerRe
/**
* Clears the HTTP session from search and manual-trigger related data.
*
* @param request the request with the HTTP session.
* @param session the HTTP session.
*/
private void clearSessionData(StaplerRequest request) {
request.getSession(true).removeAttribute(SESSION_SEARCH_ERROR);
request.getSession(true).removeAttribute(SESSION_BUILD_ERROR);
request.getSession(true).removeAttribute(SESSION_RESULT);
request.getSession(true).removeAttribute(SESSION_TRIGGER_MONITOR);
private void clearSessionData(HttpSession session) {
session.removeAttribute(SESSION_SEARCH_ERROR);
session.removeAttribute(SESSION_BUILD_ERROR);
session.removeAttribute(SESSION_RESULT);
session.removeAttribute(SESSION_TRIGGER_MONITOR);
}

/**
@@ -74,6 +74,8 @@ ManualGerritTrigger=\
Query and Trigger Gerrit Patches
ErrorSelectSomethingToBuild=\
Please select something to build.
ErrorSessionAlreadyClosed=\
Session is already closed. Please search again.
GerritPermissionGroup=\
Gerrit
ManualTriggerPermissionDescription=\
@@ -74,6 +74,8 @@ ManualGerritTrigger=\
Gerrit\u30d1\u30c3\u30c1\u306e\u30af\u30a8\u30ea\u3068\u30c8\u30ea\u30ac\u30fc
ErrorSelectSomethingToBuild=\
\u30d3\u30eb\u30c9\u3092\u9078\u629e\u3057\u3066\u304f\u3060\u3055\u3044
ErrorSessionAlreadyClosed=\
\u30bb\u30c3\u30b7\u30e7\u30f3\u304c\u5207\u308c\u307e\u3057\u305f\u3002\u518d\u5ea6\u691c\u7d22\u3057\u3066\u304f\u3060\u3055\u3044
GerritPermissionGroup=\
Gerrit
ManualTriggerPermissionDescription=\

0 comments on commit 02c4d4f

Please sign in to comment.