Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ jdk:
- oraclejdk11

install: true
script: mvn test -DskipAssembly
script: mvn test -DskipAssembly -B

after_success:
# TODO delete following if statement after fix of https://github.com/cobertura/cobertura/issues/271
- if [ "$TRAVIS_JDK_VERSION" == "openjdk8" ] || [ "$TRAVIS_JDK_VERSION" == "oraclejdk8" ]; then
mvn cobertura:cobertura org.eluder.coveralls:coveralls-maven-plugin:report com.updateimpact:updateimpact-maven-plugin:submit -Ptravis-coveralls,update-impact -DskipAssembly;
mvn cobertura:cobertura org.eluder.coveralls:coveralls-maven-plugin:report com.updateimpact:updateimpact-maven-plugin:submit -Ptravis-coveralls,update-impact -DskipAssembly -B;
else
echo "Not reporting coverage for $TRAVIS_JDK_VERSION due to incompatibility or to save performance";
fi;
Expand Down
1 change: 0 additions & 1 deletion core/src/test/resources/log4j2.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,5 @@
<Root level="info">
<AppenderRef ref="STDOUT"/>
</Root>
<Logger name="org.apache.struts2.views.xslt" level="debug"/>
</Loggers>
</Configuration>
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,47 @@
package org.apache.struts2.tiles;

import org.apache.tiles.request.locale.PostfixedApplicationResource;
import org.apache.tiles.request.locale.URLApplicationResource;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
import java.net.URL;

public class StrutsApplicationResource extends PostfixedApplicationResource {

private final URL url;
private final File file;

/**
* fixes WW-5011
* @return file path for "file" protocol elsewhere url path as before to keep backward-compatibility
* @see URLApplicationResource#getFile(URL)
*/
private static String getFilePath(URL url) {
String path = url.getPath();
if (!"file".equals(url.getProtocol())) {
return path;
}
try {
// fixes WW-5011 because includes ref in path - like URLApplicationResource#getFile(URL)
path = (new URI(url.toExternalForm())).getSchemeSpecificPart();
} catch (Exception e) {
// fallback solution
if (url.getRef() != null && !new File(path).exists()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment about sub-context representation and anchor/fragment "#".

A larger concern is there is now a File existence check happening here. That seems very risky as the URL can represent a non-file protocol resource.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But getLastModified method already considers it should be a file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems getLastModified() considers the possibility that resource (path) is a file, but it doesn't assume it has to be a file.

If the path doesn't exist as a file (e.g. nonexistent file or non-file URL) then exists() will be false and getLastModified() just returns 0. (As a side note the base Tiles interface also allows for an IOException to be thrown if there's no modified date available ... but Struts has attempted to avoid that).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed :) - learnt from tiles URLApplicationResource.

// it's like WW-5011
path += "#" + url.getRef();
}
}

return path;
}

public StrutsApplicationResource(URL url) {
super(url.getPath());
super(getFilePath(url));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last suggestion 😄 for the constructor (avoids calling getFilePath twice/regenerating the path string):

final String urlPath = getFilePath(url);
super(urlPath);
this.url = url;
this.file = new File(urlPath);

this.url = url;
this.file = new File(getFilePath(url));
}

@Override
Expand All @@ -41,7 +69,6 @@ public InputStream getInputStream() throws IOException {

@Override
public long getLastModified() throws IOException {
File file = new File(url.getPath());
if (file.exists()) {
return file.lastModified();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you 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 org.apache.struts2.tiles;

import org.apache.tiles.request.ApplicationResource;
import org.junit.Assert;
import org.junit.Test;

import java.net.URL;

public class StrutsApplicationResourceTest {

@Test
public void testWW5011fix() throws Exception {
URL resource = getClass().getClassLoader().getResource("emptyTiles.xml");
ApplicationResource ar = new StrutsApplicationResource(resource);
Assert.assertNotEquals(0, ar.getLastModified());

resource = getClass().getClassLoader().getResource("emptyTiles###2.xml");
ar = new StrutsApplicationResource(resource);
Assert.assertNotEquals(0, ar.getLastModified());

resource = getClass().getClassLoader().getResource("emptyTiles.xml");
ar = new StrutsApplicationResource(new URL(resource + "#ref1"));
Assert.assertNotEquals(0, ar.getLastModified());
}
}
22 changes: 22 additions & 0 deletions plugins/tiles/src/test/resources/emptyTiles###2.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you 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.
*/
-->
<tiles-definitions></tiles-definitions>
22 changes: 22 additions & 0 deletions plugins/tiles/src/test/resources/emptyTiles.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you 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.
*/
-->
<tiles-definitions></tiles-definitions>