Merge pull request #524 from wblachowski/bugfix/not-escape-urls-xml

Remove handling unescaped URLs in suites

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))

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:
+``<url href="https://en.wikipedia.org/wiki/Main_Page?a=b&c=d"/>``
+has to be updated into:
+``<url href="https://en.wikipedia.org/wiki/Main_Page?a=b&amp;c=d"/>``
+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. 

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 |
 

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());

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<ExtendedUrl> extendUrlsList(List<Url> 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());

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);
+  }
+
 }

test-executor/src/test/resources/testSuite.xml

@@ -60,8 +60,8 @@
 		</compare>
 		<urls>
 			<url href="http://www.google.pl" description="Uncle Google"/>
-			<url href="https://www.google.pl/search?q=googe&oq=googe&aqs=chrome..69i57j0l5.767j0j7&sourceid=chrome&es_sm=93&ie=UTF-8"/>
-			<url href="http://www.google.pl/search?g=<>'" />
+			<url href="https://www.google.pl/search?q=googe&amp;oq=googe&amp;aqs=chrome..69i57j0l5.767j0j7&amp;sourceid=chrome&amp;es_sm=93&amp;ie=UTF-8"/>
+			<url href="http://www.google.pl/search?g=&lt;&gt;&apos;" />
 		</urls>
 	</test>
 </suite>

test-executor/src/test/resources/testSuiteWithUnescapedUrls.xml

@@ -0,0 +1,67 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    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.
+
+-->
+<suite name="TS1" company="cognifide" project="project">
+	<test name="test1" useProxy="true">
+		<collect>
+			<open/>
+			<sleep duration="1500"/>
+			<resolution width="640" height="1136" />
+			<screen name="iphone5"/>
+			<source/>
+			<status-codes/>
+			<js-errors/>
+		</collect>
+		<compare>
+			<!-- executed for fullScreen/iphone5 -->
+			<screen comparator="layout"/>
+			<!-- executed only for pageSource -->
+			<source comparator="w3c"/>
+			<status-codes filterRange="400,999" name="errors"/>
+			<status-codes filterCodes="302" name="redirects"/>
+			<source comparator="source" compareType="all"/>
+			<js-errors/>
+		</compare>
+		<urls>
+			<url href="http://www.cognifide.com" description="Cognifide"/>
+			<url href="http://www.onet.pl" description="Onet"/>
+			<url href="http://www.wp.pl" />
+			<url href="http://m.interia.pl/info#iwa_block=pasek-ding"/>
+			<url href="http://www.google.com/" description="Google"/>
+		</urls>
+	</test>
+	<test name="test2" useProxy="false">
+		<collect>
+			<open/>
+			<sleep duration="1500"/>
+			<screen/>
+			<source/>
+		</collect>
+		<compare>
+			<screen/>
+			<source comparator="w3c"/>
+		</compare>
+		<urls>
+			<url href="http://www.google.pl" description="Uncle Google"/>
+			<url href="https://www.google.pl/search?q=googe&oq=googe&aqs=chrome..69i57j0l5.767j0j7&sourceid=chrome&es_sm=93&ie=UTF-8"/>
+			<url href="http://www.google.pl/search?g=<>'" />
+		</urls>
+	</test>
+</suite>