Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 0e17f04cdc78
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 3ffb84daf34a
Choose a head ref
  • 3 commits
  • 4 files changed
  • 1 contributor

Commits on Jun 10, 2018

  1. [refactor] avoid regexp on JarResource create + hide internal class

    @kares
    kares committed Jun 10, 2018
    Copy the full SHA
    8b0b706 View commit details

  2. prefer using impls as delegates + deprecate unused methods

    @kares
    kares committed Jun 10, 2018

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
    GPG key ID: 4AEE18F83AFDEB23
    Expired
    Verified
    Learn about vigilant mode
    Copy the full SHA
    7fce4a6 View commit details

  3. [refactor] dry-out and review JRubyFile creation 

    no need to use JavaSecuredFile for isAbsolute check
while also generate less noise on listFiles impl
    @kares
    kares committed Jun 10, 2018
    Copy the full SHA
    3ffb84d View commit details
6 changes: 6 additions & 0 deletions core/src/main/java/org/jruby/RubyFileTest.java
View file
Original file line number Diff line number Diff line change
@@ -80,6 +80,7 @@ public static IRubyObject chardev_p(IRubyObject recv, IRubyObject filename) {
return recv.getRuntime().newBoolean(stat != null && stat.isCharDev());
}

@Deprecated
public static IRubyObject directory_p(IRubyObject recv, IRubyObject filename) {
return directory_p(recv.getRuntime().getCurrentContext(), recv, filename);
}
@@ -118,6 +119,7 @@ public static IRubyObject executable_real_p(IRubyObject recv, IRubyObject filena
}
}

@Deprecated
public static IRubyObject exist_p(IRubyObject recv, IRubyObject filename) {
return exist_p(recv.getRuntime().getCurrentContext(), recv, filename);
}
@@ -131,6 +133,7 @@ public static IRubyObject exist_p(ThreadContext context, IRubyObject recv, IRuby
!Ruby.isSecurityRestricted() && fileResource(context, path).exists());
}

@Deprecated
public static RubyBoolean file_p(IRubyObject recv, IRubyObject filename) {
return file_p(recv.getRuntime().getCurrentContext(), recv, filename);
}
@@ -194,6 +197,7 @@ public static IRubyObject pipe_p(IRubyObject recv, IRubyObject filename) {
return recv.getRuntime().newBoolean(stat != null && stat.isNamedPipe());
}

@Deprecated
public static IRubyObject readable_p(IRubyObject recv, IRubyObject filename) {
return readable_p(recv.getRuntime().getCurrentContext(), recv, filename);
}
@@ -247,6 +251,7 @@ public static IRubyObject size(ThreadContext context, IRubyObject recv, IRubyObj
return context.runtime.newFixnum(stat.st_size());
}

@Deprecated
public static IRubyObject size_p(IRubyObject recv, IRubyObject filename) {
return size_p(recv.getRuntime().getCurrentContext(), recv, filename);
}
@@ -292,6 +297,7 @@ public static RubyBoolean writable_p(IRubyObject recv, IRubyObject filename) {
return filename.getRuntime().newBoolean(fileResource(filename).canWrite());
}

