diff --git a/CHANGELOG.md b/CHANGELOG.md index 98b795571..a330fa125 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,9 @@ All notable changes to AET will be documented in this file. ## Unreleased **List of changes that are finished but not yet released in any final version.** -- [PR-526](https://github.com/Cognifide/aet/pull/526) Added sending urls to collectors in packets. ([#431](https://github.com/Cognifide/aet/issues/431)) +- [PR-524](https://github.com/Cognifide/aet/pull/524) Remove handling unescaped URLs in suites +- [PR-526](https://github.com/Cognifide/aet/pull/526) Added sending urls to collectors in packets. ([#431](https://github.com/Cognifide/aet/issues/431)) - [PR-565](https://github.com/Cognifide/aet/pull/565) Fixing styles of note-editor icon ([#371](https://github.com/Cognifide/aet/issues/371)) - [PR-567](https://github.com/Cognifide/aet/pull/567) Added missing tooltips in the report - "Previous url" and "Next url" ([#566](https://github.com/Cognifide/aet/issues/566)) - [PR-563](https://github.com/Cognifide/aet/pull/563) Fixed bug with tooltips for "Re-run" buttons. ([#476](https://github.com/Cognifide/aet/issues/476)) diff --git a/documentation/src/main/wiki/UpgradeNotes.md b/documentation/src/main/wiki/UpgradeNotes.md index 12da62ee8..4cbde29c3 100644 --- a/documentation/src/main/wiki/UpgradeNotes.md +++ b/documentation/src/main/wiki/UpgradeNotes.md @@ -7,6 +7,15 @@ You may see all changes in the [Changelog](https://github.com/Cognifide/aet/blob ## Unreleased +Changes: +* All suites have to be valid xmls. This is mostly related to URL's whch can have special characters like '&'. Such characters have to be escaped. +This changes requires to update all suites and escape all URL's inside. +For example - given suite line: +```` +has to be updated into: +```` +Related issue: [#441](https://github.com/Cognifide/aet/issues/441) + ## Version 3.2.1 Changes: * Counting line and column number of an accessibility issue occurrence in [[Accessibility Collector|AccessibilityCollector]] has been improved. diff --git a/documentation/src/main/wiki/Urls.md b/documentation/src/main/wiki/Urls.md index 80fa98ed3..4aedaf0eb 100644 --- a/documentation/src/main/wiki/Urls.md +++ b/documentation/src/main/wiki/Urls.md @@ -9,7 +9,7 @@ A single url which will be processed by a particular test. | Attribute name | Description | Mandatory | | -------------- | ----------- | --------- | -| `href` | A page address (also see note under the name attribute) | yes | +| `href` | A page address (also see note under the name attribute). If there are some XML-specific characters (e.g. `&`) in the url, they have to be escaped. | yes | | `name` | An identifier for the url. It is used to identify the data for the url. If provided it should be unique for each test in the test suite. If not provided it is set to the encoded `href` value. It should consists of letters, digits and/or characters: `-`, `_` only. Note that if `href=""` with the provided url `name` attribute and the suite `domain` attribute is also valid. | no | | ~~`description`~~ | ~~An additional description for the url that will be shown in the html report~~ | no longer supported | diff --git a/test-executor/src/main/java/com/cognifide/aet/executor/xmlparser/xml/XmlTestSuiteParser.java b/test-executor/src/main/java/com/cognifide/aet/executor/xmlparser/xml/XmlTestSuiteParser.java index 9988cf333..9e8624003 100644 --- a/test-executor/src/main/java/com/cognifide/aet/executor/xmlparser/xml/XmlTestSuiteParser.java +++ b/test-executor/src/main/java/com/cognifide/aet/executor/xmlparser/xml/XmlTestSuiteParser.java @@ -22,12 +22,10 @@ import com.cognifide.aet.executor.xmlparser.xml.models.Collect; import com.cognifide.aet.executor.xmlparser.xml.models.Compare; import com.cognifide.aet.executor.xmlparser.xml.models.TestSuite; -import com.cognifide.aet.executor.xmlparser.xml.utils.EscapeUtils; import com.google.common.base.Charsets; import com.google.common.io.Files; import java.io.File; import java.io.IOException; -import org.apache.commons.lang3.StringUtils; import org.simpleframework.xml.Serializer; import org.simpleframework.xml.convert.Registry; import org.simpleframework.xml.convert.RegistryStrategy; @@ -49,8 +47,7 @@ public TestSuiteRun parse(File testSuiteFile) throws ParseException { public TestSuiteRun parse(String testSuiteString) throws ParseException { try { Serializer serializer = getSerializer(); - TestSuite testSuite = serializer - .read(TestSuite.class, EscapeUtils.escapeUrls(testSuiteString)); + TestSuite testSuite = serializer.read(TestSuite.class, testSuiteString); return testSuite.adaptToTestSuiteRun(); } catch (Exception e) { String message = String.format("Something is wrong with your suite definition! Detected errors: %n%s", e.getMessage()); diff --git a/test-executor/src/main/java/com/cognifide/aet/executor/xmlparser/xml/models/ModelConverterUtils.java b/test-executor/src/main/java/com/cognifide/aet/executor/xmlparser/xml/models/ModelConverterUtils.java index 29b52f000..5bd6731f3 100644 --- a/test-executor/src/main/java/com/cognifide/aet/executor/xmlparser/xml/models/ModelConverterUtils.java +++ b/test-executor/src/main/java/com/cognifide/aet/executor/xmlparser/xml/models/ModelConverterUtils.java @@ -30,6 +30,11 @@ public final class ModelConverterUtils { private static final String EMPTY_URL_MESSAGE = "Empty url"; private static final String DUPLICATED_URL_MESSAGE = "Duplicated url"; + private static final Integer DESCRIPTION_MAX_LENGTH = 40; + private static final String DESCRIPTION_PATTERN = "^[a-zA-Z0-9\\_\\- ]+$"; + private static final String TOO_LONG_DESCRIPTION_MSG = String + .format("URL attribute description is longer than max %d chars", DESCRIPTION_MAX_LENGTH); + private static final String INVALID_DESCRIPTION_MSG = "Invalid URL description provided"; private ModelConverterUtils() { // empty utils constructor @@ -54,6 +59,7 @@ static List extendUrlsList(List urls) } else { extendedUrls.add(extendedUrl); } + validateUrlDescription(builder, extendedUrl); } if (builder.length() > 0) { throw new ParseException(builder.toString()); @@ -71,7 +77,18 @@ private static String extractUrlName(Url url) throws UnsupportedEncodingExceptio return urlName; } - private static void buildErrorMessage(StringBuilder builder, String baseMessage, ExtendedUrl url) { + private static void validateUrlDescription(StringBuilder builder, ExtendedUrl url) { + String description = url.getDescription(); + if (StringUtils.isNotEmpty(description) && description.length() > DESCRIPTION_MAX_LENGTH) { + buildErrorMessage(builder, TOO_LONG_DESCRIPTION_MSG, url); + } + if (StringUtils.isNotEmpty(description) && !description.matches(DESCRIPTION_PATTERN)) { + buildErrorMessage(builder, INVALID_DESCRIPTION_MSG, url); + } + } + + private static void buildErrorMessage(StringBuilder builder, String baseMessage, + ExtendedUrl url) { builder.append(baseMessage); formatIfNotBlank(builder, ", with URL: %s", url.getUrl()); formatIfNotBlank(builder, ", with name: %s", url.getName()); diff --git a/test-executor/src/main/java/com/cognifide/aet/executor/xmlparser/xml/utils/EscapeUtils.java b/test-executor/src/main/java/com/cognifide/aet/executor/xmlparser/xml/utils/EscapeUtils.java deleted file mode 100644 index c52c58ca6..000000000 --- a/test-executor/src/main/java/com/cognifide/aet/executor/xmlparser/xml/utils/EscapeUtils.java +++ /dev/null @@ -1,78 +0,0 @@ -/** - * AET - * - * Copyright (C) 2013 Cognifide Limited - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except - * in compliance with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License - * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express - * or implied. See the License for the specific language governing permissions and limitations under - * the License. - */ -package com.cognifide.aet.executor.xmlparser.xml.utils; - -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import org.apache.commons.lang3.StringEscapeUtils; -import org.apache.commons.lang3.StringUtils; - -public final class EscapeUtils { - - private static final Pattern URL_TAG_PATTERN = Pattern.compile("()"); - - private static final Pattern ATTRIBUTES_PATTERN = Pattern.compile("([^=\"]*=\"[^\"]*\")(\\s*)"); - - private static final String DESCRIPTION_PATTERN = "^[a-zA-Z0-9\\_\\- ]+$"; - - private static final Integer DESCRIPTION_MAX_LENGTH = 40; - - private EscapeUtils() { - // empty utils constructor - } - - public static String escapeUrls(String xmlString) { - Matcher matcher = URL_TAG_PATTERN.matcher(xmlString); - StringBuffer urlTagStringBuffer = new StringBuffer(); - while (matcher.find()) { - String attrs = matcher.group(3); - Matcher attrMatcher = ATTRIBUTES_PATTERN.matcher(attrs); - StringBuffer attrsStringBuffer = new StringBuffer(); - while (attrMatcher.find()) { - String attr = attrMatcher.group(1); - if (attr.startsWith("description=")) { - validateAttr(attr); - } - - if (attr.startsWith("href=")) { - attr = String.format("href=\"%s\"", - StringEscapeUtils.escapeXml11(attr.substring(6, attr.length() - 1))); - } - attrMatcher.appendReplacement(attrsStringBuffer, attr + " "); - } - attrMatcher.appendTail(attrsStringBuffer); - String urlString = String.format("", attrsStringBuffer.toString()); - matcher.appendReplacement(urlTagStringBuffer, urlString); - } - matcher.appendTail(urlTagStringBuffer); - return urlTagStringBuffer.toString(); - } - - public static void validateAttr(String attribute) { - String description = attribute.substring(13, attribute.length() - 1); - - if (StringUtils.isNotEmpty(description) && description.length() > DESCRIPTION_MAX_LENGTH) { - throw new IllegalArgumentException(String.format( - "URL attribute description is longer than max %d chars: %s", DESCRIPTION_MAX_LENGTH, - description)); - } - if (StringUtils.isNotEmpty(description) && !description.matches(DESCRIPTION_PATTERN)) { - throw new IllegalArgumentException(String.format("Invalid URL description provided: %s", - description)); - } - } - -} diff --git a/test-executor/src/test/java/com/cognifide/aet/executor/xmlparser/xml/XmlTestSuiteParserTest.java b/test-executor/src/test/java/com/cognifide/aet/executor/xmlparser/xml/XmlTestSuiteParserTest.java index c68b38a0b..3890f557d 100644 --- a/test-executor/src/test/java/com/cognifide/aet/executor/xmlparser/xml/XmlTestSuiteParserTest.java +++ b/test-executor/src/test/java/com/cognifide/aet/executor/xmlparser/xml/XmlTestSuiteParserTest.java @@ -58,4 +58,14 @@ public void testParse_withWrongUrl_inTwoTestCase_expectParseException() throws E TestSuiteRun testSuite = testSuiteParser.parse(testSuitFile); } + @Test(expected = ParseException.class) + public void testParse_withUnescapedUrls_expectParseException() throws Exception { + URL resourceURL = getClass().getResource("/testSuiteWithUnescapedUrls.xml"); + File testSuiteFile = new File(resourceURL.toURI()); + + TestSuiteParser testSuiteParser = new XmlTestSuiteParser(); + + TestSuiteRun testSuite = testSuiteParser.parse(testSuiteFile); + } + } diff --git a/test-executor/src/test/java/com/cognifide/aet/executor/xmlparser/xml/utils/EscapeUtilsTest.java b/test-executor/src/test/java/com/cognifide/aet/executor/xmlparser/xml/utils/EscapeUtilsTest.java deleted file mode 100644 index 1bede4131..000000000 --- a/test-executor/src/test/java/com/cognifide/aet/executor/xmlparser/xml/utils/EscapeUtilsTest.java +++ /dev/null @@ -1,42 +0,0 @@ -/** - * AET - * - * Copyright (C) 2013 Cognifide Limited - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except - * in compliance with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License - * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express - * or implied. See the License for the specific language governing permissions and limitations under - * the License. - */ -package com.cognifide.aet.executor.xmlparser.xml.utils; - -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; - -import java.io.FileInputStream; -import java.io.IOException; -import java.net.URL; -import org.apache.commons.io.IOUtils; -import org.junit.Test; - -public class EscapeUtilsTest { - - @Test - public void escapeUrls_expectEscapedResults() throws IOException { - String xmlString = readContentFromFile("/testSuite_escapedUrls.xml"); - String escapedXmlString = EscapeUtils.escapeUrls(xmlString); - String resultString = readContentFromFile("/testSuite_escapedUrls_result.xml"); - assertThat(escapedXmlString, is(resultString)); - } - - private String readContentFromFile(String path) throws IOException { - URL resourceURL = getClass().getResource(path); - return IOUtils.toString(new FileInputStream(resourceURL.getPath()), "UTF-8"); - } - -} diff --git a/test-executor/src/test/resources/testSuite.xml b/test-executor/src/test/resources/testSuite.xml index 8fe866acd..9b773db86 100644 --- a/test-executor/src/test/resources/testSuite.xml +++ b/test-executor/src/test/resources/testSuite.xml @@ -60,8 +60,8 @@ - - + + \ No newline at end of file diff --git a/test-executor/src/test/resources/testSuiteWithUnescapedUrls.xml b/test-executor/src/test/resources/testSuiteWithUnescapedUrls.xml new file mode 100644 index 000000000..8fe866acd --- /dev/null +++ b/test-executor/src/test/resources/testSuiteWithUnescapedUrls.xml @@ -0,0 +1,67 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/test-executor/src/test/resources/testSuite_escapedUrls.xml b/test-executor/src/test/resources/testSuite_escapedUrls.xml deleted file mode 100644 index 36a56b3e8..000000000 --- a/test-executor/src/test/resources/testSuite_escapedUrls.xml +++ /dev/null @@ -1,39 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/test-executor/src/test/resources/testSuite_escapedUrls_result.xml b/test-executor/src/test/resources/testSuite_escapedUrls_result.xml deleted file mode 100644 index c0c346bef..000000000 --- a/test-executor/src/test/resources/testSuite_escapedUrls_result.xml +++ /dev/null @@ -1,39 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file