Skip to content

Commit

Permalink
Properly encoded URIs use %20 (#8271)
Browse files Browse the repository at this point in the history
fixes #5360
  • Loading branch information
ajgassner committed May 18, 2020
1 parent f709ac4 commit 96926f1
Show file tree
Hide file tree
Showing 5 changed files with 259 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Optional;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -148,15 +149,20 @@ public static String generateURI(String name, String id) {
try {
builder.append(UI.getCurrent().getUIId()).append(PATH_SEPARATOR);
builder.append(id).append(PATH_SEPARATOR);
builder.append(
URLEncoder.encode(name, StandardCharsets.UTF_8.name()));
builder.append(encodeString(name));
} catch (UnsupportedEncodingException e) {
// UTF8 has to be supported
throw new RuntimeException(e);
}
return builder.toString();
}

private static String encodeString(String name)
throws UnsupportedEncodingException {
return URLEncoder.encode(name, StandardCharsets.UTF_8.name())
.replace("+", "%20");
}

private static Optional<URI> getPathUri(String path) {
int index = path.lastIndexOf('/');
boolean hasPrefix = index >= 0;
Expand All @@ -166,11 +172,8 @@ private static Optional<URI> getPathUri(String path) {
}
String prefix = path.substring(0, index + 1);
String name = path.substring(prefix.length());
// path info returns decoded name but space ' ' remains encoded '+'
name = name.replace('+', ' ');
try {
URI uri = new URI(prefix
+ URLEncoder.encode(name, StandardCharsets.UTF_8.name()));
URI uri = new URI(prefix + encodeString(name));
return Optional.of(uri);
} catch (UnsupportedEncodingException e) {
// UTF8 has to be supported
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,13 @@
*/
package com.vaadin.flow.server;

import javax.servlet.ServletException;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Optional;

import javax.servlet.ServletException;

import net.jcip.annotations.NotThreadSafe;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
Expand All @@ -34,8 +31,6 @@
import com.vaadin.flow.component.UI;
import com.vaadin.flow.internal.CurrentInstance;

import net.jcip.annotations.NotThreadSafe;

@NotThreadSafe
public class StreamResourceRegistryTest {

Expand Down Expand Up @@ -134,18 +129,31 @@ public void registerTwoResourcesWithSameName_resourcesHasDifferentURI() {
}

@Test
public void getResourceUrlIsEncoded() throws UnsupportedEncodingException {
public void getResourceUriIsEncoded_withQueryParams() {
assertResourceUriIsEncoded("a?b=c d&e", "a%3Fb%3Dc%20d%26e");
}

@Test
public void getResourceUriIsEncoded_withContainingPlus() {
assertResourceUriIsEncoded("image++.svg", "image%2B%2B.svg");
}

@Test
public void getResourceUriIsEncoded_withSimpleSpace() {
assertResourceUriIsEncoded("my file.png", "my%20file.png");
}

private void assertResourceUriIsEncoded(String resourceName,
String suffix) {
StreamResourceRegistry registry = new StreamResourceRegistry(session);

StreamResource resource = new StreamResource("a?b=c d&e",
() -> makeEmptyStream());
StreamResource resource = new StreamResource(resourceName,
this::makeEmptyStream);
StreamRegistration registration = registry.registerResource(resource);

URI url = registration.getResourceUri();
String suffix = URLEncoder.encode(resource.getName(),
StandardCharsets.UTF_8.name());
Assert.assertTrue("Resource url is not encoded",
url.toString().endsWith(suffix));
URI uri = registration.getResourceUri();
Assert.assertTrue("Resource URI is not properly encoded",
uri.toString().endsWith(suffix));
}

private InputStream makeEmptyStream() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
package com.vaadin.flow.server.communication;

import javax.servlet.ServletConfig;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.ServletOutputStream;
import java.io.ByteArrayInputStream;
import java.io.IOException;

import net.jcip.annotations.NotThreadSafe;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;

import com.vaadin.flow.component.UI;
import com.vaadin.flow.internal.CurrentInstance;
import com.vaadin.flow.server.MockServletConfig;
import com.vaadin.flow.server.MockVaadinSession;
import com.vaadin.flow.server.StreamResource;
import com.vaadin.flow.server.StreamResourceRegistry;
import com.vaadin.flow.server.VaadinResponse;
import com.vaadin.flow.server.VaadinService;
import com.vaadin.flow.server.VaadinServlet;
import com.vaadin.flow.server.VaadinServletRequest;
import com.vaadin.tests.util.AlwaysLockedVaadinSession;
import com.vaadin.tests.util.MockUI;

import static com.vaadin.flow.server.communication.StreamRequestHandler.DYN_RES_PREFIX;

@NotThreadSafe
public class StreamRequestHandlerTest {

private StreamRequestHandler handler = new StreamRequestHandler();
private MockVaadinSession session;
private VaadinServletRequest request;
private VaadinResponse response;
private StreamResourceRegistry streamResourceRegistry;
private UI ui;

@Before
public void setUp() throws ServletException {
ServletConfig servletConfig = new MockServletConfig();
VaadinServlet servlet = new VaadinServlet();
servlet.init(servletConfig);
VaadinService service = servlet.getService();

session = new AlwaysLockedVaadinSession(service) {
@Override
public StreamResourceRegistry getResourceRegistry() {
return streamResourceRegistry;
}
};
streamResourceRegistry = new StreamResourceRegistry(session);
request = Mockito.mock(VaadinServletRequest.class);
ServletContext servletContext = Mockito.mock(ServletContext.class);
Mockito.when(servletContext.getMimeType(Mockito.anyString()))
.thenReturn(null);
Mockito.when(request.getServletContext()).thenReturn(servletContext);
response = Mockito.mock(VaadinResponse.class);
ui = new MockUI();
UI.setCurrent(ui);
}

@After
public void cleanup() {
CurrentInstance.clearAll();
}

@Test
public void streamResourceNameEndsWithPluses_streamFactory_resourceIsStreamed()
throws IOException {
testStreamResourceInputStreamFactory("end with multiple pluses",
"readme++.md");
}

@Test
public void streamResourceNameEndsWithPluses_resourceWriter_resourceIsStreamed()
throws IOException {
testStreamResourceStreamResourceWriter("end with multiple pluses",
"readme++.md");
}

@Test
public void streamResourceNameContainsSpaceEndsWithPluses_streamFactory_resourceIsStreamed()
throws IOException {
testStreamResourceInputStreamFactory(
"end with space and multiple pluses", "readme ++.md");
}

@Test
public void streamResourceNameContainsSpaceEndsWithPluses_resourceWriter_resourceIsStreamed()
throws IOException {
testStreamResourceStreamResourceWriter(
"end with space and multiple pluses", "readme ++.md");
}

@Test
public void streamResourceNameEndsInPlus_streamFactory_resourceIsStreamed()
throws IOException {
testStreamResourceInputStreamFactory("end in plus", "readme+.md");
}

@Test
public void streamResourceNameEndsInPlus_resourceWriter_resourceIsStreamed()
throws IOException {
testStreamResourceStreamResourceWriter("end in plus", "readme+.md");
}

@Test
public void streamResourceNameContainsPlus_streamFactory_resourceIsStreamed()
throws IOException {
testStreamResourceInputStreamFactory("plus in middle",
"readme+mine.md");
}

@Test
public void streamResourceNameContainsPlus_resourceWriter_resourceIsStreamed()
throws IOException {
testStreamResourceStreamResourceWriter("plus in middle",
"readme+mine.md");
}

@Test
public void streamResourceNameContainsPlusAndSpaces_streamFactory_resourceIsStreamed()
throws IOException {
testStreamResourceInputStreamFactory("plus surrounded by spaces",
"readme + mine.md");
}

@Test
public void streamResourceNameContainsPlusAndSpaces_resourceWriter_resourceIsStreamed()
throws IOException {
testStreamResourceStreamResourceWriter("plus surrounded by spaces",
"readme + mine.md");
}

private void testStreamResourceInputStreamFactory(String testString,
String fileName) throws IOException {

final byte[] testBytes = testString.getBytes();
StreamResource res = new StreamResource(fileName,
() -> new ByteArrayInputStream(testBytes));

streamResourceRegistry.registerResource(res);

ServletOutputStream outputStream = Mockito
.mock(ServletOutputStream.class);
Mockito.when(response.getOutputStream()).thenReturn(outputStream);
Mockito.when(request.getPathInfo()).thenReturn(
String.format("/%s%s/%s/%s", DYN_RES_PREFIX,
ui.getId().orElse("-1"), res.getId(), res.getName()));

handler.handleRequest(session, request, response);

Mockito.verify(response).getOutputStream();

ArgumentCaptor<byte[]> argument = ArgumentCaptor.forClass(byte[].class);
Mockito.verify(outputStream)
.write(argument.capture(), Mockito.anyInt(), Mockito.anyInt());

byte[] buf = new byte[1024];
for (int i = 0; i < testBytes.length; i++) {
buf[i] = testBytes[i];
}
Assert.assertArrayEquals("Output differed from expected", buf,
argument.getValue());
Mockito.verify(response).setCacheTime(Mockito.anyInt());
Mockito.verify(response).setContentType("application/octet-stream");
}

private void testStreamResourceStreamResourceWriter(String testString,
String fileName) throws IOException {

final byte[] testBytes = testString.getBytes();
StreamResource res = new StreamResource(fileName,
(stream, session) -> stream.write(testBytes));

streamResourceRegistry.registerResource(res);

ServletOutputStream outputStream = Mockito
.mock(ServletOutputStream.class);
Mockito.when(response.getOutputStream()).thenReturn(outputStream);
Mockito.when(request.getPathInfo()).thenReturn(
String.format("/%s%s/%s/%s", DYN_RES_PREFIX,
ui.getId().orElse("-1"), res.getId(), res.getName()));

handler.handleRequest(session, request, response);

Mockito.verify(response).getOutputStream();

ArgumentCaptor<byte[]> argument = ArgumentCaptor.forClass(byte[].class);
Mockito.verify(outputStream).write(argument.capture());

Assert.assertArrayEquals("Output differed from expected", testBytes,
argument.getValue());
Mockito.verify(response).setCacheTime(Mockito.anyInt());
Mockito.verify(response).setContentType("application/octet-stream");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,18 @@ public StreamResourceView() {
StreamResource resource = new StreamResource("filename",
() -> new ByteArrayInputStream(
"foo".getBytes(StandardCharsets.UTF_8)));
Anchor download = new Anchor("", "Download file");
Anchor download = new Anchor("", "Download filename");
download.setHref(resource);
download.setId("link");
add(download);

StreamResource plusResource = new StreamResource("file+.jpg",
() -> new ByteArrayInputStream(
"foo".getBytes(StandardCharsets.UTF_8)));
Anchor plusDownload = new Anchor("", "Download file+.jpg");
plusDownload.setHref(plusResource);
plusDownload.setId("plus-link");

add(download, plusDownload);

NativeButton reattach = new NativeButton("Remove and add back",
event -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

import org.apache.commons.io.FilenameUtils;
import org.apache.commons.io.IOUtils;
import org.junit.Assert;
import org.junit.Test;
Expand All @@ -34,7 +35,14 @@ public class StreamResourceIT extends AbstractStreamResourceIT {
public void getDynamicVaadinResource() throws IOException {
open();

assertDownloadedContent();
assertDownloadedContent("link", "filename");
}

@Test
public void getDynamicVaadinPlusResource() throws IOException {
open();

assertDownloadedContent("plus-link", "file%2B.jpg");
}

@Test
Expand All @@ -44,11 +52,11 @@ public void detact_attachALink_getDynamicVaadinResource()

findElement(By.id("detach-attach")).click();

assertDownloadedContent();
assertDownloadedContent("link", "filename");
}

private void assertDownloadedContent() throws IOException {
WebElement link = findElement(By.id("link"));
private void assertDownloadedContent(String downloadId, String filename) throws IOException {
WebElement link = findElement(By.id(downloadId));
Assert.assertEquals("Anchor element should have router-ignore " +
"attribute", "",
link.getAttribute("router-ignore"));
Expand All @@ -62,6 +70,8 @@ private void assertDownloadedContent() throws IOException {
String text = lines.stream().collect(Collectors.joining());
Assert.assertEquals("foo", text);
}

Assert.assertEquals(filename, FilenameUtils.getName(url));
}

}

0 comments on commit 96926f1

Please sign in to comment.