Skip to content

Commit

Permalink
Fixed #397 - Refactored Code Replaced UrlEscapers calls with Encoding…
Browse files Browse the repository at this point in the history
…Utils
  • Loading branch information
khmarbaise committed Mar 30, 2019
1 parent 37dacfb commit d4b0c13
Show file tree
Hide file tree
Showing 10 changed files with 27 additions and 28 deletions.
4 changes: 4 additions & 0 deletions ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Release 0.3.9 (NOT RELEASED YET)

* [Fixed Issue 397][issue-397]

Refactored Code Replaced UrlEscapers calls with EncodingUtils.

* [Fixed Issue 396][issue-396]

Add Unit Test for EncodingUtils.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ public void createJob(FolderJob folder, String jobName, String jobXml) throws IO
* @throws IOException in case of an error.
*/
public void createJob(FolderJob folder, String jobName, String jobXml, Boolean crumbFlag) throws IOException {
client.post_xml(UrlUtils.toBaseUrl(folder) + "createItem?name=" + EncodingUtils.encodeParam(jobName), jobXml, crumbFlag);
client.post_xml(UrlUtils.toBaseUrl(folder) + "createItem?name=" + EncodingUtils.formParameter(jobName), jobXml, crumbFlag);
}

/**
Expand Down Expand Up @@ -438,7 +438,7 @@ public void createView(FolderJob folder, String viewName, String viewXml) throws
* @throws IOException in case of an error.
*/
public void createView(FolderJob folder, String viewName, String viewXml, Boolean crumbFlag) throws IOException {
client.post_xml(UrlUtils.toBaseUrl(folder) + "createView?name=" + EncodingUtils.encodeParam(viewName), viewXml,
client.post_xml(UrlUtils.toBaseUrl(folder) + "createView?name=" + EncodingUtils.formParameter(viewName), viewXml,
crumbFlag);
}

Expand Down Expand Up @@ -488,7 +488,7 @@ public void createFolder(FolderJob folder, String jobName, Boolean crumbFlag) th
// https://gist.github.com/stuart-warren/7786892 was slightly helpful
// here
ImmutableMap<String, String> params = ImmutableMap.of("mode", "com.cloudbees.hudson.plugins.folder.Folder",
"name", EncodingUtils.encodeParam(jobName), "from", "", "Submit", "OK");
"name", EncodingUtils.formParameter(jobName), "from", "", "Submit", "OK");
client.post_form(UrlUtils.toBaseUrl(folder) + "createItem?", params, crumbFlag);
}

