diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fa8e3982..d2fee9e46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Next Release - Add ability to set expiration date for a collaboration ([#788](https://github.com/box/box-java-sdk/pull/788)) +- Add path parameter sanitization ([#790](https://github.com/box/box-java-sdk/pull/790)) ## 2.45.0 [2020-04-02] - Add preflight check before chunked uploads ([#782](https://github.com/box/box-java-sdk/pull/782)) diff --git a/src/main/java/com/box/sdk/BoxFile.java b/src/main/java/com/box/sdk/BoxFile.java index dad28ee2b..0c34d8880 100644 --- a/src/main/java/com/box/sdk/BoxFile.java +++ b/src/main/java/com/box/sdk/BoxFile.java @@ -1051,7 +1051,7 @@ public Metadata createMetadata(String typeName, Metadata metadata) { * @return the metadata returned from the server. */ public Metadata createMetadata(String typeName, String scope, Metadata metadata) { - URL url = METADATA_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID(), scope, typeName); + URL url = METADATA_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID(), scope, typeName); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, "POST"); request.addHeader("Content-Type", "application/json"); request.setBody(metadata.toString()); @@ -1297,7 +1297,7 @@ public Metadata getMetadata(String typeName) { * @return the metadata returned from the server. */ public Metadata getMetadata(String typeName, String scope) { - URL url = METADATA_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID(), scope, typeName); + URL url = METADATA_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID(), scope, typeName); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, "GET"); BoxJSONResponse response = (BoxJSONResponse) request.send(); return new Metadata(JsonObject.readFrom(response.getJSON())); @@ -1317,7 +1317,7 @@ public Metadata updateMetadata(Metadata metadata) { scope = Metadata.ENTERPRISE_METADATA_SCOPE; } - URL url = METADATA_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID(), + URL url = METADATA_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID(), scope, metadata.getTemplateName()); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, "PUT"); request.addHeader("Content-Type", "application/json-patch+json"); @@ -1350,7 +1350,7 @@ public void deleteMetadata(String typeName) { * @param scope the metadata scope (global or enterprise). */ public void deleteMetadata(String typeName, String scope) { - URL url = METADATA_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID(), scope, typeName); + URL url = METADATA_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID(), scope, typeName); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, "DELETE"); request.send(); } diff --git a/src/main/java/com/box/sdk/BoxFileVersion.java b/src/main/java/com/box/sdk/BoxFileVersion.java index 2d7704e62..02bcedfaf 100644 --- a/src/main/java/com/box/sdk/BoxFileVersion.java +++ b/src/main/java/com/box/sdk/BoxFileVersion.java @@ -271,7 +271,7 @@ public void download(OutputStream output, ProgressListener listener) { * Promotes this version of the file to be the latest version. */ public void promote() { - URL url = VERSION_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.fileID, "current"); + URL url = VERSION_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.fileID, "current"); JsonObject jsonObject = new JsonObject(); jsonObject.add("type", "file_version"); diff --git a/src/main/java/com/box/sdk/BoxFolder.java b/src/main/java/com/box/sdk/BoxFolder.java index 5216ae33d..f55e6a77a 100644 --- a/src/main/java/com/box/sdk/BoxFolder.java +++ b/src/main/java/com/box/sdk/BoxFolder.java @@ -370,7 +370,7 @@ public BoxFolder.Info createFolder(String name) { * @param recursive true to recursively delete this folder's contents; otherwise false. */ public void delete(boolean recursive) { - URL url = DELETE_FOLDER_URL.build(this.getAPI().getBaseURL(), this.getID(), recursive); + URL url = DELETE_FOLDER_URL.buildAlpha(this.getAPI().getBaseURL(), this.getID(), recursive); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, "DELETE"); BoxAPIResponse response = request.send(); response.disconnect(); @@ -849,7 +849,7 @@ public Metadata createMetadata(String templateName, Metadata metadata) { * @return the metadata returned from the server. */ public Metadata createMetadata(String templateName, String scope, Metadata metadata) { - URL url = METADATA_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID(), scope, templateName); + URL url = METADATA_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID(), scope, templateName); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, "POST"); request.addHeader("Content-Type", "application/json"); request.setBody(metadata.toString()); @@ -925,7 +925,7 @@ public Metadata getMetadata(String templateName) { * @return the metadata returned from the server. */ public Metadata getMetadata(String templateName, String scope) { - URL url = METADATA_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID(), scope, templateName); + URL url = METADATA_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID(), scope, templateName); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, "GET"); BoxJSONResponse response = (BoxJSONResponse) request.send(); return new Metadata(JsonObject.readFrom(response.getJSON())); @@ -938,7 +938,7 @@ public Metadata getMetadata(String templateName, String scope) { * @return the metadata returned from the server. */ public Metadata updateMetadata(Metadata metadata) { - URL url = METADATA_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID(), metadata.getScope(), + URL url = METADATA_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID(), metadata.getScope(), metadata.getTemplateName()); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, "PUT"); request.addHeader("Content-Type", "application/json-patch+json"); @@ -971,7 +971,7 @@ public void deleteMetadata(String templateName) { * @param scope the scope of the template (usually "global" or "enterprise"). */ public void deleteMetadata(String templateName, String scope) { - URL url = METADATA_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID(), scope, templateName); + URL url = METADATA_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID(), scope, templateName); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, "DELETE"); BoxAPIResponse response = request.send(); response.disconnect(); diff --git a/src/main/java/com/box/sdk/BoxMetadataCascadePolicy.java b/src/main/java/com/box/sdk/BoxMetadataCascadePolicy.java index 7015038be..b25deefa4 100644 --- a/src/main/java/com/box/sdk/BoxMetadataCascadePolicy.java +++ b/src/main/java/com/box/sdk/BoxMetadataCascadePolicy.java @@ -99,7 +99,7 @@ public BoxMetadataCascadePolicy.Info getInfo(String... fields) { if (fields.length > 0) { builder.appendParam("fields", fields); } - URL url = METADATA_CASCADE_POLICIES_URL_TEMPLATE.buildWithQuery(this.getAPI().getBaseURL(), + URL url = METADATA_CASCADE_POLICIES_URL_TEMPLATE.buildAlphaWithQuery(this.getAPI().getBaseURL(), builder.toString(), this.getID()); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, "GET"); BoxJSONResponse response = (BoxJSONResponse) request.send(); @@ -140,7 +140,7 @@ public static BoxMetadataCascadePolicy.Info create(final BoxAPIConnection api, S */ public void forceApply(String conflictResolution) { - URL url = FORCE_METADATA_CASCADE_POLICIES_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID()); + URL url = FORCE_METADATA_CASCADE_POLICIES_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID()); BoxJSONRequest request = new BoxJSONRequest(this.getAPI(), url, "POST"); JsonObject requestJSON = new JsonObject() .add("conflict_resolution", conflictResolution); @@ -152,7 +152,7 @@ public void forceApply(String conflictResolution) { * Deletes the metadata cascade policy. */ public void delete() { - URL url = METADATA_CASCADE_POLICIES_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID()); + URL url = METADATA_CASCADE_POLICIES_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID()); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, "DELETE"); BoxAPIResponse response = request.send(); } diff --git a/src/main/java/com/box/sdk/BoxStoragePolicyAssignment.java b/src/main/java/com/box/sdk/BoxStoragePolicyAssignment.java index c492e48ca..b4f1d7944 100644 --- a/src/main/java/com/box/sdk/BoxStoragePolicyAssignment.java +++ b/src/main/java/com/box/sdk/BoxStoragePolicyAssignment.java @@ -68,7 +68,7 @@ public static BoxStoragePolicyAssignment.Info create(BoxAPIConnection api, Strin * @param info the updated info. */ public void updateInfo(BoxStoragePolicyAssignment.Info info) { - URL url = STORAGE_POLICY_ASSIGNMENT_WITH_ID_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID()); + URL url = STORAGE_POLICY_ASSIGNMENT_WITH_ID_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID()); BoxJSONRequest request = new BoxJSONRequest(this.getAPI(), url, "PUT"); request.setBody(info.getPendingChanges()); @@ -105,7 +105,7 @@ public static BoxStoragePolicyAssignment.Info getAssignmentForTarget(final BoxAP * @return information about this {@link BoxStoragePolicyAssignment}. */ public BoxStoragePolicyAssignment.Info getInfo() { - URL url = STORAGE_POLICY_ASSIGNMENT_WITH_ID_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID()); + URL url = STORAGE_POLICY_ASSIGNMENT_WITH_ID_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID()); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, HttpMethod.GET); BoxJSONResponse response = (BoxJSONResponse) request.send(); @@ -116,7 +116,7 @@ public BoxStoragePolicyAssignment.Info getInfo() { * Deletes this BoxStoragePolicyAssignment. */ public void delete() { - URL url = STORAGE_POLICY_ASSIGNMENT_WITH_ID_URL_TEMPLATE.build(this.getAPI().getBaseURL(), this.getID()); + URL url = STORAGE_POLICY_ASSIGNMENT_WITH_ID_URL_TEMPLATE.buildAlpha(this.getAPI().getBaseURL(), this.getID()); BoxAPIRequest request = new BoxAPIRequest(this.getAPI(), url, HttpMethod.DELETE); request.send(); diff --git a/src/main/java/com/box/sdk/EventStream.java b/src/main/java/com/box/sdk/EventStream.java index 8badf564a..30bc89335 100644 --- a/src/main/java/com/box/sdk/EventStream.java +++ b/src/main/java/com/box/sdk/EventStream.java @@ -114,7 +114,8 @@ public void start() { final long initialPosition; if (this.startingPosition == STREAM_POSITION_NOW) { - BoxAPIRequest request = new BoxAPIRequest(this.api, EVENT_URL.build(this.api.getBaseURL(), "now"), "GET"); + BoxAPIRequest request = new BoxAPIRequest(this.api, + EVENT_URL.buildAlpha(this.api.getBaseURL(), "now"), "GET"); BoxJSONResponse response = (BoxJSONResponse) request.send(); JsonObject jsonObject = JsonObject.readFrom(response.getJSON()); initialPosition = jsonObject.get("next_stream_position").asLong(); @@ -210,7 +211,7 @@ public void run() { } BoxAPIRequest request = new BoxAPIRequest(EventStream.this.api, - EVENT_URL.build(EventStream.this.api.getBaseURL(), position), "GET"); + EVENT_URL.buildAlpha(EventStream.this.api.getBaseURL(), position), "GET"); BoxJSONResponse response = (BoxJSONResponse) request.send(); JsonObject jsonObject = JsonObject.readFrom(response.getJSON()); JsonArray entriesArray = jsonObject.get("entries").asArray(); diff --git a/src/main/java/com/box/sdk/MetadataTemplate.java b/src/main/java/com/box/sdk/MetadataTemplate.java index dfbb98d87..a43066f24 100644 --- a/src/main/java/com/box/sdk/MetadataTemplate.java +++ b/src/main/java/com/box/sdk/MetadataTemplate.java @@ -289,7 +289,7 @@ public static MetadataTemplate updateMetadataTemplate(BoxAPIConnection api, Stri } QueryStringBuilder builder = new QueryStringBuilder(); - URL url = METADATA_TEMPLATE_URL_TEMPLATE.build(api.getBaseURL(), scope, template); + URL url = METADATA_TEMPLATE_URL_TEMPLATE.buildAlpha(api.getBaseURL(), scope, template); BoxJSONRequest request = new BoxJSONRequest(api, url, "PUT"); request.setBody(array.toString()); @@ -308,7 +308,7 @@ public static MetadataTemplate updateMetadataTemplate(BoxAPIConnection api, Stri */ public static void deleteMetadataTemplate(BoxAPIConnection api, String scope, String template) { - URL url = METADATA_TEMPLATE_URL_TEMPLATE.build(api.getBaseURL(), scope, template); + URL url = METADATA_TEMPLATE_URL_TEMPLATE.buildAlpha(api.getBaseURL(), scope, template); BoxJSONRequest request = new BoxJSONRequest(api, url, "DELETE"); request.send(); @@ -547,7 +547,7 @@ public static MetadataTemplate getMetadataTemplate( if (fields.length > 0) { builder.appendParam("fields", fields); } - URL url = METADATA_TEMPLATE_URL_TEMPLATE.buildWithQuery( + URL url = METADATA_TEMPLATE_URL_TEMPLATE.buildAlphaWithQuery( api.getBaseURL(), builder.toString(), scope, templateName); BoxAPIRequest request = new BoxAPIRequest(api, url, "GET"); BoxJSONResponse response = (BoxJSONResponse) request.send(); @@ -562,7 +562,7 @@ public static MetadataTemplate getMetadataTemplate( */ public static MetadataTemplate getMetadataTemplateByID(BoxAPIConnection api, String templateID) { - URL url = METADATA_TEMPLATE_BY_ID_URL_TEMPLATE.build(api.getBaseURL(), templateID); + URL url = METADATA_TEMPLATE_BY_ID_URL_TEMPLATE.buildAlpha(api.getBaseURL(), templateID); BoxAPIRequest request = new BoxAPIRequest(api, url, "GET"); BoxJSONResponse response = (BoxJSONResponse) request.send(); return new MetadataTemplate(response.getJSON()); @@ -605,7 +605,7 @@ public static Iterable getEnterpriseMetadataTemplates( builder.appendParam("fields", fields); } return new BoxResourceIterable( - api, ENTERPRISE_METADATA_URL_TEMPLATE.buildWithQuery( + api, ENTERPRISE_METADATA_URL_TEMPLATE.buildAlphaWithQuery( api.getBaseURL(), builder.toString(), scope), limit) { @Override diff --git a/src/main/java/com/box/sdk/URLTemplate.java b/src/main/java/com/box/sdk/URLTemplate.java index d33ffd53a..d446a999f 100644 --- a/src/main/java/com/box/sdk/URLTemplate.java +++ b/src/main/java/com/box/sdk/URLTemplate.java @@ -2,11 +2,14 @@ import java.net.MalformedURLException; import java.net.URL; +import java.util.regex.Pattern; /** * A template class to build URLs from base URL, path, URL parameters and Query String. */ public class URLTemplate { + private static final Pattern NUMERIC = Pattern.compile("^[0-9]*$"); + private static final Pattern ALPHA_NUMERIC = Pattern.compile("^[a-zA-Z0-9!@#$%^&*()_+\\-]*$"); private String template; /** @@ -18,38 +21,102 @@ public URLTemplate(String template) { } /** - * Build a URL with URL Parameters. + * Build a URL with numeric URL Parameters. * @param base base URL * @param values URL parameters * @return URL */ public URL build(String base, Object... values) { + for (Object value: values) { + String valueString = String.valueOf(value); + Boolean test = NUMERIC.matcher(valueString).matches(); + if (!NUMERIC.matcher(valueString).matches()) { + assert false : "An invalid path parameter passed in. It must be numeric."; + } + } + String urlString = String.format(base + this.template, values); + + URL url = null; + try { + url = new URL(urlString); + } catch (MalformedURLException e) { + assert false : "A valid URL could not be constructed from the provided parameters."; + } + + return url; + } + + /** + * Build a URL with alphanumeric URL Parameters. + * @param base base URL + * @param values URL parameters + * @return URL + */ + public URL buildAlpha(String base, Object... values) { + for (Object value: values) { + String valueString = String.valueOf(value); + Boolean test = ALPHA_NUMERIC.matcher(valueString).matches(); + if (!ALPHA_NUMERIC.matcher(valueString).matches()) { + assert false : "An invalid path parameter passed in. It must be alphanumeric."; + } + } String urlString = String.format(base + this.template, values); URL url = null; try { url = new URL(urlString); } catch (MalformedURLException e) { - assert false : "An invalid URL template indicates a bug in the SDK."; + assert false : "A valid URL could not be constructed from the provided parameters."; } return url; } /** - * Build a URL with Query String and URL Parameters. + * Build a URL with Query String and numeric URL Parameters. * @param base base URL * @param queryString query string * @param values URL Parameters * @return URL */ public URL buildWithQuery(String base, String queryString, Object... values) { + for (Object value: values) { + String valueString = String.valueOf(value); + if (!NUMERIC.matcher(valueString).matches()) { + assert false : "An invalid path param passed in. It must be numeric."; + } + } + String urlString = String.format(base + this.template, values) + queryString; + URL url = null; + try { + url = new URL(urlString); + } catch (MalformedURLException e) { + assert false : "A valid URL could not be constructed from the provided parameters."; + } + + return url; + } + + /** + * Build a URL with Query String and alphanumeric URL Parameters. + * @param base base URL + * @param queryString query string + * @param values URL Parameters + * @return URL + */ + public URL buildAlphaWithQuery(String base, String queryString, Object... values) { + for (Object value: values) { + String valueString = String.valueOf(value); + if (!ALPHA_NUMERIC.matcher(valueString).matches()) { + assert false : "An invalid path param passed in. It must be alphanumeric."; + } + } String urlString = String.format(base + this.template, values) + queryString; URL url = null; try { url = new URL(urlString); } catch (MalformedURLException e) { - assert false : "An invalid URL template indicates a bug in the SDK."; + assert false : "A valid URL could not be constructed from the provided parameters."; } return url; diff --git a/src/test/java/com/box/sdk/BatchAPIRequestTest.java b/src/test/java/com/box/sdk/BatchAPIRequestTest.java index 675d4e0ee..9d451af4b 100644 --- a/src/test/java/com/box/sdk/BatchAPIRequestTest.java +++ b/src/test/java/com/box/sdk/BatchAPIRequestTest.java @@ -185,7 +185,7 @@ public void testMixedBodyTypesWorks() { requests.add(createAppUserRequest); //Update Metadata Template request - uses JSON array as body - URL updateMetadataTemplateURL = MetadataTemplate.METADATA_TEMPLATE_URL_TEMPLATE.build(api.getBaseURL(), + URL updateMetadataTemplateURL = MetadataTemplate.METADATA_TEMPLATE_URL_TEMPLATE.buildAlpha(api.getBaseURL(), "global", "properties"); BoxJSONRequest updateMetadataTemplateRequest = new BoxJSONRequest(updateMetadataTemplateURL, HttpMethod.PUT); updateMetadataTemplateRequest.setBody("[{\"op\":\"removeField\",\"fieldKey\":\"foo\"}]"); @@ -267,7 +267,7 @@ public void testBatchRequestEndToEnd() throws IOException { .withHeader("Content-Type", "application/json") .withBody(response))); - URL addMetadataOnFile = BoxFile.METADATA_URL_TEMPLATE.build(this.api.getBaseURL(), + URL addMetadataOnFile = BoxFile.METADATA_URL_TEMPLATE.buildAlpha(this.api.getBaseURL(), fileID, scope, template); BoxAPIRequest createMetadataRequest = new BoxAPIRequest(addMetadataOnFile, HttpMethod.POST); createMetadataRequest.setBody(metadataObject.toString()); diff --git a/src/test/java/com/box/sdk/BoxURLTemplateTest.java b/src/test/java/com/box/sdk/BoxURLTemplateTest.java new file mode 100644 index 000000000..e8a465d5d --- /dev/null +++ b/src/test/java/com/box/sdk/BoxURLTemplateTest.java @@ -0,0 +1,73 @@ +package com.box.sdk; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import java.net.URL; + +/** + * Unit tests for {@link URLTemplate}. + */ +public class BoxURLTemplateTest { + public static final URLTemplate ADD_COMMENT_URL_TEMPLATE = new URLTemplate("test/%s"); + public static final String BASE_URL = TestConfig.getAPIConnection().getBaseURL(); + + /** + * Unit test for {@link URLTemplate#build(String...)} + */ + @Test + @Category(UnitTest.class) + public void testBuildSucceeds() { + String param = "12345"; + URLTemplate template = new URLTemplate("test/%s"); + URL url = template.build(BASE_URL, param); + String expectedURL = BASE_URL + "test/" + param; + Assert.assertEquals(url.toString(), expectedURL); + } + + /** + * Unit test for {@link URLTemplate#build(String...)} + */ + @Test + @Category(UnitTest.class) + public void testBuildFails() { + URLTemplate template = new URLTemplate("test/%s"); + try { + URL url = template.build(BASE_URL, "1fdsa45"); + } catch (AssertionError e) { + Assert.assertEquals("An invalid path parameter passed in. It must be numeric.", e.getMessage()); + return; + } + Assert.fail("Never threw an AssertionError"); + } + + /** + * Unit test for {@link URLTemplate#buildAlpha(String...)} + */ + @Test + @Category(UnitTest.class) + public void testBuildAlphaSucceeds() { + String param = "$123-45_67"; + URLTemplate template = new URLTemplate("test/%s"); + URL url = template.buildAlpha(BASE_URL, param); + String expectedURL = BASE_URL + "test/" + param; + Assert.assertEquals(url.toString(), expectedURL); + } + + /** + * Unit test for {@link URLTemplate#buildAlpha(String...)} + */ + @Test + @Category(UnitTest.class) + public void testBuildAlphaFails() { + URLTemplate template = new URLTemplate("test/%s"); + try { + URL url = template.buildAlpha(BASE_URL, "1234.45/43/5"); + } catch (AssertionError e) { + Assert.assertEquals("An invalid path parameter passed in. It must be alphanumeric.", e.getMessage()); + return; + } + Assert.fail("Never threw an AssertionError"); + } +}