LUCENE-6886: Directory.createTempOutput always uses .tmp extension, and codecs are not allowed to

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1713103 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -85,6 +85,10 @@ Changes in Runtime Behavior
 * LUCENE-6789: IndexSearcher's default Similarity is changed to BM25Similarity.
   Use ClassicSimilarity to get the old vector space DefaultSimilarity. (Robert Muir)
 
+* LUCENE-6886: Reserve the .tmp file name extension for temp files,
+  and codec components are no longer allowed to use this extension
+  (Robert Muir, Mike McCandless)
+
 ======================= Lucene 5.4.0 =======================
 
 New Features

lucene/codecs/src/java/org/apache/lucene/codecs/memory/FSTTermsWriter.java

@@ -120,7 +120,7 @@ import org.apache.lucene.util.fst.Util;
  */
 
 public class FSTTermsWriter extends FieldsConsumer {
-  static final String TERMS_EXTENSION = "tmp";
+  static final String TERMS_EXTENSION = "tfp";
   static final String TERMS_CODEC_NAME = "FSTTerms";
   public static final int TERMS_VERSION_START = 2;
   public static final int TERMS_VERSION_CURRENT = TERMS_VERSION_START;

lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java

@@ -17,13 +17,6 @@ package org.apache.lucene.index;
  * limitations under the License.
  */
 
-import org.apache.lucene.store.AlreadyClosedException;
-import org.apache.lucene.store.Directory;
-import org.apache.lucene.util.CollectionUtil;
-import org.apache.lucene.util.Constants;
-import org.apache.lucene.util.IOUtils;
-import org.apache.lucene.util.InfoStream;
-
 import java.io.Closeable;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -34,11 +27,19 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.regex.Matcher;
 
+import org.apache.lucene.store.AlreadyClosedException;
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.util.CollectionUtil;
+import org.apache.lucene.util.Constants;
+import org.apache.lucene.util.IOUtils;
+import org.apache.lucene.util.InfoStream;
+
 /*
  * This class keeps track of each SegmentInfos instance that
  * is still "live", either because it corresponds to a
@@ -284,6 +285,11 @@ final class IndexFileDeleter implements Closeable {
         String segmentName = IndexFileNames.parseSegmentName(fileName);
         assert segmentName.startsWith("_"): "wtf? file=" + fileName;
 
+        if (fileName.toLowerCase(Locale.ROOT).endsWith(".tmp")) {
+          // A temp file: don't try to look at its gen
+          continue;
+        }
+
         maxSegmentName = Math.max(maxSegmentName, Integer.parseInt(segmentName.substring(1), Character.MAX_RADIX));
 
         Long curGen = maxPerSegmentGen.get(segmentName);

lucene/core/src/java/org/apache/lucene/index/SegmentInfo.java

@@ -21,6 +21,7 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -268,6 +269,9 @@ public final class SegmentInfo {
       if (!m.matches()) {
         throw new IllegalArgumentException("invalid codec filename '" + file + "', must match: " + IndexFileNames.CODEC_FILE_PATTERN.pattern());
       }
+      if (file.toLowerCase(Locale.ROOT).endsWith(".tmp")) {
+        throw new IllegalArgumentException("invalid codec filename '" + file + "', cannot end with .tmp extension");
+      }
     }
   }
   

lucene/core/src/java/org/apache/lucene/store/BaseDirectory.java

@@ -18,7 +18,6 @@ package org.apache.lucene.store;
  */
 
 import java.io.IOException;
-import java.util.Random;
 
 /**
  * Base implementation for a concrete {@link Directory} that uses a {@link LockFactory} for locking.
@@ -32,22 +31,6 @@ public abstract class BaseDirectory extends Directory {
    * this Directory instance). */
   protected final LockFactory lockFactory;
 
-  /** Subclasses can use this to generate temp file name candidates */
-  protected static final Random tempFileRandom;
-
-  static {
-    String prop = System.getProperty("tests.seed");
-    int seed;
-    if (prop != null) {
-      // So if there is a test failure that relied on temp file names,
-      //we remain reproducible based on the test seed:
-      seed = prop.hashCode();
-    } else {
-      seed = (int) System.currentTimeMillis();
-    }
-    tempFileRandom = new Random(seed);
-  }
-
   /** Sole constructor. */
   protected BaseDirectory(LockFactory lockFactory) {
     super();
@@ -64,13 +47,13 @@ public abstract class BaseDirectory extends Directory {
 
   @Override
   protected final void ensureOpen() throws AlreadyClosedException {
-    if (!isOpen)
+    if (!isOpen) {
       throw new AlreadyClosedException("this Directory is closed");
+    }
   }
 
   @Override
   public String toString() {
     return super.toString()  + " lockFactory=" + lockFactory;
   }
-  
 }

lucene/core/src/java/org/apache/lucene/store/Directory.java

@@ -73,7 +73,8 @@ public abstract class Directory implements Closeable {
   public abstract IndexOutput createOutput(String name, IOContext context) throws IOException;
 
   /** Creates a new, empty file for writing in the directory, with a
-   *  temporary file name derived from prefix and suffix.  Use
+   *  temporary file name including prefix and suffix, ending with the
+   *  reserved extension <code>.tmp</code>.  Use
    *  {@link IndexOutput#getName} to see what name was used.  */
   public abstract IndexOutput createTempOutput(String prefix, String suffix, IOContext context) throws IOException;
 

lucene/core/src/java/org/apache/lucene/store/FSDirectory.java

@@ -31,7 +31,9 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicLong;
 
+import org.apache.lucene.index.IndexFileNames;
 import org.apache.lucene.util.Constants;
 import org.apache.lucene.util.IOUtils;
 
@@ -118,6 +120,9 @@ public abstract class FSDirectory extends BaseDirectory {
 
   protected final Path directory; // The underlying filesystem directory
 
+  /** Used to generate temp file names in {@link #createTempOutput}. */
+  private final AtomicLong nextTempFileCounter = new AtomicLong();
+
   /** Create a new FSDirectory for the named location (ctor for subclasses).
    * The directory is created at the named location if it does not yet exist.
    * 
@@ -231,9 +236,10 @@ public abstract class FSDirectory extends BaseDirectory {
   public IndexOutput createTempOutput(String prefix, String suffix, IOContext context) throws IOException {
     ensureOpen();
     while (true) {
-      String name = prefix + tempFileRandom.nextInt(Integer.MAX_VALUE) + "." + suffix;
       try {
-        return new FSIndexOutput(name, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW);
+        String name = IndexFileNames.segmentFileName(prefix, suffix + "_" + Long.toString(nextTempFileCounter.getAndIncrement(), Character.MAX_RADIX), "tmp");
+        return new FSIndexOutput(name,
+                                 StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW);
       } catch (FileAlreadyExistsException faee) {
         // Retry with next random name
       }

lucene/core/src/java/org/apache/lucene/store/RAMDirectory.java

@@ -28,6 +28,7 @@ import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicLong;
 
+import org.apache.lucene.index.IndexFileNames;
 import org.apache.lucene.util.Accountable;
 import org.apache.lucene.util.Accountables;
 
@@ -51,6 +52,9 @@ public class RAMDirectory extends BaseDirectory implements Accountable {
   protected final Map<String,RAMFile> fileMap = new ConcurrentHashMap<>();
   protected final AtomicLong sizeInBytes = new AtomicLong();
   
+  /** Used to generate temp file names in {@link #createTempOutput}. */
+  private final AtomicLong nextTempFileCounter = new AtomicLong();
+
   /** Constructs an empty {@link Directory}. */
   public RAMDirectory() {
     this(new SingleInstanceLockFactory());
@@ -186,7 +190,7 @@ public class RAMDirectory extends BaseDirectory implements Accountable {
 
     // ... then try to find a unique name for it:
     while (true) {
-      String name = prefix + tempFileRandom.nextInt(Integer.MAX_VALUE) + "." + suffix;
+      String name = IndexFileNames.segmentFileName(prefix, suffix + "_" + Long.toString(nextTempFileCounter.getAndIncrement(), Character.MAX_RADIX), "tmp");
       if (fileMap.putIfAbsent(name, file) == null) {
         return new RAMOutputStream(name, file, true);
       }

lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java

@@ -18,9 +18,11 @@ package org.apache.lucene.index;
  */
 
 import java.io.ByteArrayOutputStream;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.PrintStream;
 import java.io.StringReader;
+import java.nio.file.NoSuchFileException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -2810,5 +2812,31 @@ public class TestIndexWriter extends LuceneTestCase {
     assertFalse(r2.getIndexCommit().getSegmentsFileName().equals(r.getIndexCommit().getSegmentsFileName()));
     IOUtils.close(r, r2, w, dir);
   }
+
+  public void testLeftoverTempFiles() throws Exception {
+    Directory dir = newDirectory();
+    if (dir instanceof MockDirectoryWrapper) {
+      ((MockDirectoryWrapper) dir).setEnableVirusScanner(false);
+    }
+    IndexWriterConfig iwc = new IndexWriterConfig(new MockAnalyzer(random()));
+    IndexWriter w = new IndexWriter(dir, iwc);
+    w.close();
+    
+    IndexOutput out = dir.createTempOutput("_0", "bkd", IOContext.DEFAULT);
+    String tempName = out.getName();
+    out.close();
+    iwc = new IndexWriterConfig(new MockAnalyzer(random()));
+    w = new IndexWriter(dir, iwc);
+
+    // Make sure IW deleted the unref'd file:
+    try {
+      dir.openInput(tempName, IOContext.DEFAULT);
+      fail("did not hit exception");
+    } catch (FileNotFoundException | NoSuchFileException e) {
+      // expected
+    }
+    w.close();
+    dir.close();
+  }
 }
 

lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java

@@ -30,6 +30,7 @@ import java.util.HashSet;
 import java.util.IdentityHashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Random;
 import java.util.Set;
@@ -620,6 +621,10 @@ public class MockDirectoryWrapper extends BaseDirectoryWrapper {
     
     IndexOutput delegateOutput = in.createTempOutput(prefix, suffix, LuceneTestCase.newIOContext(randomState, context));
     String name = delegateOutput.getName();
+    if (name.toLowerCase(Locale.ROOT).endsWith(".tmp") == false) {
+      throw new IllegalStateException("wrapped directory failed to use .tmp extension: got: " + name);
+    }
+
     unSyncedFiles.add(name);
     createdFiles.add(name);
     final IndexOutput io = new MockIndexOutputWrapper(this, delegateOutput, name);