8302475: Enhance HTTP client file downloading
Reviewed-by: dfuchs, rhalade
This commit is contained in:
Michael McMahon
Henry Jen
parent
34dbb22505
commit
4ae3d8f2cd
@ -45,6 +45,7 @@ import java.net.http.HttpResponse;
|
||||
import java.net.http.HttpResponse.BodyHandler;
|
||||
import java.net.http.HttpResponse.ResponseInfo;
|
||||
import java.net.http.HttpResponse.BodySubscriber;
|
||||
import java.util.Set;
|
||||
import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
import jdk.internal.net.http.ResponseSubscribers.PathSubscriber;
|
||||
@ -229,16 +230,120 @@ public final class ResponseBodyHandlers {
|
||||
static final String DISPOSITION_TYPE = "attachment;";
|
||||
|
||||
/** The "filename" parameter. */
|
||||
static final Pattern FILENAME = Pattern.compile("filename\\s*=", CASE_INSENSITIVE);
|
||||
static final Pattern FILENAME = Pattern.compile("filename\\s*=\\s*", CASE_INSENSITIVE);
|
||||
|
||||
static final List<String> PROHIBITED = List.of(".", "..", "", "~" , "|");
|
||||
|
||||
// Characters disallowed in token values
|
||||
|
||||
static final Set<Character> NOT_ALLOWED_IN_TOKEN = Set.of(
|
||||
'(', ')', '<', '>', '@',
|
||||
',', ';', ':', '\\', '"',
|
||||
'/', '[', ']', '?', '=',
|
||||
'{', '}', ' ', '\t');
|
||||
|
||||
static boolean allowedInToken(char c) {
|
||||
if (NOT_ALLOWED_IN_TOKEN.contains(c))
|
||||
return false;
|
||||
// exclude CTL chars <= 31, == 127, or anything >= 128
|
||||
return isTokenText(c);
|
||||
}
|
||||
|
||||
static final UncheckedIOException unchecked(ResponseInfo rinfo,
|
||||
String msg) {
|
||||
String s = String.format("%s in response [%d, %s]", msg, rinfo.statusCode(), rinfo.headers());
|
||||
return new UncheckedIOException(new IOException(s));
|
||||
}
|
||||
|
||||
static final UncheckedIOException unchecked(String msg) {
|
||||
return new UncheckedIOException(new IOException(msg));
|
||||
}
|
||||
|
||||
// Process a "filename=" parameter, which is either a "token"
|
||||
// or a "quoted string". If a token, it is terminated by a
|
||||
// semicolon or the end of the string.
|
||||
// If a quoted string (surrounded by "" chars then the closing "
|
||||
// terminates the name.
|
||||
// quoted strings may contain quoted-pairs (eg embedded " chars)
|
||||
|
||||
static String processFilename(String src) throws UncheckedIOException {
|
||||
if ("".equals(src))
|
||||
return src;
|
||||
if (src.charAt(0) == '\"') {
|
||||
return processQuotedString(src.substring(1));
|
||||
} else {
|
||||
return processToken(src);
|
||||
}
|
||||
}
|
||||
|
||||
static boolean isTokenText(char c) throws UncheckedIOException {
|
||||
return c > 31 && c < 127;
|
||||
}
|
||||
|
||||
static boolean isQuotedStringText(char c) throws UncheckedIOException {
|
||||
return c > 31;
|
||||
}
|
||||
|
||||
static String processQuotedString(String src) throws UncheckedIOException {
|
||||
boolean inqpair = false;
|
||||
int len = src.length();
|
||||
StringBuilder sb = new StringBuilder();
|
||||
|
||||
for (int i=0; i<len; i++) {
|
||||
char c = src.charAt(i);
|
||||
if (!isQuotedStringText(c)) {
|
||||
throw unchecked("Illegal character");
|
||||
}
|
||||
if (c == '\"') {
|
||||
if (!inqpair) {
|
||||
return sb.toString();
|
||||
} else {
|
||||
sb.append(c);
|
||||
}
|
||||
} else if (c == '\\') {
|
||||
if (!inqpair) {
|
||||
inqpair = true;
|
||||
continue;
|
||||
} else {
|
||||
// the quoted char is '\'
|
||||
sb.append(c);
|
||||
}
|
||||
} else {
|
||||
sb.append(c);
|
||||
}
|
||||
if (inqpair) {
|
||||
inqpair = false;
|
||||
}
|
||||
}
|
||||
// not terminated by "
|
||||
throw unchecked("Invalid quoted string");
|
||||
}
|
||||
|
||||
static String processToken(String src) throws UncheckedIOException {
|
||||
int end = 0;
|
||||
int len = src.length();
|
||||
boolean whitespace = false;
|
||||
|
||||
for (int i=0; i<len; i++) {
|
||||
char c = src.charAt(i);
|
||||
if (c == ';') {
|
||||
break;
|
||||
}
|
||||
if (c == ' ' || c == '\t') {
|
||||
// WS only until ; or end of string
|
||||
whitespace = true;
|
||||
continue;
|
||||
}
|
||||
end++;
|
||||
if (whitespace || !allowedInToken(c)) {
|
||||
String msg = whitespace ? "whitespace must be followed by a semicolon"
|
||||
: c + " is not allowed in a token";
|
||||
throw unchecked(msg);
|
||||
}
|
||||
}
|
||||
return src.substring(0, end);
|
||||
}
|
||||
|
||||
@Override
|
||||
public BodySubscriber<Path> apply(ResponseInfo responseInfo) {
|
||||
String dispoHeader = responseInfo.headers().firstValue("Content-Disposition")
|
||||
@ -256,13 +361,7 @@ public final class ResponseBodyHandlers {
|
||||
}
|
||||
int n = matcher.end();
|
||||
|
||||
int semi = dispoHeader.substring(n).indexOf(";");
|
||||
String filenameParam;
|
||||
if (semi < 0) {
|
||||
filenameParam = dispoHeader.substring(n);
|
||||
} else {
|
||||
filenameParam = dispoHeader.substring(n, n + semi);
|
||||
}
|
||||
String filenameParam = processFilename(dispoHeader.substring(n));
|
||||
|
||||
// strip all but the last path segment
|
||||
int x = filenameParam.lastIndexOf("/");
|
||||
@ -276,19 +375,6 @@ public final class ResponseBodyHandlers {
|
||||
|
||||
filenameParam = filenameParam.trim();
|
||||
|
||||
if (filenameParam.startsWith("\"")) { // quoted-string
|
||||
if (!filenameParam.endsWith("\"") || filenameParam.length() == 1) {
|
||||
throw unchecked(responseInfo,
|
||||
"Badly quoted Content-Disposition filename parameter");
|
||||
}
|
||||
filenameParam = filenameParam.substring(1, filenameParam.length() -1 );
|
||||
} else { // token,
|
||||
if (filenameParam.contains(" ")) { // space disallowed
|
||||
throw unchecked(responseInfo,
|
||||
"unquoted space in Content-Disposition filename parameter");
|
||||
}
|
||||
}
|
||||
|
||||
if (PROHIBITED.contains(filenameParam)) {
|
||||
throw unchecked(responseInfo,
|
||||
"Prohibited Content-Disposition filename parameter:"
|
||||
|
||||
@ -71,7 +71,7 @@ import static org.testng.Assert.fail;
|
||||
/*
|
||||
* @test
|
||||
* @summary Basic test for ofFileDownload
|
||||
* @bug 8196965
|
||||
* @bug 8196965 8302475
|
||||
* @library /test/lib /test/jdk/java/net/httpclient/lib
|
||||
* @build jdk.httpclient.test.lib.http2.Http2TestServer jdk.test.lib.net.SimpleSSLContext
|
||||
* jdk.test.lib.Platform jdk.test.lib.util.FileUtils
|
||||
@ -126,18 +126,18 @@ public class AsFileDownloadTest {
|
||||
{ "024", "attachment; filename=me.txt; filename*=utf-8''you.txt", "me.txt" },
|
||||
{ "025", "attachment; filename=\"m y.txt\"; filename*=utf-8''you.txt", "m y.txt" },
|
||||
|
||||
{ "030", "attachment; filename=foo/file1.txt", "file1.txt" },
|
||||
{ "031", "attachment; filename=foo/bar/file2.txt", "file2.txt" },
|
||||
{ "032", "attachment; filename=baz\\file3.txt", "file3.txt" },
|
||||
{ "033", "attachment; filename=baz\\bar\\file4.txt", "file4.txt" },
|
||||
{ "034", "attachment; filename=x/y\\file5.txt", "file5.txt" },
|
||||
{ "035", "attachment; filename=x/y\\file6.txt", "file6.txt" },
|
||||
{ "036", "attachment; filename=x/y\\z/file7.txt", "file7.txt" },
|
||||
{ "037", "attachment; filename=x/y\\z/\\x/file8.txt", "file8.txt" },
|
||||
{ "038", "attachment; filename=/root/file9.txt", "file9.txt" },
|
||||
{ "039", "attachment; filename=../file10.txt", "file10.txt" },
|
||||
{ "040", "attachment; filename=..\\file11.txt", "file11.txt" },
|
||||
{ "041", "attachment; filename=foo/../../file12.txt", "file12.txt" },
|
||||
{ "030", "attachment; filename=\"foo/file1.txt\"", "file1.txt" },
|
||||
{ "031", "attachment; filename=\"foo/bar/file2.txt\"", "file2.txt" },
|
||||
{ "032", "attachment; filename=\"baz\\\\file3.txt\"", "file3.txt" },
|
||||
{ "033", "attachment; filename=\"baz\\\\bar\\\\file4.txt\"", "file4.txt" },
|
||||
{ "034", "attachment; filename=\"x/y\\\\file5.txt\"", "file5.txt" },
|
||||
{ "035", "attachment; filename=\"x/y\\\\file6.txt\"", "file6.txt" },
|
||||
{ "036", "attachment; filename=\"x/y\\\\z/file7.txt\"", "file7.txt" },
|
||||
{ "037", "attachment; filename=\"x/y\\\\z/\\\\x/file8.txt\"", "file8.txt" },
|
||||
{ "038", "attachment; filename=\"/root/file9.txt\"", "file9.txt" },
|
||||
{ "039", "attachment; filename=\"../file10.txt\"", "file10.txt" },
|
||||
{ "040", "attachment; filename=\"..\\\\file11.txt\"", "file11.txt" },
|
||||
{ "041", "attachment; filename=\"foo/../../file12.txt\"", "file12.txt" },
|
||||
};
|
||||
|
||||
@DataProvider(name = "positive")
|
||||
@ -178,19 +178,24 @@ public class AsFileDownloadTest {
|
||||
BodyHandler bh = ofFileDownload(tempDir.resolve(uri.getPath().substring(1)),
|
||||
CREATE, TRUNCATE_EXISTING, WRITE);
|
||||
HttpResponse<Path> response = client.send(request, bh);
|
||||
|
||||
Path body = response.body();
|
||||
out.println("Got response: " + response);
|
||||
out.println("Got body Path: " + response.body());
|
||||
out.println("Got body Path: " + body);
|
||||
String fileContents = new String(Files.readAllBytes(response.body()), UTF_8);
|
||||
out.println("Got body: " + fileContents);
|
||||
|
||||
assertEquals(response.statusCode(), 200);
|
||||
assertEquals(response.body().getFileName().toString(), expectedFilename);
|
||||
assertEquals(body.getFileName().toString(), expectedFilename);
|
||||
assertTrue(response.headers().firstValue("Content-Disposition").isPresent());
|
||||
assertEquals(response.headers().firstValue("Content-Disposition").get(),
|
||||
contentDispositionValue);
|
||||
assertEquals(fileContents, "May the luck of the Irish be with you!");
|
||||
|
||||
if (!body.toAbsolutePath().startsWith(tempDir.toAbsolutePath())) {
|
||||
System.out.println("Tempdir = " + tempDir.toAbsolutePath());
|
||||
System.out.println("body = " + body.toAbsolutePath());
|
||||
throw new AssertionError("body in wrong location");
|
||||
}
|
||||
// additional checks unrelated to file download
|
||||
caseInsensitivityOfHeaders(request.headers());
|
||||
caseInsensitivityOfHeaders(response.headers());
|
||||
|
||||
Loading…
Reference in New Issue
Block a user