@Deprecated
public static RubyBoolean zero_p(IRubyObject recv, IRubyObject filename) {
return zero_p(recv.getRuntime().getCurrentContext(), recv, filename);
}
21 changes: 9 additions & 12 deletions core/src/main/java/org/jruby/RubyKernel.java
View file
Original file line number Diff line number Diff line change
@@ -1445,16 +1445,14 @@ public static IRubyObject test(ThreadContext context, IRubyObject recv, IRubyObj
int cmd;
if (args[0] instanceof RubyFixnum) {
cmd = (int)((RubyFixnum) args[0]).getLongValue();
} else if (args[0] instanceof RubyString &&
((RubyString) args[0]).getByteList().length() > 0) {
} else if (args[0] instanceof RubyString && ((RubyString) args[0]).getByteList().length() > 0) {
// MRI behavior: use first byte of string value if len > 0
cmd = ((RubyString) args[0]).getByteList().charAt(0);
} else {
cmd = (int) args[0].convertToInteger().getLongValue();
}

// MRI behavior: raise ArgumentError for 'unknown command' before
// checking number of args.
// MRI behavior: raise ArgumentError for 'unknown command' before checking number of args
switch(cmd) {
case 'A': case 'b': case 'c': case 'C': case 'd': case 'e': case 'f': case 'g': case 'G':
case 'k': case 'M': case 'l': case 'o': case 'O': case 'p': case 'r': case 'R': case 's':
@@ -1490,11 +1488,11 @@ public static IRubyObject test(ThreadContext context, IRubyObject recv, IRubyObj
case 'C': // ?C | Time | Last change time for file1
return context.runtime.newFileStat(args[1].convertToString().toString(), false).ctime();
case 'd': // ?d | boolean | True if file1 exists and is a directory
return RubyFileTest.directory_p(recv, args[1]);
return RubyFileTest.directory_p(context, recv, args[1]);
case 'e': // ?e | boolean | True if file1 exists
return RubyFileTest.exist_p(recv, args[1]);
return RubyFileTest.exist_p(context, recv, args[1]);
case 'f': // ?f | boolean | True if file1 exists and is a regular file
return RubyFileTest.file_p(recv, args[1]);
return RubyFileTest.file_p(context, recv, args[1]);
case 'g': // ?g | boolean | True if file1 has the \CF{setgid} bit
return RubyFileTest.setgid_p(recv, args[1]);
case 'G': // ?G | boolean | True if file1 exists and has a group ownership equal to the caller's group
@@ -1512,12 +1510,11 @@ public static IRubyObject test(ThreadContext context, IRubyObject recv, IRubyObj
case 'p': // ?p | boolean | True if file1 exists and is a fifo
return RubyFileTest.pipe_p(recv, args[1]);
case 'r': // ?r | boolean | True if file1 is readable by the effective uid/gid of the caller
return RubyFileTest.readable_p(recv, args[1]);
return RubyFileTest.readable_p(context, recv, args[1]);
case 'R': // ?R | boolean | True if file is readable by the real uid/gid of the caller
// FIXME: Need to implement an readable_real_p in FileTest
return RubyFileTest.readable_p(recv, args[1]);
return RubyFileTest.readable_p(context, recv, args[1]);
case 's': // ?s | int/nil | If file1 has nonzero size, return the size, otherwise nil
return RubyFileTest.size_p(recv, args[1]);
return RubyFileTest.size_p(context, recv, args[1]);
case 'S': // ?S | boolean | True if file1 exists and is a socket
return RubyFileTest.socket_p(recv, args[1]);
case 'u': // ?u | boolean | True if file1 has the setuid bit set
@@ -1532,7 +1529,7 @@ public static IRubyObject test(ThreadContext context, IRubyObject recv, IRubyObj
case 'X': // ?X | boolean | True if file1 exists and is executable by the real uid/gid
return RubyFileTest.executable_real_p(recv, args[1]);
case 'z': // ?z | boolean | True if file1 exists and has a zero length
return RubyFileTest.zero_p(recv, args[1]);
return RubyFileTest.zero_p(context, recv, args[1]);
case '=': // ?= | boolean | True if the modification times of file1 and file2 are equal
return context.runtime.newFileStat(args[1].convertToString().toString(), false).mtimeEquals(args[2]);
case '<': // ?< | boolean | True if the modification time of file1 is prior to that of file2
59 changes: 22 additions & 37 deletions core/src/main/java/org/jruby/util/JRubyFile.java
View file
Original file line number Diff line number Diff line change
@@ -115,10 +115,7 @@ private static FileResource createResource(Ruby runtime, String cwd, String path
}

public static String normalizeSeps(String path) {
if (Platform.IS_WINDOWS) {
return path.replace(File.separatorChar, '/');
}
return path;
return Platform.IS_WINDOWS ? path.replace(File.separatorChar, '/') : path;
}

private static JRubyFile createNoUnicodeConversion(String cwd, String pathname) {
@@ -128,19 +125,22 @@ private static JRubyFile createNoUnicodeConversion(String cwd, String pathname)
if (pathname.startsWith("file:")) {
pathname = pathname.substring(5);
}
File internal = new JavaSecuredFile(pathname);
return createNoUnicodeConversion(cwd, pathname, new File(pathname));
}

private static JRubyFile createNoUnicodeConversion(String cwd, String pathname, File path) {
// File and company do not seem to recognize bare \ and / on Windows as absolute. Cheat!
if (internal.isAbsolute() || Platform.IS_WINDOWS && (pathname.startsWith("/") || pathname.startsWith("\\"))) {
return new JRubyFile(internal);
if (path.isAbsolute() || Platform.IS_WINDOWS && (pathname.startsWith("/") || pathname.startsWith("\\"))) {
return new JRubyFile(path);
}
if(cwd != null && cwd.startsWith("uri:") && !pathname.startsWith("uri:") && !pathname.contains("!/")) {
if (cwd != null && cwd.startsWith("uri:") && !pathname.startsWith("uri:") && !pathname.contains("!/")) {
return new JRubyFile(cwd + '/' + pathname);
}
internal = new JavaSecuredFile(cwd, pathname);
if(!internal.isAbsolute()) {
path = new File(cwd, pathname);
if (!path.isAbsolute()) {
throw new IllegalArgumentException("Neither current working directory ("+cwd+") nor pathname ("+pathname+") led to an absolute path");
}
return new JRubyFile(internal);
return new JRubyFile(path);
}

public static String getFileProperty(String property) {
@@ -250,42 +250,27 @@ public String[] list(FilenameFilter filter) {

@Override
public File[] listFiles() {
File[] files = super.listFiles();
if (files == null) {
return null;
}

JRubyFile[] smartFiles = new JRubyFile[files.length];
for (int i = 0, j = files.length; i < j; i++) {
smartFiles[i] = createNoUnicodeConversion(super.getAbsolutePath(), files[i].getPath());
}
return smartFiles;
return convertFiles(super.listFiles());
}

@Override
public File[] listFiles(final FileFilter filter) {
final File[] files = super.listFiles(filter);
if (files == null) {
return null;
}

JRubyFile[] smartFiles = new JRubyFile[files.length];
for (int i = 0,j = files.length; i < j; i++) {
smartFiles[i] = createNoUnicodeConversion(super.getAbsolutePath(), files[i].getPath());
}
return smartFiles;
return convertFiles(super.listFiles(filter));
}

@Override
public File[] listFiles(final FilenameFilter filter) {
final File[] files = super.listFiles(filter);
if (files == null) {
return null;
}
return convertFiles(super.listFiles(filter));
}

private JRubyFile[] convertFiles(final File[] files) {
if (files == null) return null; // non-existent directory

final String absolutePath = super.getAbsolutePath();
JRubyFile[] smartFiles = new JRubyFile[files.length];
for (int i = 0,j = files.length; i < j; i++) {
smartFiles[i] = createNoUnicodeConversion(super.getAbsolutePath(), files[i].getPath());
for (int i = 0; i < files.length; i++) {
final File file = files[i];
smartFiles[i] = createNoUnicodeConversion(absolutePath, file.getPath(), file);
}
return smartFiles;
}
45 changes: 25 additions & 20 deletions core/src/main/java/org/jruby/util/JarResource.java
View file
Original file line number Diff line number Diff line change
@@ -5,23 +5,29 @@
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.util.jar.JarEntry;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public abstract class JarResource extends AbstractFileResource {
private static Pattern PREFIX_MATCH = Pattern.compile("^(?:jar:)?(?:file:)?(.*)$");
abstract class JarResource extends AbstractFileResource {

private static final JarCache jarCache = new JarCache();

public static JarResource create(String pathname) {
if (!pathname.contains("!")) return null; // Optimization: no ! no jar!
int bang = pathname.indexOf('!');
if (bang == -1) return null; // no ! no jar!

Matcher matcher = PREFIX_MATCH.matcher(pathname);
String sanitized = matcher.matches() ? matcher.group(1) : pathname;
if (pathname.startsWith("jar:")) {
if (pathname.startsWith("file:", 4)) {
pathname = pathname.substring(9); bang -= 9; // 4 + 5
}
else {
pathname = pathname.substring(4); bang -= 4;
}
}
else if (pathname.startsWith("file:")) {
pathname = pathname.substring(5); bang -= 5;
}

int bang = sanitized.indexOf('!');
String jarPath = sanitized.substring(0, bang);
String entryPath = sanitized.substring(bang + 1);
String jarPath = pathname.substring(0, bang);
String entryPath = pathname.substring(bang + 1);
// normalize path -- issue #2017
if (entryPath.startsWith("//")) entryPath = entryPath.substring(1);

@@ -38,32 +44,31 @@ public static JarResource create(String pathname) {
private static JarResource createJarResource(String jarPath, String entryPath, boolean rootSlashPrefix) {
JarCache.JarIndex index = jarCache.getIndex(jarPath);

if (index == null) {
// Jar doesn't exist
if (index == null) { // Jar doesn't exist
try {
jarPath = URLDecoder.decode(jarPath, "UTF-8");
entryPath = URLDecoder.decode(entryPath, "UTF-8");
} catch (IllegalArgumentException iae) {
}
catch (IllegalArgumentException e) {
// something in the path did not decode, so it's probably not a URI
// See jruby/jruby#2264.
return null;
} catch (UnsupportedEncodingException e) {
throw new RuntimeException( "hmm - system does not know UTF-8 string encoding :(" );
}
catch (UnsupportedEncodingException e) {
throw new AssertionError(e);
}
index = jarCache.getIndex(jarPath);

if (index == null) {
// Jar doesn't exist
return null;
}
if (index == null) return null; // Jar doesn't exist
}

// Try it as directory first, because jars tend to have foo/ entries
// and it's not really possible disambiguate between files and directories.
String[] entries = index.getDirEntries(entryPath);
if (entries != null) {
return new JarDirectoryResource(jarPath, rootSlashPrefix, entryPath, entries);
} else if (entryPath.length() > 1 && entryPath.endsWith("/")) { // in case 'foo/' passed
}
if (entryPath.length() > 1 && entryPath.endsWith("/")) { // in case 'foo/' passed
entries = index.getDirEntries(entryPath.substring(0, entryPath.length() - 1));

if (entries != null) {