HDFS-16503. Should verify whether the path name is valid in the WebHDFS #4067
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🎊 +1 overall
This message was automatically generated. |
|
Hi @tasanuma @ayushtkn @Hexiaoqiao , could you please take a look? Thanks. |
ayushtkn
reviewed
Mar 14, 2022
| @Test | ||
| public void testWebHdfsCreateWithInvalidPath() throws Exception { | ||
| final Configuration conf = WebHdfsTestUtil.createConf(); | ||
| cluster = new MiniDFSCluster.Builder(conf).numDataNodes(1).build(); |
There was a problem hiding this comment.
do you need a datanode, if you aren't writing any data, you can survive with 0 datanodes as well?
btw if not, the default is 1 only, you don't need to explicitly put one there. IIRC
There was a problem hiding this comment.
do you need a datanode, if you aren't writing any data, you can survive with 0 datanodes as well? btw if not, the default is 1 only, you don't need to explicitly put one there. IIRC
Thanks you @ayushtkn very much for the review. I fixed it.
|
🎊 +1 overall
This message was automatically generated. |
ayushtkn
reviewed
Mar 15, 2022
| @@ -478,7 +478,7 @@ private Path makeAbsolute(Path f) { | |||
| return f.isAbsolute()? f: new Path(workingDir, f); | |||
| } | |||
|
|
|||
| static Map<?, ?> jsonParse(final HttpURLConnection c, | |||
| public static Map<?, ?> jsonParse(final HttpURLConnection c, | |||
| final boolean useErrorStream) throws IOException { | |||
There was a problem hiding this comment.
Is it made public only for test?
If so please add VisibleForTesting Annotation
There was a problem hiding this comment.
OK. Thanks for your review.
ayushtkn
approved these changes
Mar 15, 2022
Thank you @ayushtkn . |
|
🎊 +1 overall
This message was automatically generated. |
|
Thanks @ayushtkn . |
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
…FS (apache#4067). Contributed by tomscut. Signed-off-by: Ayush Saxena <ayushsaxena@apache.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JIRA: HDFS-16503.
When creating a file using WebHDFS, there are two main steps:
Currently
NameNodeRpcSerververifies that pathName is valid, butNamenodeWebHdfsMethodsandRouterWebHdfsMethodsdo not.So if we use an invalid path(such as duplicated slash), the first step returns success, but the second step throws an
InvalidPathException.IMO, we should also do the validation in WebHdfs, which is consistent with the
NameNodeRpcServer.The same WebHDFS operations are:
CREATE,APPEND,OPEN,GETFILECHECKSUM. So we can addDFSUtil.isValidNametoredirectURIforNamenodeWebHdfsMethodsandRouterWebHdfsMethods.