diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index a2fd2234fc9..555c97d8247 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -57,6 +57,10 @@ Bug Fixes * LUCENE-7891: Lucene's taxonomy facets now uses a non-buggy LRU cache by default. (Jan-Willem van den Broek via Mike McCandless) +* LUCENE-7959: Improve NativeFSLockFactory's exception message if it cannot create + write.lock for an empty index due to bad permissions/read-only filesystem/etc. + (Erick Erickson, Shawn Heisey, Robert Muir) + Build * SOLR-11181: Switch order of maven artifact publishing procedure: deploy first diff --git a/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java b/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java index 4f17d951a6f..106b02e1619 100644 --- a/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java +++ b/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java @@ -93,15 +93,27 @@ public final class NativeFSLockFactory extends FSLockFactory { Path lockFile = lockDir.resolve(lockName); + IOException creationException = null; try { Files.createFile(lockFile); } catch (IOException ignore) { // we must create the file to have a truly canonical path. // if it's already created, we don't care. if it cant be created, it will fail below. + creationException = ignore; } // fails if the lock file does not exist - final Path realPath = lockFile.toRealPath(); + final Path realPath; + try { + realPath = lockFile.toRealPath(); + } catch (IOException e) { + // if we couldn't resolve the lock file, it might be because we couldn't create it. + // so append any exception from createFile as a suppressed exception, in case its useful + if (creationException != null) { + e.addSuppressed(creationException); + } + throw e; + } // used as a best-effort check, to see if the underlying file has changed final FileTime creationTime = Files.readAttributes(realPath, BasicFileAttributes.class).creationTime(); diff --git a/lucene/core/src/test/org/apache/lucene/store/TestNativeFSLockFactory.java b/lucene/core/src/test/org/apache/lucene/store/TestNativeFSLockFactory.java index 5d202bff9e3..dc771781a34 100644 --- a/lucene/core/src/test/org/apache/lucene/store/TestNativeFSLockFactory.java +++ b/lucene/core/src/test/org/apache/lucene/store/TestNativeFSLockFactory.java @@ -18,9 +18,17 @@ package org.apache.lucene.store; import java.io.IOException; +import java.nio.channels.SeekableByteChannel; +import java.nio.file.AccessDeniedException; +import java.nio.file.FileSystem; import java.nio.file.Files; +import java.nio.file.OpenOption; import java.nio.file.Path; +import java.nio.file.attribute.FileAttribute; +import java.util.Set; +import org.apache.lucene.mockfile.FilterFileSystemProvider; +import org.apache.lucene.mockfile.FilterPath; import org.apache.lucene.util.IOUtils; import org.apache.lucene.util.TestUtil; @@ -89,4 +97,38 @@ public class TestNativeFSLockFactory extends BaseLockFactoryTestCase { IOUtils.closeWhileHandlingException(lock); } } + + /** MockFileSystem that throws AccessDeniedException on creating test.lock */ + static class MockBadPermissionsFileSystem extends FilterFileSystemProvider { + public MockBadPermissionsFileSystem(FileSystem delegateInstance) { + super("mockbadpermissions://", delegateInstance); + } + + @Override + public SeekableByteChannel newByteChannel(Path path, Set options, FileAttribute... attrs) throws IOException { + if (path.getFileName().toString().equals("test.lock")) { + throw new AccessDeniedException(path.toString(), null, "fake access denied"); + } + return super.newByteChannel(path, options, attrs); + } + } + + public void testBadPermissions() throws IOException { + // create a mock filesystem that will throw exc on creating test.lock + Path tmpDir = createTempDir(); + tmpDir = FilterPath.unwrap(tmpDir).toRealPath(); + FileSystem mock = new MockBadPermissionsFileSystem(tmpDir.getFileSystem()).getFileSystem(null); + Path mockPath = mock.getPath(tmpDir.toString()); + + // we should get an IOException (typically NoSuchFileException but no guarantee) with + // our fake AccessDenied added as suppressed. + Directory dir = getDirectory(mockPath.resolve("indexDir")); + IOException expected = expectThrows(IOException.class, () -> { + dir.obtainLock("test.lock"); + }); + AccessDeniedException suppressed = (AccessDeniedException) expected.getSuppressed()[0]; + assertTrue(suppressed.getMessage().contains("fake access denied")); + + dir.close(); + } }