Expand Down Expand Up @@ -627,7 +627,7 @@ public void updateJob(FolderJob folder, String jobName, String jobXml, boolean c

/**
* @param jobName name of the job.
* @param name name of the parameter.
* @param name name of the formParameter.
* @param description of the parameters.
* @param defaultValue the defaultValue for the parameters.
* @throws IOException in case of an error.
Expand Down Expand Up @@ -884,7 +884,7 @@ public void renameJob(FolderJob folder, String oldJobName, String newJobName) th
public void renameJob(FolderJob folder, String oldJobName, String newJobName, Boolean crumbFlag)
throws IOException {
client.post(UrlUtils.toJobBaseUrl(folder, oldJobName)
+ "/doRename?newName=" + EncodingUtils.encodeParam(newJobName),
+ "/doRename?newName=" + EncodingUtils.formParameter(newJobName),
crumbFlag);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import net.sf.json.JSONObject;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.NameValuePair;
Expand Down Expand Up @@ -290,10 +289,10 @@ public void post_form(String path, Map<String, String> data, boolean crumbFlag)
// helpful here
List<String> queryParams = Lists.newArrayList();
for (String param : data.keySet()) {
queryParams.add(param + "=" + EncodingUtils.encodeParam(data.get(param)));
queryParams.add(param + "=" + EncodingUtils.formParameter(data.get(param)));
}

queryParams.add("json=" + EncodingUtils.encodeParam(JSONObject.fromObject(data).toString()));
queryParams.add("json=" + EncodingUtils.formParameter(JSONObject.fromObject(data).toString()));
String value = mapper.writeValueAsString(data);
StringEntity stringEntity = new StringEntity(value, ContentType.APPLICATION_FORM_URLENCODED);
request = new HttpPost(UrlUtils.toNoApiUri(uri, context, path) + StringUtils.join(queryParams, "&"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public interface JenkinsHttpConnection extends Closeable {
* useful for other API calls as well. Unlike post and post_xml, the path is
* *not* modified by adding "/toJsonApiUri/json". Additionally, the params
* in data are provided as both request parameters including a json
* parameter, *and* in the JSON-formatted StringEntity, because this is what
* formParameter, *and* in the JSON-formatted StringEntity, because this is what
* the folder creation call required. It is unclear if any other jenkins
* APIs operate in this fashion.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ private EncodingUtils() {

public static String encode(String pathPart) {
// jenkins doesn't like the + for space, use %20 instead
String escape = UrlEscapers.urlPathSegmentEscaper().escape(pathPart);
return escape;
return UrlEscapers.urlPathSegmentEscaper().escape(pathPart);
}

public static String encodeParam(String pathPart) {
public static String formParameter(String pathPart) {
// jenkins doesn't like the + for space, use %20 instead
return UrlEscapers.urlFormParameterEscaper().escape(pathPart);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import java.io.IOException;
import java.util.List;

import com.google.common.net.UrlEscapers;
import com.offbytwo.jenkins.client.util.EncodingUtils;

/**
* @author Kelly Plummer
Expand Down Expand Up @@ -46,7 +46,7 @@ public ComputerWithDetails details() throws IOException {
if ("master".equals(displayName)) {
name = "(master)";
} else {
name = UrlEscapers.urlPathSegmentEscaper().escape(displayName);
name = EncodingUtils.encode(displayName);
}
// TODO: Check if depth=2 is a good idea or if it could be solved
// better.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import java.util.Map;

import com.google.common.base.Function;
import com.google.common.net.UrlEscapers;
import com.offbytwo.jenkins.client.util.EncodingUtils;

public class ComputerWithDetails extends Computer {

Expand Down Expand Up @@ -72,7 +72,7 @@ public LoadStatistics getLoadStatistics() throws IOException {
if ("master".equals(displayName)) {
name = "(master)";
} else {
name = UrlEscapers.urlPathSegmentEscaper().escape(displayName);
name = EncodingUtils.encode(displayName);
}

// TODO: ?depth=2 good idea or could this being done better?
Expand All @@ -85,7 +85,7 @@ public void toggleOffline(boolean crumbFlag) throws IOException {
if ("master".equals(displayName)) {
name = "(master)";
} else {
name = UrlEscapers.urlPathSegmentEscaper().escape(displayName);
name = EncodingUtils.encode(displayName);
}

Map<String, String> data = new HashMap<String, String>();
Expand All @@ -102,7 +102,7 @@ public void changeOfflineCause(String cause, boolean crumbFlag) throws IOExcepti
if ("master".equals(displayName)) {
name = "(master)";
} else {
name = UrlEscapers.urlPathSegmentEscaper().escape(displayName);
name = EncodingUtils.encode(displayName);
}

Map<String, String> data = new HashMap<String, String>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void createFolder(String folderName, Boolean crumbFlag) throws IOExceptio
// https://gist.github.com/stuart-warren/7786892 was slightly helpful
// here
ImmutableMap<String, String> params = ImmutableMap.of("mode", "com.cloudbees.hudson.plugins.folder.Folder",
"name", EncodingUtils.encodeParam(folderName), "from", "", "Submit", "OK");
"name", EncodingUtils.formParameter(folderName), "from", "", "Submit", "OK");
client.post_form(this.getUrl() + "/createItem?", params, crumbFlag);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@

import com.google.common.base.Function;
import com.google.common.collect.Collections2;
import com.google.common.escape.Escaper;
import com.google.common.net.UrlEscapers;
import com.offbytwo.jenkins.client.util.EncodingUtils;

public class Job extends BaseModel {

Expand Down Expand Up @@ -181,8 +180,7 @@ public int hashCode() {
private static class MapEntryToQueryStringPair implements Function<Map.Entry<String, String>, String> {
@Override
public String apply(Map.Entry<String, String> entry) {
Escaper escaper = UrlEscapers.urlFormParameterEscaper();
return escaper.escape(entry.getKey()) + "=" + escaper.escape(entry.getValue());
return EncodingUtils.formParameter(entry.getKey()) + "=" + EncodingUtils.formParameter(entry.getValue());
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.offbytwo.jenkins.client.util;

import org.assertj.core.api.Assertions;
import org.junit.Test;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -32,25 +31,25 @@ public void encodeShouldReturnEncodingUmlautAndOthers() {

@Test
public void encodeParamShouldReturnEncodedExclamationMarkDoubleQuoteAmpersampSpace() {
String result = EncodingUtils.encodeParam("!\"& ");
String result = EncodingUtils.formParameter("!\"& ");
assertThat(result).isEqualTo("%21%22%26+");
}

@Test
public void encodeParamShouldReturnNotEncodeSafeChars() {
String result = EncodingUtils.encodeParam("-_.*");
String result = EncodingUtils.formParameter("-_.*");
assertThat(result).isEqualTo("-_.*");
}

@Test
public void encodeParamShouldReturnEncodedUmlautAndOthers() {
String result = EncodingUtils.encodeParam("äöü#{}");
String result = EncodingUtils.formParameter("äöü#{}");
assertThat(result).isEqualTo("%C3%A4%C3%B6%C3%BC%23%7B%7D");
}

@Test
public void encodeParamShouldReturnEncodedCharacters() {
String result = EncodingUtils.encodeParam("-._~!$'()*,;&=@:+");
String result = EncodingUtils.formParameter("-._~!$'()*,;&=@:+");
assertThat(result).isEqualTo("-._%7E%21%24%27%28%29*%2C%3B%26%3D%40%3A%2B");
}

Expand Down

0 comments on commit d4b0c13

Please sign in to comment.