Author: tomwhite
Date: Tue Dec 15 05:43:03 2009
New Revision: 890651
URL: http://svn.apache.org/viewvc?rev=890651&view=rev
Log:
HADOOP-5901. FileSystem.fixName() has unexpected behaviour. Contributed by
Aaron Kimball.
Modified:
hadoop/common/trunk/CHANGES.txt
hadoop/common/trunk/src/java/org/apache/hadoop/fs/FileSystem.java
hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestFileSystemCaching.java
Modified: hadoop/common/trunk/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/CHANGES.txt?rev=890651&r1=890650&r2=890651&view=diff
==============================================================================
--- hadoop/common/trunk/CHANGES.txt (original)
+++ hadoop/common/trunk/CHANGES.txt Tue Dec 15 05:43:03 2009
@@ -94,6 +94,9 @@
HADOOP-6391. Classpath should not be part of command line arguments.
(Cristian Ivascu via tomwhite)
+ HADOOP-5901. FileSystem.fixName() has unexpected behaviour.
+ (Aaron Kimball via tomwhite)
+
Release 0.21.0 - Unreleased
INCOMPATIBLE CHANGES
Modified: hadoop/common/trunk/src/java/org/apache/hadoop/fs/FileSystem.java
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/src/java/org/apache/hadoop/fs/FileSystem.java?rev=890651&r1=890650&r2=890651&view=diff
==============================================================================
--- hadoop/common/trunk/src/java/org/apache/hadoop/fs/FileSystem.java (original)
+++ hadoop/common/trunk/src/java/org/apache/hadoop/fs/FileSystem.java Tue Dec
15 05:43:03 2009
@@ -21,6 +21,7 @@
import java.io.FileNotFoundException;
import java.io.IOException;
import java.net.URI;
+import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.EnumSet;
@@ -108,7 +109,15 @@
* @return the uri of the default filesystem
*/
public static URI getDefaultUri(Configuration conf) {
- return URI.create(fixName(conf.get(FS_DEFAULT_NAME_KEY, DEFAULT_FS)));
+ try {
+ String uri = conf.get(FS_DEFAULT_NAME_KEY, null);
+ checkName(uri);
+ return new URI(uri);
+ } catch (Exception e) {
+ // fs.default.name not set, or set to an invalid value. Create
+ // one based on a known-good URI
+ return URI.create(DEFAULT_FS);
+ }
}
/** Set the default filesystem URI in a configuration.
@@ -122,9 +131,12 @@
/** Set the default filesystem URI in a configuration.
* @param conf the configuration to alter
* @param uri the new default filesystem uri
+ * @throws IOException if the URI is invalid.
*/
- public static void setDefaultUri(Configuration conf, String uri) {
- setDefaultUri(conf, URI.create(fixName(uri)));
+ public static void setDefaultUri(Configuration conf, String uri)
+ throws IOException {
+ checkName(uri);
+ setDefaultUri(conf, URI.create(uri));
}
/** Called after a new FileSystem instance is constructed.
@@ -138,22 +150,52 @@
/** Returns a URI whose scheme and authority identify this FileSystem.*/
public abstract URI getUri();
-
- /** Update old-format filesystem names, for back-compatibility. This should
- * eventually be replaced with a checkName() method that throws an exception
- * for old-format names. */
- private static String fixName(String name) {
- // convert old-format name to new-format name
- if (name.equals("local")) { // "local" is now "file:///".
- LOG.warn("\"local\" is a deprecated filesystem name."
- +" Use \"file:///\" instead.");
- name = "file:///";
- } else if (name.indexOf('/')==-1) { // unqualified is "hdfs://"
- LOG.warn("\""+name+"\" is a deprecated filesystem name."
- +" Use \"hdfs://"+name+"/\" instead.");
- name = "hdfs://"+name;
+
+ /** Checks that a FileSystem name is given in an understandable format.
+ * The old "local" alias for "file:///" is unsupported, as are any
+ * URIs without a scheme component.
+ * @throws IOException if a name is in an unsupported format
+ */
+ private static void checkName(String name) throws IOException {
+ if (null == name) {
+ throw new IOException("Null FS name provided to checkName()");
+ } else if ("local".equals(name)) {
+ throw new IOException ("FileSystem 'local' is not supported; use
'file:///'");
+ } else {
+ // Try parsing this into a URI
+ try {
+ URI uri = new URI(name);
+
+ // No scheme; don't know how to parse this.
+ if (null == uri.getScheme()) {
+ throw new IOException("FileSystem name '" + name
+ + "' is provided in an unsupported format. (Try 'hdfs://"
+ + name + "' instead?)");
+ }
+
+ // This may have been a misparse. java.net.URI specifies that
+ // a URI is of the form:
+ // URI ::= [SCHEME-PART:]SCHEME-SPECIFIC-PART
+ //
+ // The scheme-specific-part may be parsed in numerous ways, but if
+ // it starts with a '/' character, that makes it a "hierarchical URI",
+ // subject to the following parsing:
+ // SCHEME-SPECIFIC-PART ::= "//" AUTHORITY-PART
+ // AUTHORITY-PART ::= [USER-INFO-PART] HOSTNAME [ ":" PORT ]
+ //
+ // In Hadoop, we require a host-based authority as well.
+ // java.net.URI parses left-to-right, so deprecated hostnames of the
+ // form 'foo:8020' will have 'foo' as their scheme and '8020' as their
+ // scheme-specific-part. We don't want this behavior.
+ if (null == uri.getAuthority()) {
+ throw new IOException("FileSystem name '" + name
+ + "' is provided in an unsupported format. (Try 'hdfs://"
+ + name + "' instead?)");
+ }
+ } catch (URISyntaxException use) {
+ throw new IOException("FileSystem name cannot be understood as a URI",
use);
+ }
}
- return name;
}
/**
Modified:
hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestFileSystemCaching.java
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestFileSystemCaching.java?rev=890651&r1=890650&r2=890651&view=diff
==============================================================================
---
hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestFileSystemCaching.java
(original)
+++
hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestFileSystemCaching.java
Tue Dec 15 05:43:03 2009
@@ -18,8 +18,12 @@
package org.apache.hadoop.fs;
+import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertSame;
+import static junit.framework.Assert.assertNotNull;
import static junit.framework.Assert.assertNotSame;
+import static junit.framework.Assert.assertTrue;
+import static junit.framework.Assert.fail;
import java.net.URI;
@@ -47,4 +51,94 @@
assertNotSame(fs1, fs2);
}
+ @Test
+ public void testGetLocal() throws Exception {
+ Configuration conf = new Configuration();
+ conf.set(CommonConfigurationKeys.FS_DEFAULT_NAME_KEY,
+ CommonConfigurationKeys.FS_DEFAULT_NAME_DEFAULT);
+ FileSystem fs1 = FileSystem.get(conf);
+ assertTrue(fs1 instanceof LocalFileSystem);
+
+ FileSystem fs2 = FileSystem.get(LocalFileSystem.NAME, conf);
+ assertTrue(fs2 instanceof LocalFileSystem);
+ }
+
+ @Test
+ public void testGetDefaultFs() throws Exception {
+ Configuration conf = new Configuration();
+ FileSystem fs = FileSystem.get(conf);
+ assertNotNull(fs);
+ }
+
+ private void checkDefaultFsParam(Configuration conf, String uri)
+ throws Exception {
+ conf.set(CommonConfigurationKeys.FS_DEFAULT_NAME_KEY, uri);
+ assertNotNull(FileSystem.get(conf));
+ }
+
+ @Test
+ public void testInvalidDefaultFsParam() throws Exception {
+ Configuration conf = new Configuration();
+
+ // All of the following set the default filesystem config
+ // to an invalid URI. Subsequent requests for a FileSystem
+ // should return the internal default fs.
+
+ checkDefaultFsParam(conf, "this-is-an-/invalid_uri");
+ checkDefaultFsParam(conf, "");
+ checkDefaultFsParam(conf, "/foo");
+ checkDefaultFsParam(conf, "/foo/bar");
+ checkDefaultFsParam(conf, "foo");
+ checkDefaultFsParam(conf, "foo:8020");
+ checkDefaultFsParam(conf, "foo/bar");
+ checkDefaultFsParam(conf, "foo:8020/bar");
+ checkDefaultFsParam(conf, "hdfs://");
+ checkDefaultFsParam(conf, "local");
+ }
+
+ @Test
+ public void testGetInvalidFs() throws Exception {
+ Configuration conf = new Configuration();
+
+ // None of these are valid FileSystem URIs. The default FS
+ // should be returned in all cases.
+
+ assertNotNull(FileSystem.get(URI.create("/foo"), conf));
+ assertNotNull(FileSystem.get(URI.create("/foo/bar"), conf));
+ assertNotNull(FileSystem.get(URI.create("foo"), conf));
+ assertNotNull(FileSystem.get(URI.create("foo/bar"), conf));
+ assertNotNull(FileSystem.get(URI.create("local"), conf));
+ }
+
+ private void checkBadUri(Configuration conf, String uri) {
+ try {
+ FileSystem.setDefaultUri(conf, uri);
+ fail("Expected invalid URI: " + uri);
+ } catch (Exception e) {
+ // Got expected exception; ok.
+ }
+ }
+
+ @Test
+ public void testDefaultUri() throws Exception {
+ Configuration conf = new Configuration();
+ URI uri = FileSystem.getDefaultUri(conf);
+ assertNotNull(uri.getScheme());
+
+ final URI exampleGoodUri = new URI("hdfs://foo.example.com:999");
+
+ FileSystem.setDefaultUri(conf, exampleGoodUri);
+ URI out = FileSystem.getDefaultUri(conf);
+ assertEquals(exampleGoodUri, out);
+
+ checkBadUri(conf, "bla"); // no scheme
+ checkBadUri(conf, "local"); // deprecated syntax
+ checkBadUri(conf, "foo:8020"); // no scheme, deprecated syntax.
+ checkBadUri(conf, "hdfs://"); // not a valid uri; requires authority-part
+ checkBadUri(conf, ""); // not a uri.
+
+ // Check that none of these actually changed the conf.
+ out = FileSystem.getDefaultUri(conf);
+ assertEquals(exampleGoodUri, out);
+ }
}
