Skip to content

Commit

Permalink
HADOOP-14929. Cleanup usage of decodecomponent and use QueryStringDec…
Browse files Browse the repository at this point in the history 
…oder from netty. Contributed by Bharat Viswanadham.
  • Loading branch information
@arp7
arp7 committed Nov 11, 2017
1 parent 8a1bd9a commit 1d6f8be
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 60 deletions.
66 changes: 7 additions & 59 deletions ...dfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/web/webhdfs/ParameterParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@

import java.io.IOException;
import java.net.URI;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.EnumSet;
import java.util.List;
Expand Down Expand Up @@ -143,9 +142,13 @@ public boolean createParent() {
}

public EnumSet<CreateFlag> createFlag() {
String cf =
decodeComponent(param(CreateFlagParam.NAME), StandardCharsets.UTF_8);

String cf = "";
if (param(CreateFlagParam.NAME) != null) {
QueryStringDecoder decoder = new QueryStringDecoder(
param(CreateFlagParam.NAME),
StandardCharsets.UTF_8);
cf = decoder.path();
}
return new CreateFlagParam(cf).getValue();
}

Expand All @@ -158,61 +161,6 @@ private String param(String key) {
return p == null ? null : p.get(0);
}

/**
* The following function behaves exactly the same as netty's
* <code>QueryStringDecoder#decodeComponent</code> except that it
* does not decode the '+' character as space. WebHDFS takes this scheme
* to maintain the backward-compatibility for pre-2.7 releases.
*/
private static String decodeComponent(final String s, final Charset charset) {
if (s == null) {
return "";
}
final int size = s.length();
boolean modified = false;
for (int i = 0; i < size; i++) {
final char c = s.charAt(i);
if (c == '%' || c == '+') {
modified = true;
break;
}
}
if (!modified) {
return s;
}
final byte[] buf = new byte[size];
int pos = 0; // position in `buf'.
for (int i = 0; i < size; i++) {
char c = s.charAt(i);
if (c == '%') {
if (i == size - 1) {
throw new IllegalArgumentException("unterminated escape sequence at" +
" end of string: " + s);
}
c = s.charAt(++i);
if (c == '%') {
buf[pos++] = '%'; // "%%" -> "%"
break;
}
if (i == size - 1) {
throw new IllegalArgumentException("partial escape sequence at end " +
"of string: " + s);
}
c = decodeHexNibble(c);
final char c2 = decodeHexNibble(s.charAt(++i));
if (c == Character.MAX_VALUE || c2 == Character.MAX_VALUE) {
throw new IllegalArgumentException(
"invalid escape sequence `%" + s.charAt(i - 1) + s.charAt(
i) + "' at index " + (i - 2) + " of: " + s);
}
c = (char) (c * 16 + c2);
// Fall through.
}
buf[pos++] = (byte) c;
}
return new String(buf, 0, pos, charset);
}

/**
* Helper to decode half of a hexadecimal number from a string.
* @param c The ASCII character of the hexadecimal number to decode.
Expand Down
81 changes: 80 additions & 1 deletion ...src/test/java/org/apache/hadoop/hdfs/server/datanode/web/webhdfs/TestParameterParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,25 @@
package org.apache.hadoop.hdfs.server.datanode.web.webhdfs;

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.CreateFlag;
import org.apache.hadoop.hdfs.DFSTestUtil;
import org.apache.hadoop.hdfs.HAUtilClient;
import org.apache.hadoop.hdfs.security.token.delegation.DelegationTokenIdentifier;
import org.apache.hadoop.hdfs.web.resources.DelegationParam;
import org.apache.hadoop.hdfs.web.resources.NamenodeAddressParam;
import org.apache.hadoop.hdfs.web.resources.OffsetParam;
import org.apache.hadoop.security.token.Token;
import org.apache.hadoop.test.GenericTestUtils;
import org.junit.Assert;
import org.junit.Test;

import static org.junit.Assert.fail;

import io.netty.handler.codec.http.QueryStringDecoder;

import java.io.IOException;
import java.util.EnumSet;

import static org.mockito.Mockito.mock;

public class TestParameterParser {
private static final String LOGICAL_NAME = "minidfs";
Expand Down Expand Up @@ -63,6 +67,81 @@ public void testDecodePath() {
Assert.assertEquals(EXPECTED_PATH, testParser.path());
}

@Test
public void testCreateFlag() {
final String path = "/test1?createflag=append,sync_block";
Configuration conf = new Configuration();
QueryStringDecoder decoder = new QueryStringDecoder(
WebHdfsHandler.WEBHDFS_PREFIX + path);
ParameterParser testParser = new ParameterParser(decoder, conf);
EnumSet<CreateFlag> actual = testParser.createFlag();
EnumSet<CreateFlag> expected = EnumSet.of(CreateFlag.APPEND,
CreateFlag.SYNC_BLOCK);
Assert.assertEquals(expected.toString(), actual.toString());


final String path1 = "/test1?createflag=append";
decoder = new QueryStringDecoder(
WebHdfsHandler.WEBHDFS_PREFIX + path1);
testParser = new ParameterParser(decoder, conf);

actual = testParser.createFlag();
expected = EnumSet.of(CreateFlag.APPEND);
Assert.assertEquals(expected, actual);

final String path2 = "/test1";
decoder = new QueryStringDecoder(
WebHdfsHandler.WEBHDFS_PREFIX + path2);
testParser = new ParameterParser(decoder, conf);
actual = testParser.createFlag();
Assert.assertEquals(0, actual.size());

final String path3 = "/test1?createflag=create,overwrite";
decoder = new QueryStringDecoder(
WebHdfsHandler.WEBHDFS_PREFIX + path3);
testParser = new ParameterParser(decoder, conf);
actual = testParser.createFlag();
expected = EnumSet.of(CreateFlag.CREATE, CreateFlag
.OVERWRITE);
Assert.assertEquals(expected.toString(), actual.toString());


final String path4 = "/test1?createflag=";
decoder = new QueryStringDecoder(
WebHdfsHandler.WEBHDFS_PREFIX + path4);
testParser = new ParameterParser(decoder, conf);
actual = testParser.createFlag();
Assert.assertEquals(0, actual.size());

//Incorrect value passed to createflag
try {
final String path5 = "/test1?createflag=overwrite,";
decoder = new QueryStringDecoder(
WebHdfsHandler.WEBHDFS_PREFIX + path5);
testParser = new ParameterParser(decoder, conf);
actual = testParser.createFlag();
fail("It should throw Illegal Argument Exception");
} catch (Exception e) {
GenericTestUtils
.assertExceptionContains("No enum constant", e);
}

//Incorrect value passed to createflag
try {
final String path6 = "/test1?createflag=,";
decoder = new QueryStringDecoder(
WebHdfsHandler.WEBHDFS_PREFIX + path6);
testParser = new ParameterParser(decoder, conf);
actual = testParser.createFlag();
fail("It should throw Illegal Argument Exception");
} catch (Exception e) {
GenericTestUtils
.assertExceptionContains("No enum constant", e);
}


}

@Test
public void testOffset() throws IOException {
final long X = 42;
Expand Down

0 comments on commit 1d6f8be

Please sign in to comment.