Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8345249: Apply some conservative cleanups in FileURLConnection #22459

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1995, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1995, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -46,17 +46,15 @@ public class FileURLConnection extends URLConnection {
private static final String TEXT_PLAIN = "text/plain";
private static final String LAST_MODIFIED = "last-modified";

String contentType;
InputStream is;
private final File file;
private InputStream is;
private List<String> directoryListing;

File file;
String filename;
boolean isDirectory = false;
boolean exists = false;
List<String> files;
private boolean isDirectory = false;
private boolean exists = false;

long length = -1;
long lastModified = 0;
private long length = -1;
private long lastModified = 0;

protected FileURLConnection(URL u, File file) {
super(u);
Expand All @@ -71,20 +69,17 @@ protected FileURLConnection(URL u, File file) {
*/
public void connect() throws IOException {
if (!connected) {
try {
filename = file.toString();
isDirectory = file.isDirectory();
if (isDirectory) {
String[] fileList = file.list();
if (fileList == null)
throw new FileNotFoundException(filename + " exists, but is not accessible");
files = Arrays.<String>asList(fileList);
} else {
is = new BufferedInputStream(new FileInputStream(filename));
}
} catch (IOException e) {
throw e;

isDirectory = file.isDirectory();
if (isDirectory) {
String[] fileList = file.list();
if (fileList == null)
throw new FileNotFoundException(file.getPath() + " exists, but is not accessible");
directoryListing = Arrays.<String>asList(fileList);
Copy link
Member

Choose a reason for hiding this comment

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

Is the <String> parameterization necessary? I tested on jshell, and it seems since the destination variable's type is List<String> we don't need this explicit parameterization to distinguish varargs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, indeed this can be dropped. See 06b2a23.

} else {
is = new BufferedInputStream(new FileInputStream(file.getPath()));
}

connected = true;
}
}
Expand All @@ -109,7 +104,7 @@ private void initializeHeaders() {

if (!isDirectory) {
FileNameMap map = java.net.URLConnection.getFileNameMap();
contentType = map.getContentTypeFor(filename);
String contentType = map.getContentTypeFor(file.getPath());
if (contentType != null) {
properties.add(CONTENT_TYPE, contentType);
}
Expand Down Expand Up @@ -179,32 +174,28 @@ public long getLastModified() {
public synchronized InputStream getInputStream()
throws IOException {

int iconHeight;
int iconWidth;

connect();

if (is == null) {
if (isDirectory) {
FileNameMap map = java.net.URLConnection.getFileNameMap();

StringBuilder sb = new StringBuilder();

if (files == null) {
throw new FileNotFoundException(filename);
if (directoryListing == null) {
throw new FileNotFoundException(file.getPath());
}

files.sort(Collator.getInstance());
directoryListing.sort(Collator.getInstance());

for (int i = 0 ; i < files.size() ; i++) {
String fileName = files.get(i);
for (int i = 0; i < directoryListing.size() ; i++) {
String fileName = directoryListing.get(i);
sb.append(fileName);
sb.append("\n");
Copy link
Member

Choose a reason for hiding this comment

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

We can use a StringJoiner or stream to join the strings instead. (Note there is a suffix \n in the strings)

Copy link
Contributor Author

@eirbjo eirbjo Nov 29, 2024

Choose a reason for hiding this comment

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

StringJoiner will not include the trailing line break. I want to be conservative in this PR so reluctant to introduce streams for this.

Opted to use an enhanced for-loop. See f9efa3b.

EDIT: Aha, StringJoiner, not String.join :-) Still, I think I prefer keeping the explicit iteration for this PR.

}
// Put it into a (default) locale-specific byte-stream.
is = new ByteArrayInputStream(sb.toString().getBytes());
} else {
throw new FileNotFoundException(filename);
throw new FileNotFoundException(file.getPath());
}
}
return is;
Expand Down