Skip to content

Fix Dir.glob ../ and cases #8542

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.

Sign up for GitHub

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

Merged
enebo merged 5 commits into from
Feb 11, 2025
Merged

Fix Dir.glob ../ and cases #8542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c89686b
Revert: Fix Dir.glob with cases #6681
danini-the-panini Dec 21, 2024
3cd262c
Handle case-preserving case insensitive filesystems in Dir.glob
danini-the-panini Jan 16, 2025
f59a30f
handle schemas in glob patterns
danini-the-panini Jan 18, 2025
ea49a09
increase dir glob sceme detection indices
Jan 23, 2025
d7870b6
use Byte.toUnisignedInt and add documentation for GlobMagic
danini-the-panini Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/src/main/java/org/jruby/RubyDir.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ private static void globOptions(ThreadContext context, IRubyObject[] args, Strin

options.base = base;
}
if (processFlags) options.flags &= ~Dir.FNM_CASEFOLD;
}
}

Expand Down
128 changes: 86 additions & 42 deletions core/src/main/java/org/jruby/util/Dir.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
*/
public class Dir {
public final static boolean DOSISH = Platform.IS_WINDOWS;
public final static boolean CASEFOLD_FILESYSTEM = DOSISH;
public final static boolean CASEFOLD_FILESYSTEM = DOSISH || Platform.IS_MAC;
Comment thread
headius marked this conversation as resolved.

public final static int FNM_NOESCAPE = 0x01;
public final static int FNM_PATHNAME = 0x02;
Expand Down Expand Up @@ -653,21 +653,24 @@ public static ArrayList<String> braces(String pattern, int flags, ArrayList<Stri
return patterns;
}

private static boolean has_magic(byte[] bytes, int begin, int end, int flags) {
/* NOTE: Order here is important */
enum GlobMagic { PLAIN, ALPHA, MAGICAL };

private static GlobMagic has_magic(byte[] bytes, int begin, int end, int flags) {
boolean escape = (flags & FNM_NOESCAPE) == 0;
boolean nocase = (flags & FNM_CASEFOLD) != 0;
int open = 0;
boolean hasalpha = false;

for (int i = begin; i < end; i++) {
switch (bytes[i]) {
case '?':
case '*':
return true;
return GlobMagic.MAGICAL;
case '[': /* Only accept an open brace if there is a close */
open++; /* brace to match it. Bracket expressions must be */
continue; /* complete, according to Posix.2 */
case ']':
if (open > 0) return true;
if (open > 0) return GlobMagic.MAGICAL;

continue;
case '\\':
Expand All @@ -677,15 +680,21 @@ private static boolean has_magic(byte[] bytes, int begin, int end, int flags) {
// if (escape && i == end) return false;
// break;

if (escape) return true; // force magic logic whenever there's escaped chars
if (escape) return GlobMagic.MAGICAL; // force magic logic whenever there's escaped chars

continue;
default:
if (FNM_SYSCASE == 0 && nocase && Character.isLetter((char)(bytes[i] & 0xFF))) return true;
if (Platform.IS_WINDOWS || isAlpha(bytes[i])) {
hasalpha = true;
}
}
}

return false;
return hasalpha ? GlobMagic.ALPHA : GlobMagic.PLAIN;
}

private static boolean isAlpha(byte b) {
return Character.isLetter(Byte.toUnsignedInt(b));
}

private static int remove_backslashes(byte[] bytes, int index, int end) {
Expand Down Expand Up @@ -723,11 +732,37 @@ private static byte[] extract_elem(byte[] bytes, int begin, int end) {
return extract_path(bytes, begin, elementEnd);
}

private static byte[] extractScheme(byte[] path, int begin, int end) {
int colon = findScheme(path, begin, end);
if (colon == -1) return null;
do { colon++; } while (colon < end && path[colon] == '/');
colon--;
byte[] scheme = new byte[colon - begin];
System.arraycopy(path, begin, scheme, 0, colon - begin);
return scheme;
}

private static byte[] prependScheme(byte[] scheme, byte[] path, int begin, int end) {
byte [] newpath = new byte[scheme.length + (end - begin)];
System.arraycopy(scheme, 0, newpath, 0, scheme.length);
System.arraycopy(path, begin, newpath, scheme.length, end - begin);
return newpath;
}

// Win drive letter X:/
private static boolean beginsWithDriveLetter(byte[] path, int begin, int end) {
return DOSISH && begin + 2 < end && path[begin + 1] == ':' && isdirsep(path[begin + 2]);
}

private static int findScheme(byte[] path, int begin, int end) {
String schemeStr = new String(path, begin, Math.min(16, end - begin));
if (schemeStr.startsWith("uri:classloader:")) return 16;
if (schemeStr.startsWith("uri:")) return 4;
if (schemeStr.startsWith("file:")) return 5;
if (schemeStr.startsWith("classpath:")) return 10;
return -1;
}

// Is this nothing or literally root directory for the OS.
private static boolean isRoot(byte[] base) {
int length = base.length;
Expand Down Expand Up @@ -776,50 +811,39 @@ private static int addToResultIfExists(Ruby runtime, String cwd, byte[] bytes, i
FileResource file = JRubyFile.createResource(runtime, cwd, fileName);

if (file.exists()) {
byte[] newBytes;

// get the real filename (case-sensitive)
if(file.isFile() && cwd != null) {
String path = file.isSymLink() ? file.absolutePath() : file.canonicalPath();

// compensate for missing slash
if (fileName.endsWith("/")) {
path += "/";
}

if(fileName.contains("./")) {
newBytes = ("./" + path.substring((path.length() - end + 2))).getBytes();
} else {
int tempBegin = path.length() - fileName.length();
newBytes = path.substring(tempBegin).getBytes();
}

end = newBytes.length;
} else {
newBytes = bytes;
}

return func.call(newBytes, begin, end - begin, enc, arg);
return func.call(bytes, begin, end - begin, enc, arg);
}

return 0;
}

private static int glob_helper(Ruby runtime, String cwd, ByteList path, int sub, int flags, GlobFunc<GlobArgs> func, GlobArgs arg) {
int begin = path.getBegin();
final int end = begin + path.length();
final Encoding enc = path.getEncoding();
final byte[] bytes = path.getUnsafeBytes();
final byte[] scheme = extractScheme(bytes, begin, end);
if (scheme != null) begin += scheme.length;
return glob_helper(runtime, cwd, scheme, bytes, begin, end, enc, sub, flags, func, arg);
}

private static int glob_helper(Ruby runtime, String cwd, byte[] scheme, ByteList path, int sub, int flags, GlobFunc<GlobArgs> func, GlobArgs arg) {
final int begin = path.getBegin();
final int end = begin + path.getRealSize();
final int end = begin + path.length();
Comment on lines -810 to +832

@danini-the-panini danini-the-panini Jan 24, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i'm not sure if this changes anything. looks like length just returns realSize :/

@enebo enebo Jan 24, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@danini-the-panini length() is there to satisfy Java CharSequence. It is fine to leave it since it will always match byte length but it is a weird method since it implies it should return char length.

@headius headius Jan 27, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have discussed in the past modifying the CharSequence methods of ByteList to actually work against the contained characters rather than just the bytes but at this point it's so entrenched we'd have to break the world to do it. I think this is fine.

final Encoding enc = path.getEncoding();
return glob_helper(runtime, cwd, path.getUnsafeBytes(), begin, end, enc, sub, flags, func, arg);
return glob_helper(runtime, cwd, scheme, path.getUnsafeBytes(), begin, end, enc, sub, flags, func, arg);
}

private static int glob_helper(Ruby runtime, String cwd,
private static int glob_helper(Ruby runtime, String cwd, byte[] scheme,
byte[] path, int begin, int end, Encoding enc, int sub,
int flags, GlobFunc<GlobArgs> func, GlobArgs arg) {
int status = 0;

int ptr = sub != -1 ? sub : begin;

if ( ! has_magic(path, ptr, end, flags) ) {
final GlobMagic nonMagic = CASEFOLD_FILESYSTEM ? GlobMagic.PLAIN : GlobMagic.ALPHA;

if ( has_magic(path, ptr, end, flags).compareTo(nonMagic) <= 0 ) {
Comment thread
headius marked this conversation as resolved.
if ( DOSISH || (flags & FNM_NOESCAPE) == 0 ) {
if ( sub != -1 ) { // can modify path (our internal buf[])
end = remove_backslashes(path, sub, end);
Expand All @@ -833,6 +857,11 @@ private static int glob_helper(Ruby runtime, String cwd,
}
}

if (scheme != null) {
path = prependScheme(scheme, path, begin, end);
begin = 0; end = path.length;
}

if (end > begin) {
if ( isAbsolutePath(path, begin, end) ) {
status = addToResultIfExists(runtime, null, path, begin, end, enc, flags, func, arg);
Expand All @@ -854,10 +883,14 @@ private static int glob_helper(Ruby runtime, String cwd,
if ( path[ptr] == '/' ) ptr++;

final int SLASH_INDEX = indexOf(path, ptr, end, (byte) '/');
if (has_magic(path, ptr, SLASH_INDEX == -1 ? end : SLASH_INDEX, flags) ) {
final GlobMagic magical = has_magic(path, ptr, SLASH_INDEX == -1 ? end : SLASH_INDEX, flags);
if (magical.compareTo(nonMagic) > 0) {
finalize: do {
byte[] base = extract_path(path, begin, ptr);
byte[] dir = begin == ptr ? new byte[] { '.' } : base;
if (scheme != null) {
dir = prependScheme(scheme, dir, 0, dir.length);
}
byte[] magic = extract_elem(path, ptr, end);
boolean recursive = false;

Expand All @@ -884,15 +917,17 @@ private static int glob_helper(Ruby runtime, String cwd,
}
remainingPathStartIndex = lengthOfBase > 0 ? remainingPathStartIndex : remainingPathStartIndex + 1;
buf.append(path, remainingPathStartIndex, end - remainingPathStartIndex);
status = glob_helper(runtime, cwd, buf, lengthOfBase, flags, func, arg);
status = glob_helper(runtime, cwd, scheme, buf, lengthOfBase, flags, func, arg);
if ( status != 0 ) break finalize;
}
} else {
break mainLoop;
}

boolean skipdot = (flags & FNM_GLOB_SKIPDOT) != 0;
flags |= FNM_GLOB_SKIPDOT;
if (recursive || magical.compareTo(GlobMagic.MAGICAL) >= 0) {
flags |= FNM_GLOB_SKIPDOT;
}

final String[] files = files(resource);

Expand All @@ -917,19 +952,28 @@ private static int glob_helper(Ruby runtime, String cwd,
buf.append(base);
buf.append( isRoot(base) ? EMPTY : SLASH );
buf.append( getBytesInUTF8(file) );
resource = JRubyFile.createResource(runtime, cwd, new String(buf.unsafeBytes(), buf.begin(), buf.length(), enc.getCharset()));
byte [] bufBytes = buf.unsafeBytes();
int bufBegin = buf.begin();
int bufLen = buf.length();
if (scheme != null) {
bufBytes = prependScheme(scheme, bufBytes, bufBegin, bufBegin + bufLen);
bufBegin = 0;
bufLen = bufBytes.length;
}
resource = JRubyFile.createResource(runtime, cwd, new String(bufBytes, bufBegin, bufLen, enc.getCharset()));
if ( !resource.isSymLink() && resource.isDirectory() && !".".equals(file) && !"..".equals(file) ) {
final int len = buf.getRealSize();
buf.append(SLASH);
buf.append(DOUBLE_STAR);
buf.append(path, SLASH_INDEX, end - SLASH_INDEX);
status = glob_helper(runtime, cwd, buf, buf.getBegin() + len, flags, func, arg);
status = glob_helper(runtime, cwd, scheme, buf, buf.getBegin() + len, flags, func, arg);
if ( status != 0 ) break;
}
continue;
}
if ( fnmatch(magic, 0, magic.length, fileBytes, 0, fileBytes.length, flags) == 0 ) {
buf.length(0);
if (scheme != null && SLASH_INDEX == -1) buf.append(scheme);
buf.append(base);
buf.append( isRoot(base) ? EMPTY : SLASH );
buf.append( getBytesInUTF8(file) );
Expand All @@ -955,7 +999,7 @@ private static int glob_helper(Ruby runtime, String cwd,
buf.length(0);
buf.append(link);
buf.append(path, SLASH_INDEX, end - SLASH_INDEX);
status = glob_helper(runtime, cwd, buf, buf.getBegin() + len, flags, func, arg);
status = glob_helper(runtime, cwd, scheme, buf, buf.getBegin() + len, flags, func, arg);
}
}
}
Expand Down