[
https://issues.apache.org/jira/browse/HDFS-13226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16388924#comment-16388924
]
maobaolong commented on HDFS-13226:
-----------------------------------
{code:java}
public boolean validate() {
boolean ret = super.validate();
if (this.getSourcePath() == null || this.getSourcePath().length() == 0) {
LOG.error("Invalid entry, no source path specified ", this);
ret = false;
}
if (!this.getSourcePath().startsWith("/")) {
LOG.error("Invalid entry, all mount points must start with / ", this);
ret = false;
}
if (this.getDestinations() == null || this.getDestinations().size() == 0) {
LOG.error("Invalid entry, no destination paths specified ", this);
ret = false;
}
for (RemoteLocation loc : getDestinations()) {
String nsId = loc.getNameserviceId();
if (nsId == null || nsId.length() == 0) {
LOG.error("Invalid entry, invalid destination nameservice ", this);
ret = false;
}
if (loc.getDest() == null || loc.getDest().length() == 0) {
LOG.error("Invalid entry, invalid destination path ", this);
ret = false;
}
if (!loc.getDest().startsWith("/")) {
LOG.error("Invalid entry, all destination must start with / ", this);
ret = false;
}
}
return ret;
}
{code}
Let's discuss about this method. I think this method have the following problem:
- if this.getSourcePath() return null, the this.getSourcePath().startsWith("/")
will throw NPE.
- if the sourcePath is null, the validate method will not be invoked, instead,
the NPE will be taken place as follow stack.
{code:java}
java.lang.NullPointerException
at
org.apache.hadoop.hdfs.federation.protocol.proto.HdfsServerFederationProtos$GetMountTableEntriesRequestProto$Builder.setSrcPath(HdfsServerFederationProtos.java:15937)
at
org.apache.hadoop.hdfs.server.federation.store.protocol.impl.pb.GetMountTableEntriesRequestPBImpl.setSrcPath(GetMountTableEntriesRequestPBImpl.java:73)
at
org.apache.hadoop.hdfs.server.federation.store.protocol.GetMountTableEntriesRequest.newInstance(GetMountTableEntriesRequest.java:38)
at
org.apache.hadoop.hdfs.tools.federation.RouterAdmin.addMount(RouterAdmin.java:280)
at
org.apache.hadoop.hdfs.tools.federation.RouterAdmin.addMount(RouterAdmin.java:258)
at
org.apache.hadoop.hdfs.tools.federation.RouterAdmin.run(RouterAdmin.java:154)
and normalizeFileSystemPath method found the null or empty src or dest first.
{code}
- if the nsId is null,
{code:bash}
java.lang.NullPointerException
at
org.apache.hadoop.hdfs.tools.federation.RouterAdmin.addMount(RouterAdmin.java:224)
at
org.apache.hadoop.hdfs.tools.federation.RouterAdmin.run(RouterAdmin.java:154)
{code}
- if the source start with more than one '/', the entry will be create
successfully as a different mount entry as the single '/' version.
for example
{code:bash}
hdfs dfsrouteradmin -add //addnode/ ns1 //addnode/
hdfs dfsrouteradmin -add /addnode/ ns1 /addnode/
Mount Table Entries:
Source Destinations Owner
Group Mode Quota/Usage
//addnode/ ns1->//addnode/ hadp
hadp rwxr-xr-x [NsQuota: -/-, SsQuota: -/-]
/addnode ns1->/addnode hadp
hadp rwxr-xr-x [NsQuota: -/-, SsQuota: -/-]
{code}
- if an error have occurred, should we check the following problem?
So, I want to modify to the following version of validate:
{code:java}
public boolean validate() {
if (!super.validate()) {
return false;
}
if (!this.getSourcePath().startsWith("/")
|| this.getSourcePath().startsWith("//")) {
LOG.error("Invalid entry, all mount points must start with a single / ",
this);
return false;
}
for (RemoteLocation loc : getDestinations()) {
String nsId = loc.getNameserviceId();
if (nsId.length() == 0) {
LOG.error("Invalid entry, invalid destination nameservice ", this);
return false;
} else if (!loc.getDest().startsWith("/") ||
this.getSourcePath().startsWith("//")) {
LOG.error("Invalid entry, all destination must start with a single / ",
this);
return false;
}
}
return true;
}
{code}
Or
{code:java}
@Override
public boolean validate() {
if (!super.validate()) {
return false;
}
if (!this.getSourcePath().startsWith("/")
|| this.getSourcePath().startsWith("//")) {
throw new IllegalArgumentException("Invalid entry, all mount points must
start with a single / ");
}
for (RemoteLocation loc : getDestinations()) {
String nsId = loc.getNameserviceId();
if (nsId.length() == 0) {
throw new IllegalArgumentException("Invalid entry, invalid destination
nameservice ");
} else if (!loc.getDest().startsWith("/") ||
this.getSourcePath().startsWith("//")) {
throw new IllegalArgumentException("Invalid entry, all destination must
start with a single / ");
}
}
return true;
}
{code}
> RBF: We should throw the failure validate and refuse this mount entry
> ---------------------------------------------------------------------
>
> Key: HDFS-13226
> URL: https://issues.apache.org/jira/browse/HDFS-13226
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs
> Affects Versions: 3.2.0
> Reporter: maobaolong
> Assignee: maobaolong
> Priority: Major
> Labels: RBF
> Fix For: 3.2.0
>
> Attachments: HDFS-13226.001.patch
>
>
> one of the mount entry source path rule is that the source path must start
> with '\', somebody didn't follow the rule and execute the following command:
> {code:bash}
> $ hdfs dfsrouteradmin -add addnode/ ns1 /addnode/
> {code}
> But, the console show we are successful add this entry.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
