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

Add methods requiring connect to forbidden apis #22964

Merged
merged 11 commits into from
Feb 7, 2017

Conversation

Tim-Brooks
Copy link
Contributor

@Tim-Brooks Tim-Brooks commented Feb 3, 2017

This is related to #22116. This commit adds calls that require
SocketPermission connect to forbidden APIs.

The following calls are now forbidden:

  • java.net.URL#openStream()
  • java.net.Socket#connect(java.net.SocketAddress)
  • java.net.Socket#connect(java.net.SocketAddress, int)
  • java.nio.channels.SocketChannel#open(java.net.SocketAddress)
  • java.nio.channels.SocketChannel#connect(java.net.SocketAddress)

Copy link
Contributor

@s1monw s1monw left a comment

Choose a reason for hiding this comment

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

LGTM but FileSystemUtils didn't like it.. feel free to push once the build passes

@Tim-Brooks
Copy link
Contributor Author

Hey @s1monw - yeah there were a few places where I missed openStream(). So I'll submit updates.

For the FileSystemUtils do you just want me to suppress forbidden? Another option is to not use URL (I think). Most of the FileSystemUtils methods deal with paths. And both callers of newBufferedReader originate with a Path.

I could change:

public static BufferedReader newBufferedReader(URL url, Charset cs) throws IOException {
    CharsetDecoder decoder = cs.newDecoder();
    Reader reader = new InputStreamReader(url.openStream(), decoder);
    return new BufferedReader(reader);
}

To:

public static BufferedReader newBufferedReader(Path path, Charset cs) throws IOException {
    return Files.newBufferedReader(path, cs);
}

Or not have that method and just call Files.newBufferedReader(path, cs). But maybe I am missing a reason that we need to change these Path instances to URL? Or maybe there is a reason we don't want to use NIO in this case?

@s1monw
Copy link
Contributor

s1monw commented Feb 6, 2017

Or not have that method and just call Files.newBufferedReader(path, cs).

++ just trash that method! good catch

@Tim-Brooks
Copy link
Contributor Author

@s1monw can you give me an updated review based on my changes please?

@s1monw
Copy link
Contributor

s1monw commented Feb 7, 2017

@elasticmachine test this please

@s1monw
Copy link
Contributor

s1monw commented Feb 7, 2017

LGTM provided the tests pass

@Tim-Brooks Tim-Brooks merged commit fcc568f into elastic:master Feb 7, 2017
@Tim-Brooks Tim-Brooks deleted the add_connect_to_forbidden branch February 7, 2017 20:41
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team v6.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants