Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Issue #32 : Fixes getImageURIString to handle images with query parameters #33

Merged
merged 1 commit into from

2 participants

@sripathikrishnan

One of my css files had a background image like this -
background : url(/image/with/query/parameters/image.png?a=b&c=d)

When I ran CSSEmbedder with a http url, the code skipped the image - even though its valid to specify query parameters in css images.

I verified the existing test cases, and added a couple two verify the above scenario.

@sripathikrishnan sripathikrishnan commented on the diff
tests/net/nczonline/web/cssembed/CSSURLEmbedderTest.java
@@ -196,7 +196,7 @@ public class CSSURLEmbedderTest {
@Test
public void testReadFromAndWriteToSameFile() throws IOException {
- String filename = CSSURLEmbedderTest.class.getResource("samefiletest.css").getPath().replace("%20", " ");
+ String filename = this.getClass().getClassLoader().getResource("samefiletest.css").getPath().replace("%20", " ");

This was failing on my machine, and I presume on other machines as well. Since the image is at the root of the classpath, resolving it via the classloader will work okay on all machines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nzakas nzakas merged commit 6111b05 into nzakas:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
30 src/net/nczonline/web/cssembed/CSSURLEmbedder.java
@@ -320,13 +320,10 @@ public void embedImages(Writer out, String root) throws IOException {
* @return The appropriate data URI to use.
* @throws java.io.IOException
*/
- private String getImageURIString(String url, String originalUrl) throws IOException {
-
- //check the extension - only encode for images
- String fileType = url.substring(url.lastIndexOf(".") + 1);
+ String getImageURIString(String url, String originalUrl) throws IOException {
//it's an image, so encode it
- if (imageTypes.contains(fileType)){
+ if (isImage(url)){
DataURIGenerator.setVerbose(verbose);
@@ -392,8 +389,27 @@ private String getImageURIString(String url, String originalUrl) throws IOExcept
}
}
-
- private String getFilename(String path){
+
+ /*
+ * Detects if the given url represents an image
+ * This method simply checks the file extension.
+ * A better way to detect an image is via content type response headers or by content sniffing,
+ * but both are expensive approaches. We can do without them for now.
+ */
+ static boolean isImage(String url) {
+ int startPos = url.lastIndexOf(".") + 1;
+ /*
+ * Some images are of the form some-image.png?parameter=value
+ */
+ int endPos = url.lastIndexOf("?");
+ if(endPos == -1 || endPos <= startPos) {
+ endPos = url.length();
+ }
+ String fileType = url.substring(startPos, endPos);
+ return imageTypes.contains(fileType);
+ }
+
+ private String getFilename(String path){
if (path.indexOf("/") > -1){
return path.substring(path.lastIndexOf("/")+1);
} else if (path.indexOf("\\") > -1){
View
49 tests/net/nczonline/web/cssembed/CSSURLEmbedderTest.java
@@ -196,7 +196,7 @@ public void testAbsoluteLocalFileOverMaxImageSize() throws IOException {
@Test
public void testReadFromAndWriteToSameFile() throws IOException {
- String filename = CSSURLEmbedderTest.class.getResource("samefiletest.css").getPath().replace("%20", " ");
+ String filename = this.getClass().getClassLoader().getResource("samefiletest.css").getPath().replace("%20", " ");

This was failing on my machine, and I presume on other machines as well. Since the image is at the root of the classpath, resolving it via the classloader will work okay on all machines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
File file = new File(filename);
Reader in = new InputStreamReader(new FileInputStream(file));
@@ -232,4 +232,51 @@ public void testRegularUrlWithMhtml() throws IOException {
String result = writer.toString();
assertEquals("background: url(folder.txt);", result);
}
+
+ @Test
+ public void testRemoteUrlWithQueryString() throws IOException {
+ final String expectedUrl = "http://some-http-server.com/image/with/query/parameters/image.png?a=b&c=d";
+ String code = "background : url(/image/with/query/parameters/image.png?a=b&c=d)";
+
+ StringWriter writer = new StringWriter();
+ embedder = new CSSURLEmbedder(new StringReader(code), CSSURLEmbedder.DATAURI_OPTION, true, 0, 200) {
+ /*
+ * Override method to prevent a network call during unit tests
+ */
+ @Override
+ String getImageURIString(String url, String originalUrl) throws IOException {
+ if(url.equals("")) {
+ throw new IllegalArgumentException("Expected URL " + expectedUrl + ", but found " + url);
+ }
+ return "data:image/gif;base64,AAAABBBBCCCCDDDD";
+ }
+ };
+ embedder.embedImages(writer, "http://some-http-server.com/");
+
+ String result = writer.toString();
+ assertEquals("background : url(data:image/gif;base64,AAAABBBBCCCCDDDD)", result);
+ }
+
+ @Test
+ public void testImageDetection() {
+ String tests[] = {
+ "file://path/to/image.png",
+ "http://some.server.com/image.png",
+ "http://some.server.com/image.png?param=legalvalue&anotherparam=anothervalue",
+ "http://some.server.com/image.png?param=illegal.value.with.period"
+ };
+ boolean expectedImage[] = {
+ true, true, true, false
+ };
+
+ for(int i=0; i<tests.length; i++) {
+ if(expectedImage[i]) {
+ assertTrue("Expected " + tests[i] + " to resolve to an image", CSSURLEmbedder.isImage(tests[i]));
+ }
+ else {
+ assertFalse("Did NOT expect " + tests[i] + " to resolve as an image", CSSURLEmbedder.isImage(tests[i]));
+ }
+ }
+
+ }
}
Something went wrong with that request. Please try again.