Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-35206] - Make the InstallState object deserialization robust…
… against corrupted files (#2387)

* [JENKINS-35206] - Add unit tests for the deserialization logic

* [JENKINS-35206] - Install state should be robust against messed statuses when deserializing objects

* [JENKINS-35206] - Fix the license header in thre test file

(cherry picked from commit 1b2711c)
  • Loading branch information
oleg-nenashev authored and olivergondza committed Jun 19, 2016
1 parent 3837f44 commit e8467e3
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 1 deletion.
23 changes: 22 additions & 1 deletion core/src/main/java/jenkins/install/InstallState.java
Expand Up @@ -29,8 +29,10 @@
import hudson.Extension;
import hudson.ExtensionList;
import hudson.ExtensionPoint;
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.model.Jenkins;

import org.apache.commons.lang.StringUtils;
/**
* Jenkins install state.
*
Expand Down Expand Up @@ -141,6 +143,8 @@ public void initializeState() {
}
};

private static final Logger LOGGER = Logger.getLogger(InstallState.class.getName());

/**
* Jenkins started in test mode (JenkinsRule).
*/
Expand All @@ -165,6 +169,23 @@ public InstallState(@Nonnull String name, boolean isSetupComplete) {
*/
public void initializeState() {
}

public Object readResolve() {
// If we get invalid state from the configuration, fallback to unknown
if (StringUtils.isBlank(name)) {
LOGGER.log(Level.WARNING, "Read install state with blank name: '{0}'. It will be ignored", name);
return UNKNOWN;
}

InstallState state = InstallState.valueOf(name);
if (state == null) {
LOGGER.log(Level.WARNING, "Cannot locate an extension point for the state '{0}'. It will be ignored", name);
return UNKNOWN;
}

// Otherwise we return the actual state
return state;
}

/**
* Indicates the initial setup is complete
Expand Down
99 changes: 99 additions & 0 deletions test/src/test/java/jenkins/install/InstallStateTest.java
@@ -0,0 +1,99 @@
/*
* The MIT License
*
* Copyright (c) 2016 CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package jenkins.install;

import hudson.ExtensionList;
import jenkins.model.Jenkins;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;

/**
* Tests of {@link InstallState}.
* Effectively the most of the tests do not need the Jenkins instance, but we want to
* honor Jenkins extension points and hooks, which may influence the behavior.
* @author Oleg Nenashev
*/
public class InstallStateTest {

@Rule
public JenkinsRule j = new JenkinsRule();

@Test
public void shouldPefromCorrectConversionForAllNames() {
ExtensionList<InstallState> states = InstallState.all();
for (InstallState state : states) {
InstallState afterRoundtrip = forName(state.name());
// It also prevents occasional name duplications
assertThat("State after the roundtrip must be equal to the original state",
afterRoundtrip, equalTo(state));
assertTrue("State " + state + " should return the extension point instance after deserialization",
afterRoundtrip == state);
}
}

@Test
@Issue("JENKINS-35206")
public void shouldNotFailOnNullXMLField() {
String xml = "<jenkins.install.InstallState>\n" +
" <isSetupComplete>true</isSetupComplete>\n" +
"</jenkins.install.InstallState>";
final InstallState state = forXml(xml);
assertThat(state, equalTo(InstallState.UNKNOWN));
}

@Test
@Issue("JENKINS-35206")
public void shouldNotFailOnEmptyName() {
final InstallState state = forName("");
assertThat(state, equalTo(InstallState.UNKNOWN));
}

@Test
@Issue("JENKINS-35206")
public void shouldReturnUnknownStateForUnknownName() {
final InstallState state = forName("NonExistentStateName");
assertThat(state, equalTo(InstallState.UNKNOWN));
}

private static InstallState forName(String name) throws AssertionError {
String xml = "<jenkins.install.InstallState>\n" +
" <isSetupComplete>true</isSetupComplete>\n" +
" <name>" + name + "</name>\n" +
"</jenkins.install.InstallState>";
return forXml(xml);
}

private static InstallState forXml(String xml) throws AssertionError {
Object read = Jenkins.XSTREAM2.fromXML(xml);
assertThat(read, instanceOf(InstallState.class));
InstallState state = (InstallState) read;
return state;
}
}

0 comments on commit e8467e3

Please sign in to comment.