Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replace '+' to '%2B' to handle page strs encode #1437

Merged
merged 5 commits into from
May 20, 2021
Merged

fix: replace '+' to '%2B' to handle page strs encode #1437

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bccf714
fix: replace all space in page to '+' before decode
imbajin May 7, 2021
7e26927
add test case for page decode (not ready)
imbajin May 11, 2021
209c7bd
add suitable test case contains 3 special base64 chars
imbajin May 12, 2021
e79dbdf
add PageState unit test & assert traversal not null
imbajin May 17, 2021
174066e
add some test case for PageState
imbajin May 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions hugegraph-core/src/main/java/com/baidu/hugegraph/backend/page/PageState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ public class PageState {

public static final byte[] EMPTY_BYTES = new byte[0];
public static final PageState EMPTY = new PageState(EMPTY_BYTES, 0, 0);
public static final char SPACE = ' ';
public static final char PLUS = '+';

private final byte[] position;
private final int offset;
Expand Down Expand Up @@ -73,6 +75,12 @@ private byte[] toBytes() {

public static PageState fromString(String page) {
E.checkNotNull(page, "page");
/*
* URLDecoder will auto decode '+' to space in url due to the request
* of HTML4, so we choose to replace the space to '+' after get it
imbajin marked this conversation as resolved.
Show resolved Hide resolved
* More details refer to #1437
*/
page = page.replace(SPACE, PLUS);
return fromBytes(toBytes(page));
}

Expand Down
32 changes: 32 additions & 0 deletions hugegraph-test/src/main/java/com/baidu/hugegraph/core/VertexCoreTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import com.baidu.hugegraph.backend.id.SnowflakeIdGenerator;
import com.baidu.hugegraph.backend.id.SplicingIdGenerator;
import com.baidu.hugegraph.backend.page.PageInfo;
import com.baidu.hugegraph.backend.page.PageState;
import com.baidu.hugegraph.backend.query.Condition;
import com.baidu.hugegraph.backend.query.ConditionQuery;
import com.baidu.hugegraph.backend.query.Query;
Expand Down Expand Up @@ -6667,6 +6668,37 @@ public void testQueryByPageWithInvalidPage() {
});
}


@Test
public void testQueryByPageWithSpecialBase64CharacterPage() {
imbajin marked this conversation as resolved.
Show resolved Hide resolved
// TODO: Not ready for commit, lack valid page contains '+' or '/'
imbajin marked this conversation as resolved.
Show resolved Hide resolved
// Assert decode space succees first
final String pageWithPlus = "5p2+"; // 松
imbajin marked this conversation as resolved.
Show resolved Hide resolved
final String pageWithSlash = "c3ViamVjdHM/X2Q9MQ==";
final String pageWithSpace = "5p2 ";

// Throws 'java.nio.BufferUnderflowException' now (bad test case)
PageState.fromString(pageWithSlash);
Assert.assertEquals(PageState.fromString(pageWithPlus),
PageState.fromString(pageWithSpace));

Assume.assumeTrue("Not support paging",
imbajin marked this conversation as resolved.
Show resolved Hide resolved
storeFeatures().supportsQueryByPage());
imbajin marked this conversation as resolved.
Show resolved Hide resolved

HugeGraph graph = graph();
init100Books();

// Assert not throws exception when contains '+' or '/'
// Contains valid character '+'
graph.traversal().V().has("~page", pageWithPlus).limit(10);

// Contains valid character '/'
graph.traversal().V().has("~page", pageWithSlash).limit(10);

// Contains invalid base64 character ' ', will be replaced to '+'
graph.traversal().V().has("~page", pageWithSpace).limit(10);
imbajin marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
public void testQueryByPageWithInvalidLimit() {
Assume.assumeTrue("Not support paging",
Expand Down