Skip to content

Commit

Permalink
chore(warnings): replace String::split with Splitter::on
Browse files Browse the repository at this point in the history
This usage was particularly dangerous:

  String[] bits = type.split("\\.");
  return bits[bits.length - 1];

Depending on the input string, bits could be an empty list.
Also on every call of the method, the regex needs to be compiled.

See http://errorprone.info/bugpattern/StringSplitter
  • Loading branch information
dreynaud committed Jan 31, 2018
1 parent e01e7e9 commit bf35e28
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.google.common.base.Splitter;

import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -28,7 +29,7 @@
import java.util.LinkedHashSet;

public class StageDefinition implements Identifiable, Conditional, Cloneable {

private static final Splitter ON_DOTS = Splitter.on(".");
private String id;
private String name;
private InjectionRule inject;
Expand Down Expand Up @@ -308,8 +309,9 @@ public String getPartialId() {
if (type == null) {
return null;
}
String[] bits = type.split("\\.");
return bits[bits.length - 1];

List<String> bits = ON_DOTS.splitToList(type);
return bits.get(bits.size() - 1);
}

public PartialDefinitionContext getPartialDefinitionContext() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Splitter;
import com.hubspot.jinjava.interpret.Context;
import com.hubspot.jinjava.interpret.InterpretException;
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
Expand Down Expand Up @@ -45,6 +46,7 @@
import java.util.stream.Collectors;

public class ModuleTag implements Tag {
private static final Splitter ON_EQUALS = Splitter.on("=");

private Renderer renderer;
private ObjectMapper objectMapper;
Expand Down Expand Up @@ -91,11 +93,11 @@ public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) {
// Assign parameters into the context
Map<String, String> paramPairs = new HashMap<>();
helper.subList(1, helper.size()).forEach(p -> {
String[] parts = p.split("=");
if (parts.length != 2) {
List<String> parts = ON_EQUALS.splitToList(p);
if (parts.size() != 2) {
throw new TemplateSyntaxException(tagNode.getMaster().getImage(), "Tag 'module' expects parameters to be in a 'key=value' format: " + helper, tagNode.getLineNumber());
}
paramPairs.put(parts[0], parts[1]);
paramPairs.put(parts.get(0), parts.get(1));
});

List<String> missing = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

package com.netflix.spinnaker.orca.pipelinetemplate.v1schema.render.tags;
import com.google.common.base.Splitter;
import com.hubspot.jinjava.interpret.Context;
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
import com.hubspot.jinjava.interpret.TemplateSyntaxException;
Expand All @@ -34,6 +35,7 @@
public class PipelineIdTag implements Tag {
private static final String APPLICATION = "application";
private static final String NAME = "name";
private static final Splitter ON_EQUALS = Splitter.on("=");

private final Front50Service front50Service;

Expand All @@ -50,12 +52,12 @@ public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) {

Map<String, String> paramPairs = new HashMap<>();
helper.forEach(p -> {
String[] parts = p.split("=");
if (parts.length != 2) {
List<String> parts = ON_EQUALS.splitToList(p);
if (parts.size() != 2) {
throw new TemplateSyntaxException(tagNode.getMaster().getImage(), "Tag 'pipelineId' expects parameters to be in a 'key=value' format: " + helper, tagNode.getLineNumber());
}

paramPairs.put(parts[0], parts[1]);
paramPairs.put(parts.get(0), parts.get(1));
});

Context context = interpreter.getContext();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.netflix.spinnaker.orca.pipelinetemplate.v1schema.render.tags;

import com.google.common.base.Splitter;
import com.hubspot.jinjava.interpret.Context;
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
import com.hubspot.jinjava.interpret.TemplateSyntaxException;
Expand All @@ -13,6 +14,7 @@
import java.util.*;

public class StrategyIdTag implements Tag {
private static final Splitter ON_EQUALS = Splitter.on("=");
private static final String APPLICATION = "application";
private static final String NAME = "name";

Expand Down Expand Up @@ -41,12 +43,12 @@ public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) {

Map<String, String> paramPairs = new HashMap<>();
helper.forEach(p -> {
String[] parts = p.split("=");
if (parts.length != 2) {
throw new TemplateSyntaxException(tagNode.getMaster().getImage(), "Tag 'strategyId' expects parameters to be in a 'key=value' format: " + helper, tagNode.getLineNumber());
}
List<String> parts = ON_EQUALS.splitToList(p);
if (parts.size() != 2) {
throw new TemplateSyntaxException(tagNode.getMaster().getImage(), "Tag 'strategyId' expects parameters to be in a 'key=value' format: " + helper, tagNode.getLineNumber());
}

paramPairs.put(parts[0], parts[1]);
paramPairs.put(parts.get(0), parts.get(1));
});

Context context = interpreter.getContext();
Expand Down

0 comments on commit bf35e28

Please sign in to comment.