You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/04/22 00:51:50 UTC
[GitHub] [lucene] gautamworah96 opened a new pull request, #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
gautamworah96 opened a new pull request, #829:
URL: https://github.com/apache/lucene/pull/829
In PR (https://github.com/apache/lucene/pull/762) we missed the case where a tempDir name was using `:` in the dir name. This test was passing in Linux, MacOS environments but ended up failing in Windows build systems.
By adding these additional checks we will reduce cases of these bugs slipping through to our CI systems.
The https://github.com/apache/lucene/pull/822 PR introduced a clean way for overriding the wrapPath(path) function and adding custom checks to different `<> extends FilterPath` classes.
JIRA: https://issues.apache.org/jira/browse/LUCENE-10525
# Solution
Add a new WindowsPath class that "checks" Paths before resolving them.
# Tests
Added a basic test to check the added logic. It is not randomized at the moment because WindowsFS does not have support for case-sensitive checks (and a random test can create duplicate file entries). There are some other questions as well that need some thought (about completeness of emulation etc, how to go about it, should we enable the tests in WindowsOS etc)
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/lucene/blob/main/CONTRIBUTING.md) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Lucene maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `main` branch.
- [x] I have run `./gradlew check`.
- [x] I have added tests for my changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gautamworah96 commented on a diff in pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
gautamworah96 commented on code in PR #829:
URL: https://github.com/apache/lucene/pull/829#discussion_r857259974
##########
lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsPath.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.tests.mockfile;
+
+import java.nio.file.InvalidPathException;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.HashSet;
+
+class WindowsPath extends FilterPath {
+
+ static HashSet<Character> RESERVED_CHARACTERS =
+ new HashSet<>(Arrays.asList('<', '>', ':', '\"', '/', '\\', '|', '?', '*'));
+
+ static HashSet<String> RESERVED_NAMES =
+ new HashSet<>(
+ Arrays.asList(
+ "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7",
+ "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8",
+ "LPT9"));
+
+ WindowsPath(Path path, FilterFileSystem fileSystem) {
+ super(path, fileSystem);
+ }
+
+ @Override
+ public Path resolve(Path other) {
+ checkInvalidPath(other);
+ return wrap(delegate.resolve(toDelegate(other)));
+ }
+
+ private void checkInvalidPath(Path other) {
+ String fileName = other.toString();
Review Comment:
Unix also handles this for us. Let's just rely on that instead of duplicating that logic here. Linux tests will catch file names with a `/` in them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gautamworah96 commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
gautamworah96 commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106733480
A gradle test on WindowsFS is failing because of the check we are introducing in this test. I'll fire up my Windows 10 machine and see if the test fails on the machine as well. If yes, then we might have caught an actual bug.
To reproduce: `./gradlew test --tests TestFilesystemResourceLoader.testBaseDir -Dtests.seed=45B0B8B9CE23FD94 -Dtests.locale=vun -Dtests.timezone=Pacific/Yap -Dtests.asserts=true -Dtests.file.encoding=UTF-8`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106782689
I opened https://issues.apache.org/jira/browse/LUCENE-10529 for the random facets failure that was triggered here while testing. It is not related to your changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106736469
Lemme see, ill help look. i wouldn't trust that it is just one test. ill try your branch, and hack it locally to use windowsfs as much as possible, see what trips :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106767024
k, tests are passing now by cleaning up the `FilterPath` abstraction. Now you don't have to worry about separators or anything, because the default methods work just fine for that.
I added some suggestions for the unit test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106870648
Thanks @dweiss! Currently, we can't actually use fake-windows (WindowsFS) on an actual Windows machine. It only works everywhere else.
The problem is the mechanism we use to track file handles: on *nix it is simply an inode, but it is something too heavy-duty on windows. if we layer WindowsFS on top of actual windows, it delivers us access-denied and other fun stuff when asking for this "key" (sorry, i dont know what actual windows mechanism java uses behind the scenes).
https://github.com/apache/lucene/blob/main/lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsFS.java#L55
So this is another issue to figure out one day...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
dweiss commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106884413
What a time-waster but uh... interesting! :) Added to the pile of todos.
As for Windows - I agree with your statement from a long time ago that it's one of the slowest filesystems out there... when you're doing something with lots of files (npm, I'm looking at you) the difference between Windows and Linux on the same machine is an order of magnitude, especially when deleting files (!).
This said, I swap between a mac, linux and windows and Windows has always been my preference. Call it a dirty habit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106757373
It is also bad that your subclass has to override 4 methods and deal with strings at all. These already have default implementations.
Let's make the string-based methods `final`. they already has good default implementations, we need to make this kind of thing easier. I also spot bugs in the current code where `FilterPath.endsWith(String)` is deferring to `startsWith()`. This is not helping our cause.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on a diff in pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on code in PR #829:
URL: https://github.com/apache/lucene/pull/829#discussion_r856481822
##########
lucene/test-framework/src/test/org/apache/lucene/tests/mockfile/TestWindowsFS.java:
##########
@@ -180,4 +186,22 @@ public void testMove() throws IOException {
assertEquals(2, stream.read());
}
}
+
+ public void testFileName() {
Review Comment:
personally i wouldnt do a random test for this.
I think it would be better to have a bunch of simple unit tests.
e.g. test that `path.resolve("hasa:colon")` fails and that `path.resolve("hasa:colon/butnotthispart")` fails, and so on?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gautamworah96 commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
gautamworah96 commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1108069323
> I'd like to avoid listing tons of files as a side effect of Path resolve operations, it is many extra i/os.
Yes I was concerned about this as well (probably need a cache here). + the added test breakage and complexity in the same PR. Removed that logic for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106745369
Yeah looks like the checking has to handle the separators properly. because a lot of stuff is allowed:
```
jshell> Path p = Paths.get("/home/rmuir")
p ==> /home/rmuir
jshell> p = p.resolve("Downloads/")
p ==> /home/rmuir/Downloads
jshell> Path p = Paths.get("/home/rmuir")
p ==> /home/rmuir
jshell> p = p.resolve("workspace/lucene/")
p ==> /home/rmuir/workspace/lucene
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106739760
It's not the only test. see my temporary commit to crank up WindowsFS. Just want to flush them all out at once so we can see what's happening.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
dweiss commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106863200
(late to the party - if you need me to check something on a physical Windows machine, let me know.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106882377
and just related to this, in case we wanted to fix it one day. I think, it made sense to use "fileKey" in the beginning when this tracking was simple. But somehow its now very complicated (renames, hard links, etc). So I don't think using the operating system "fileKey" necessarily even buys us anything. Maybe we could just track everything in our own Map without asking for this problematic "fileKey".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1107342012
LOL, I figured I would just try to see if you would bite. I want to make it better, my windows knowledge is just less, so it takes me longer. Also spinning up VMs is somehow painful for me.
We should really fix this thing to be better, I think this PR is a great step towards that.
If we can simulate it "well enough" to catch the most common problems, it can prevent windows hassles and give better CI coverage of windows (even when it is fake). I'm not happy we run the "fake windows" relatively rarely on *nix, and never on windows. But the fake windows is nice for people that don't want to deal with real windows :)
I just know I feel a lot better with occasionally having tests for IndexFileDeleter having to exercise their special windows retry logic, and so on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gautamworah96 commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
gautamworah96 commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1108049460
I've added a new revision that adds some more functionality, caught some bad edge cases and beasted the test for 2000 iters on a Mac. Only when I ran `./gradlew check` finally did I realize that the `io.File` class is forbidden :/ I'll find some time tomorrow and port the logic to `nio.Files`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir merged pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir merged PR #829:
URL: https://github.com/apache/lucene/pull/829
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on a diff in pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on code in PR #829:
URL: https://github.com/apache/lucene/pull/829#discussion_r855707426
##########
lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/FilterPath.java:
##########
@@ -270,7 +270,7 @@ public static Path unwrap(Path path) {
return Unwrappable.unwrapAll(path);
}
- private final Path wrap(Path other) {
+ final Path wrap(Path other) {
Review Comment:
maybe we can make it protected rather than package private. it is just a helper, so its harmless as long as it remains `final`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106741520
Looks like a bug in the checking? We may have to "normalize" the path first or something before doing the check?
I see fails like this:
```
java.nio.file.InvalidPathException: File name: trecdocs/ contains a reserved character: /: trecdocs/
```
Looks like the tests are just doing stuff like `path.resolve("trecdocs/")` to access a subdirectory. there's no actual `/` character in the name, if you get my drift.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gautamworah96 commented on a diff in pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
gautamworah96 commented on code in PR #829:
URL: https://github.com/apache/lucene/pull/829#discussion_r857255977
##########
lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsPath.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.tests.mockfile;
+
+import java.nio.file.InvalidPathException;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.HashSet;
+
+class WindowsPath extends FilterPath {
+
+ static HashSet<Character> RESERVED_CHARACTERS =
+ new HashSet<>(Arrays.asList('<', '>', ':', '\"', '/', '\\', '|', '?', '*'));
+
+ static HashSet<String> RESERVED_NAMES =
+ new HashSet<>(
+ Arrays.asList(
+ "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7",
+ "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8",
+ "LPT9"));
+
+ WindowsPath(Path path, FilterFileSystem fileSystem) {
+ super(path, fileSystem);
+ }
+
+ @Override
+ public Path resolve(Path other) {
+ checkInvalidPath(other);
+ return wrap(delegate.resolve(toDelegate(other)));
+ }
+
+ private void checkInvalidPath(Path other) {
+ String fileName = other.toString();
Review Comment:
Using `other.getFileName().toString()` fixes the failing CI build but it also allows cases like `foo/bar` to pass through (because the fileName is then just `bar`). Needs more investigation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gautamworah96 commented on a diff in pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
gautamworah96 commented on code in PR #829:
URL: https://github.com/apache/lucene/pull/829#discussion_r857252262
##########
lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/FilterPath.java:
##########
@@ -270,7 +270,7 @@ public static Path unwrap(Path path) {
return Unwrappable.unwrapAll(path);
}
- private final Path wrap(Path other) {
+ final Path wrap(Path other) {
Review Comment:
Added in [49ef22b](https://github.com/apache/lucene/pull/829/commits/49ef22b9b2de6f484937993cbbb65da7ed57f32b)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1108062297
i think we should avoid doing case-insensitivity here. maybe for another PR?
I'd like to avoid listing tons of files as a side effect of Path resolve operations, it is many extra i/os.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org